Skip to content

Register undo/redo keyboard shortcuts for block editor #78

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kraftbj wants to merge 4 commits into
base: trunk
Choose a base branch
from
Open

Register undo/redo keyboard shortcuts for block editor #78

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
412f745
Register undo/redo keyboard shortcuts for block editor
kraftbj Feb 27, 2026
54e29bc
Restrict Ctrl+Y redo shortcut to Windows/Linux only
kraftbj Feb 27, 2026
0b979e7
Implement custom undo/redo stack for block editor
kraftbj Feb 27, 2026
6040f6a
Sync blocksRef immediately during undo/redo operations
kraftbj Feb 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions src/App.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,18 @@ export default function App() {
setSaveState( state );
}, [] );

// State for undo/redo from editor.
const [ undoState, setUndoState ] = useState( {
handleUndo: null,
handleRedo: null,
hasUndo: false,
hasRedo: false,
} );

const handleUndoReady = useCallback( ( state ) => {
setUndoState( state );
}, [] );

return (
<div className="press-this-app">
<Header
Expand All @@ -328,6 +340,10 @@ export default function App() {
onSave={ saveState.handleSave }
isSaving={ saveState.isSaving }
publishLabel={ saveState.publishLabel }
onUndo={ undoState.handleUndo }
onRedo={ undoState.handleRedo }
hasUndo={ undoState.hasUndo }
hasRedo={ undoState.hasRedo }
/>

<div className="press-this-app__body">
Expand All @@ -344,6 +360,7 @@ export default function App() {
pendingScrape={ pendingScrape }
onScrapeProcessed={ handleScrapeProcessed }
onSaveReady={ handleSaveReady }
onUndoReady={ handleUndoReady }
categoryNonce={ data.categoryNonce || '' }
ajaxUrl={ data.ajaxUrl || '' }
/>
Expand Down
33 changes: 10 additions & 23 deletions src/components/Header.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
* WordPress dependencies
*/
import { useState, useCallback, useEffect, useRef } from '@wordpress/element';
import { useSelect, useDispatch } from '@wordpress/data';
import {
Button,
TextControl,
Expand All @@ -28,7 +27,6 @@ import {
redo as redoIcon,
moreVertical,
} from '@wordpress/icons';
import { store as blockEditorStore } from '@wordpress/block-editor';

/**
* Internal dependencies
Expand Down Expand Up @@ -64,6 +62,10 @@ function isMacOS() {
* @param {Function} props.onSave Callback for save operations (status, options).
* @param {boolean} props.isSaving Whether a save operation is in progress.
* @param {string} props.publishLabel Label for the publish button.
* @param {Function} props.onUndo Undo callback.
* @param {Function} props.onRedo Redo callback.
* @param {boolean} props.hasUndo Whether undo is available.
* @param {boolean} props.hasRedo Whether redo is available.
* @return {JSX.Element} Header component.
*/
export default function Header( {
Expand All @@ -80,6 +82,10 @@ export default function Header( {
onSave,
isSaving = false,
publishLabel = __( 'Publish', 'press-this' ),
onUndo,
onRedo,
hasUndo = false,
hasRedo = false,
} ) {
const [ scanUrl, setScanUrl ] = useState( sourceUrl || '' );
const [ isScanning, setIsScanning ] = useState( false );
Expand All @@ -90,25 +96,6 @@ export default function Header( {
// Track if initial auto-scan has been performed.
const hasAutoScanned = useRef( false );

// Undo/Redo state from block editor store.
// Note: These selectors require BlockEditorProvider context.
// If called outside context, they will return false/noop.
const { hasUndo, hasRedo } = useSelect( ( select ) => {
const store = select( blockEditorStore );
// Check if hasUndo/hasRedo exist (they might not if outside BlockEditorProvider context).
const canUndo =
typeof store.hasUndo === 'function' ? store.hasUndo() : false;
const canRedo =
typeof store.hasRedo === 'function' ? store.hasRedo() : false;
return {
hasUndo: canUndo,
hasRedo: canRedo,
};
}, [] );

// Undo/Redo actions from block editor store.
const { undo, redo } = useDispatch( blockEditorStore );

// Keyboard shortcut hints based on platform.
const undoShortcut = isMacOS() ? '\u2318Z' : 'Ctrl+Z';
const redoShortcut = isMacOS() ? '\u21E7\u2318Z' : 'Ctrl+Shift+Z';
Expand Down Expand Up @@ -329,7 +316,7 @@ export default function Header( {
<Button
className="press-this-header__toolbar-button"
icon={ undoIcon }
onClick={ undo }
onClick={ onUndo }
disabled={ ! hasUndo }
aria-label={ __( 'Undo', 'press-this' ) }
/>
Expand All @@ -343,7 +330,7 @@ export default function Header( {
<Button
className="press-this-header__toolbar-button"
icon={ redoIcon }
onClick={ redo }
onClick={ onRedo }
disabled={ ! hasRedo }
aria-label={ __( 'Redo', 'press-this' ) }
/>
Expand Down
140 changes: 136 additions & 4 deletions src/components/PressThisEditor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@ function getWpRestBaseUrl( pressThisRestUrl ) {
* @param {Object} props.pendingScrape Pending scraped content to append.
* @param {Function} props.onScrapeProcessed Callback after scrape is processed.
* @param {Function} props.onSaveReady Callback when save handler is ready (receives { handleSave, isSaving, publishLabel }).
* @param {Function} props.onUndoReady Callback when undo/redo handlers are ready (receives { handleUndo, handleRedo, hasUndo, hasRedo }).
* @param {string} props.categoryNonce
* @param {string} props.ajaxUrl
* @return {JSX.Element} Press This Editor component.
Expand All @@ -236,6 +237,7 @@ export default function PressThisEditor( {
pendingScrape = null,
onScrapeProcessed = () => {},
onSaveReady = () => {},
onUndoReady = () => {},
categoryNonce = '',
ajaxUrl = '',
} ) {
Expand Down Expand Up @@ -276,6 +278,104 @@ export default function PressThisEditor( {
const [ isLoadingTags, setIsLoadingTags ] = useState( false );
const tagSearchTimeout = useRef( null );

// 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( [] );
Comment on lines +281 to +286
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
const blocksRef = useRef( blocks );
const isUndoingRef = useRef( false );
const [ hasUndo, setHasUndo ] = useState( false );
const [ hasRedo, setHasRedo ] = useState( false );

// Keep blocksRef in sync.
useEffect( () => {
blocksRef.current = blocks;
}, [ blocks ] );

const syncUndoRedoState = useCallback( () => {
setHasUndo( undoStackRef.current.length > 0 );
setHasRedo( redoStackRef.current.length > 0 );
}, [] );

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 ] );
Comment on lines +302 to +312
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.

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 ] );
Comment on lines +302 to +324
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.

// 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
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
}, [ onUndoReady, handleUndo, handleRedo, hasUndo, hasRedo ] ); // eslint-disable-line react-hooks/exhaustive-deps
}, [ onUndoReady, handleUndo, handleRedo, hasUndo, hasRedo ] );

Copilot uses AI. Check for mistakes.
Comment on lines +326 to +331
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.

// Keyboard shortcuts for undo/redo.
useEffect( () => {
function handleKeyDown( event ) {
// Don't override native undo in regular form fields (title, URL input, etc.).
const tagName = event.target.tagName.toLowerCase();
if (
tagName === 'input' ||
tagName === 'textarea' ||
tagName === 'select'
) {
return;
}

const isModKey = event.ctrlKey || event.metaKey;
if ( ! isModKey ) {
return;
}

const key = event.key.toLowerCase();

// Ctrl+Z / Cmd+Z = Undo, Ctrl+Shift+Z / Cmd+Shift+Z = Redo.
if ( key === 'z' ) {
event.preventDefault();
if ( event.shiftKey ) {
handleRedo();
} else {
handleUndo();
}
}

// Ctrl+Y = Redo (Windows/Linux convention). Do not use Cmd+Y on macOS.
if (
key === 'y' &&
! event.shiftKey &&
event.ctrlKey &&
! event.metaKey
) {
event.preventDefault();
handleRedo();
}
}

document.addEventListener( 'keydown', handleKeyDown );
return () => document.removeEventListener( 'keydown', handleKeyDown );
}, [ handleUndo, handleRedo ] );

// Parse initial content.
useEffect( () => {
if ( post.content ) {
Comment on lines 379 to 381
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
Expand All @@ -296,25 +396,57 @@ export default function PressThisEditor( {
setTitle( pendingScrape.title );
}

// Parse and append the scraped content blocks.
// Parse and append the scraped content blocks (with undo level).
if ( pendingScrape.content ) {
const newBlocks = parse( pendingScrape.content );
undoStackRef.current = [
...undoStackRef.current,
blocksRef.current,
];
redoStackRef.current = [];
setBlocks( ( prevBlocks ) => [ ...prevBlocks, ...newBlocks ] );
Comment on lines +402 to 407
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
undoStackRef.current = [
...undoStackRef.current,
blocksRef.current,
];
redoStackRef.current = [];
setBlocks( ( prevBlocks ) => [ ...prevBlocks, ...newBlocks ] );
setBlocks( ( prevBlocks ) => {
undoStackRef.current = [
...undoStackRef.current,
prevBlocks,
];
redoStackRef.current = [];
return [ ...prevBlocks, ...newBlocks ];
} );

Copilot uses AI. Check for mistakes.
syncUndoRedoState();
}

// Notify parent that we've processed the scrape.
onScrapeProcessed();
}, [ pendingScrape, onScrapeProcessed ] ); // eslint-disable-line react-hooks/exhaustive-deps
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
}, [ pendingScrape, onScrapeProcessed ] ); // eslint-disable-line react-hooks/exhaustive-deps
}, [ pendingScrape, onScrapeProcessed, title ] ); // eslint-disable-line react-hooks/exhaustive-deps

Copilot uses AI. Check for mistakes.

/**
* Handle block changes.
* Handle non-persistent block changes (e.g. typing).
* Updates blocks without creating an undo level.
*
* @param {Array} newBlocks Updated blocks.
*/
const handleBlocksChange = useCallback( ( newBlocks ) => {
const handleBlocksInput = useCallback( ( newBlocks ) => {
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
const handleBlocksInput = useCallback( ( newBlocks ) => {
const handleBlocksInput = useCallback( ( newBlocks ) => {
// Avoid conflicting updates during undo/redo operations.
if ( isUndoingRef.current ) {
isUndoingRef.current = false;
setBlocks( newBlocks );
return;
}

Copilot uses AI. Check for mistakes.
setBlocks( newBlocks );
}, [] );

/**
* Handle persistent block changes (e.g. paste, block operations).
* Creates an undo level before applying the change.
*
* @param {Array} newBlocks Updated blocks.
*/
const handleBlocksChange = useCallback(
( newBlocks ) => {
// Don't create undo levels for undo/redo operations.
if ( isUndoingRef.current ) {
isUndoingRef.current = false;
setBlocks( newBlocks );
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
setBlocks( newBlocks );

Copilot uses AI. Check for mistakes.
return;
}
undoStackRef.current = [
...undoStackRef.current,
blocksRef.current,
];
redoStackRef.current = [];
setBlocks( newBlocks );
syncUndoRedoState();
},
[ syncUndoRedoState ]
);

/**
* Insert a block into the editor.
*
Expand Down Expand Up @@ -639,7 +771,7 @@ export default function PressThisEditor( {
<div className="press-this-editor">
<BlockEditorProvider
value={ blocks }
onInput={ handleBlocksChange }
onInput={ handleBlocksInput }
onChange={ handleBlocksChange }
settings={ editorSettings }
>
Expand Down