Register undo/redo keyboard shortcuts for block editor#78
Register undo/redo keyboard shortcuts for block editor#78
Conversation
Press This uses BlockEditorProvider directly, which only registers block-level keyboard shortcuts (duplicate, remove, etc.) via BlockEditorKeyboardShortcuts. The undo/redo shortcuts (Ctrl+Z, Ctrl+Shift+Z, Ctrl+Y) are normally registered by EditorProvider from @wordpress/editor, which Press This doesn't use. Without these shortcuts, Ctrl+Z stops working after paste because Gutenberg intercepts the paste event and processes it programmatically, breaking the browser's native contentEditable undo stack. Fixes #75
There was a problem hiding this comment.
Pull request overview
This PR adds undo/redo keyboard shortcuts (Ctrl+Z/Cmd+Z for undo, Ctrl+Shift+Z/Cmd+Shift+Z for redo, and Ctrl+Y/Cmd+Y for redo) to the Press This block editor to fix a bug where undo stopped working after pasting content. Press This uses BlockEditorProvider directly, which only provides BlockEditorKeyboardShortcuts (for block-level operations) but not EditorKeyboardShortcuts (which handles undo/redo in the full Gutenberg editor). The implementation adds a document-level keyboard event listener that dispatches undo/redo actions from the block editor store while preserving native undo behavior in form fields.
Changes:
- Adds
useDispatchimport from@wordpress/datato access undo/redo actions - Implements a
useEffecthook with a document-level keydown event listener that handles Ctrl+Z, Ctrl+Shift+Z, and Ctrl+Y shortcuts - Excludes
<input>,<textarea>, and<select>elements to preserve native undo in form fields
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Cmd+Y on macOS is a system-level shortcut, not redo. Only bind the Y key when ctrlKey is true and metaKey is false, so it fires on Windows/Linux where Ctrl+Y is the standard redo convention. macOS already has Cmd+Shift+Z for redo.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The core/block-editor store does not provide undo/redo actions. In full Gutenberg, EditorProvider manages undo via core-data entity edits, but Press This uses BlockEditorProvider directly with React state, so the toolbar buttons were never functional either. Replace the non-functional store-based approach with a custom undo stack: - Separate onInput (non-persistent, e.g. typing) from onChange (persistent, e.g. paste/block ops) on BlockEditorProvider - Push to undo stack on persistent changes, clear redo stack - Wire Ctrl+Z / Cmd+Z, Ctrl+Shift+Z / Cmd+Shift+Z, Ctrl+Y keyboard shortcuts to the custom undo/redo handlers - Expose undo/redo state to Header via App for toolbar buttons - Remove unused useSelect/useDispatch/blockEditorStore from Header Fixes #75
|
Functional testing via wp-env and browser automation revealed that the initial approach (dispatching The fix was rewritten to use a custom undo/redo stack managed in React state, with |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| undoStackRef.current = [ | ||
| ...undoStackRef.current, | ||
| blocksRef.current, | ||
| ]; | ||
| redoStackRef.current = []; | ||
| setBlocks( ( prevBlocks ) => [ ...prevBlocks, ...newBlocks ] ); |
There was a problem hiding this comment.
The undo level is saved using blocksRef.current (line 402) while setBlocks uses a functional update with prevBlocks (line 405). There's a potential race condition where blocksRef.current might not be in sync with the actual previous blocks at the time setBlocks is called. Consider using the prevBlocks value from the functional update to ensure the undo stack captures the actual previous state: save prevBlocks to the undo stack within the functional update, or use setBlocks with a non-functional update to maintain consistency.
| undoStackRef.current = [ | |
| ...undoStackRef.current, | |
| blocksRef.current, | |
| ]; | |
| redoStackRef.current = []; | |
| setBlocks( ( prevBlocks ) => [ ...prevBlocks, ...newBlocks ] ); | |
| setBlocks( ( prevBlocks ) => { | |
| undoStackRef.current = [ | |
| ...undoStackRef.current, | |
| prevBlocks, | |
| ]; | |
| redoStackRef.current = []; | |
| return [ ...prevBlocks, ...newBlocks ]; | |
| } ); |
| const handleUndo = useCallback( () => { | ||
| if ( undoStackRef.current.length === 0 ) { | ||
| return; | ||
| } | ||
| const previousBlocks = undoStackRef.current.pop(); | ||
| redoStackRef.current.push( blocksRef.current ); | ||
| isUndoingRef.current = true; | ||
| setBlocks( previousBlocks ); | ||
| syncUndoRedoState(); | ||
| }, [ syncUndoRedoState ] ); |
There was a problem hiding this comment.
If a user rapidly presses undo/redo multiple times, blocksRef.current might not be updated with the new blocks before the next operation. The useEffect that syncs blocksRef (line 293) depends on blocks state, which updates asynchronously after setBlocks is called. This could lead to incorrect undo/redo stack entries. Consider updating blocksRef.current immediately in handleUndo/handleRedo right after calling setBlocks, e.g.: blocksRef.current = previousBlocks; after line 309. This ensures the ref is in sync even if React hasn't re-rendered yet.
|
|
||
| // Notify parent that we've processed the scrape. | ||
| onScrapeProcessed(); | ||
| }, [ pendingScrape, onScrapeProcessed ] ); // eslint-disable-line react-hooks/exhaustive-deps |
There was a problem hiding this comment.
The effect uses title (line 393) to check if it's empty, but title is not in the dependency array (line 411). If the effect runs multiple times with different pendingScrape values before title updates, the stale title value could cause incorrect behavior. While this is unlikely in normal usage, consider adding title to the dependency array or using a ref to track whether the title has been set by a scrape operation.
| }, [ pendingScrape, onScrapeProcessed ] ); // eslint-disable-line react-hooks/exhaustive-deps | |
| }, [ pendingScrape, onScrapeProcessed, title ] ); // eslint-disable-line react-hooks/exhaustive-deps |
| * @param {Array} newBlocks Updated blocks. | ||
| */ | ||
| const handleBlocksChange = useCallback( ( newBlocks ) => { | ||
| const handleBlocksInput = useCallback( ( newBlocks ) => { |
There was a problem hiding this comment.
handleBlocksInput doesn't check the isUndoingRef flag. If BlockEditorProvider calls both onInput and onChange during an undo/redo operation, handleBlocksInput could update the blocks state independently, potentially causing race conditions or incorrect state. Consider adding the same isUndoingRef check to handleBlocksInput to ensure consistent behavior, or verify that BlockEditorProvider never calls onInput during controlled value changes.
| const handleBlocksInput = useCallback( ( newBlocks ) => { | |
| const handleBlocksInput = useCallback( ( newBlocks ) => { | |
| // Avoid conflicting updates during undo/redo operations. | |
| if ( isUndoingRef.current ) { | |
| isUndoingRef.current = false; | |
| setBlocks( newBlocks ); | |
| return; | |
| } |
| // Parse initial content. | ||
| useEffect( () => { | ||
| if ( post.content ) { |
There was a problem hiding this comment.
The initial content parsing (line 381) doesn't create an undo level. This means the initial parsed blocks won't be on the undo stack. While this is probably intentional for initial load, it creates an inconsistency with the pendingScrape handling (lines 400-406) which does create an undo level. Consider whether initial content should also create an undo level, or document why this difference exists.
| // Don't create undo levels for undo/redo operations. | ||
| if ( isUndoingRef.current ) { | ||
| isUndoingRef.current = false; | ||
| setBlocks( newBlocks ); |
There was a problem hiding this comment.
When isUndoingRef.current is true, handleBlocksChange calls setBlocks(newBlocks) which is redundant since setBlocks was already called in handleUndo/handleRedo (lines 309, 320). If BlockEditorProvider's onChange is being triggered by external value prop changes, this creates an extra unnecessary render. Consider simply returning early without calling setBlocks again, since the blocks state is already being updated by the undo/redo operation.
| setBlocks( newBlocks ); |
Prevents a race condition where rapid undo/redo could read stale blocksRef.current before React re-renders, causing incorrect entries on the redo/undo stack.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if ( onUndoReady ) { | ||
| onUndoReady( { handleUndo, handleRedo, hasUndo, hasRedo } ); | ||
| } | ||
| }, [ onUndoReady, handleUndo, handleRedo, hasUndo, hasRedo ] ); // eslint-disable-line react-hooks/exhaustive-deps |
There was a problem hiding this comment.
The eslint-disable-line react-hooks/exhaustive-deps comment on this line is unnecessary. The dependency array [ onUndoReady, handleUndo, handleRedo, hasUndo, hasRedo ] already includes all values used inside the useEffect, so the linter would not report a violation here. The disable comment is misleading — it implies there is an intentional rule suppression, when in fact all dependencies are correctly listed.
| }, [ onUndoReady, handleUndo, handleRedo, hasUndo, hasRedo ] ); // eslint-disable-line react-hooks/exhaustive-deps | |
| }, [ onUndoReady, handleUndo, handleRedo, hasUndo, hasRedo ] ); |
| // Undo/Redo stack. | ||
| // The core/block-editor store does not provide undo/redo actions. | ||
| // In full Gutenberg, EditorProvider manages undo via core-data entity edits, | ||
| // but Press This uses BlockEditorProvider directly with React state. | ||
| const undoStackRef = useRef( [] ); | ||
| const redoStackRef = useRef( [] ); |
There was a problem hiding this comment.
The PR description states the implementation "Dispatches undo() / redo() from the block editor store", but the actual implementation uses a custom React-state-based undo/redo stack (undoStackRef, redoStackRef) rather than the block editor store's undo/redo actions. The PR description should be updated to reflect that a custom stack is used because the block editor store doesn't expose undo/redo actions at the core/block-editor level.
| const handleUndo = useCallback( () => { | ||
| if ( undoStackRef.current.length === 0 ) { | ||
| return; | ||
| } | ||
| const previousBlocks = undoStackRef.current.pop(); | ||
| redoStackRef.current.push( blocksRef.current ); | ||
| isUndoingRef.current = true; | ||
| blocksRef.current = previousBlocks; | ||
| setBlocks( previousBlocks ); | ||
| syncUndoRedoState(); | ||
| }, [ syncUndoRedoState ] ); | ||
|
|
||
| const handleRedo = useCallback( () => { | ||
| if ( redoStackRef.current.length === 0 ) { | ||
| return; | ||
| } | ||
| const nextBlocks = redoStackRef.current.pop(); | ||
| undoStackRef.current.push( blocksRef.current ); | ||
| isUndoingRef.current = true; | ||
| blocksRef.current = nextBlocks; | ||
| setBlocks( nextBlocks ); | ||
| syncUndoRedoState(); | ||
| }, [ syncUndoRedoState ] ); |
There was a problem hiding this comment.
After handleUndo() or handleRedo() sets isUndoingRef.current = true (lines 308 and 320), the flag is only reset inside handleBlocksChange (line 435). However, BlockEditorProvider's onChange callback is only triggered by user interactions within the editor — not by external changes to its value prop. So when setBlocks(previousBlocks/nextBlocks) is called from handleUndo/handleRedo, it updates the value prop of BlockEditorProvider, but handleBlocksChange is never invoked in response. This means isUndoingRef.current stays true indefinitely after any undo or redo, causing the first subsequent user-initiated persistent change (paste, block add/remove) to skip creating an undo level, and clearing the redo stack silently without pushing the current state onto the undo stack. The fix is to reset isUndoingRef.current = false at the end of handleUndo and handleRedo directly, rather than relying on handleBlocksChange to do it.
| // Expose undo/redo to parent (for Header buttons). | ||
| useEffect( () => { | ||
| if ( onUndoReady ) { | ||
| onUndoReady( { handleUndo, handleRedo, hasUndo, hasRedo } ); | ||
| } | ||
| }, [ onUndoReady, handleUndo, handleRedo, hasUndo, hasRedo ] ); // eslint-disable-line react-hooks/exhaustive-deps |
There was a problem hiding this comment.
The new undo/redo functionality (keyboard shortcuts, stack management, and the handleUndo/handleRedo callbacks) has no test coverage. The existing component tests in tests/components/ follow a pattern of checking source code for required props and behaviors. A test file similar to tests/components/header-publish-controls.test.js should be added to verify that PressThisEditor accepts and exposes onUndoReady, and that Header accepts onUndo, onRedo, hasUndo, hasRedo props and wires them to the undo/redo buttons.
Summary
undo()/redo()from the block editor store, matching whatEditorKeyboardShortcutsprovides in the full Gutenberg editor<input>,<textarea>,<select>) so native undo continues to work in the title input and URL scannerPress This uses
BlockEditorProviderdirectly rather than the fullEditorProviderfrom@wordpress/editor.BlockEditorKeyboardShortcutsonly registers block-level shortcuts (duplicate, remove, etc.) — not undo/redo. After paste, Gutenberg intercepts the event and processes it programmatically, which breaks the browser's native contentEditable undo stack. Without store-backed shortcuts, Ctrl+Z has nothing to dispatch.Fixes #75
Test plan