From 3f2cf83b3d7fa99545ff6631712b21e65d825d4d Mon Sep 17 00:00:00 2001 From: "Matthew Riley MacPherson (tofumatt)" Date: Fri, 2 Nov 2018 17:18:52 +0000 Subject: [PATCH 1/2] chore: Improve undo/redo no-op See: https://github.com/WordPress/gutenberg/pull/11379\#discussion_r230406108 --- .../editor/src/components/editor-history/redo.js | 16 ++++++---------- .../editor/src/components/editor-history/undo.js | 16 ++++++---------- 2 files changed, 12 insertions(+), 20 deletions(-) diff --git a/packages/editor/src/components/editor-history/redo.js b/packages/editor/src/components/editor-history/redo.js index 33cd412d2adf42..9a79d7057617c3 100644 --- a/packages/editor/src/components/editor-history/redo.js +++ b/packages/editor/src/components/editor-history/redo.js @@ -13,8 +13,11 @@ function EditorHistoryRedo( { hasRedo, redo } ) { icon="redo" label={ __( 'Redo' ) } shortcut={ displayShortcut.primaryShift( 'z' ) } + // If there are no redo levels we don't want to actually disable this + // button, because it will remove focus for keyboard users. + // See: https://github.com/WordPress/gutenberg/issues/3486 aria-disabled={ ! hasRedo } - onClick={ redo } + onClick={ hasRedo ? redo : undefined } className="editor-history__redo" /> ); @@ -24,14 +27,7 @@ export default compose( [ withSelect( ( select ) => ( { hasRedo: select( 'core/editor' ).hasEditorRedo(), } ) ), - withDispatch( ( dispatch, ownProps ) => ( { - redo: () => { - // If there are no redo levels this is a no-op, because we don't actually - // disable the button. - // See: https://github.com/WordPress/gutenberg/issues/3486 - if ( ownProps.hasRedo ) { - dispatch( 'core/editor' ).redo(); - } - }, + withDispatch( ( dispatch ) => ( { + redo: () => dispatch( 'core/editor' ).redo(), } ) ), ] )( EditorHistoryRedo ); diff --git a/packages/editor/src/components/editor-history/undo.js b/packages/editor/src/components/editor-history/undo.js index 008654dd434009..85010fd79f992d 100644 --- a/packages/editor/src/components/editor-history/undo.js +++ b/packages/editor/src/components/editor-history/undo.js @@ -13,8 +13,11 @@ function EditorHistoryUndo( { hasUndo, undo } ) { icon="undo" label={ __( 'Undo' ) } shortcut={ displayShortcut.primary( 'z' ) } + // If there are no undo levels we don't want to actually disable this + // button, because it will remove focus for keyboard users. + // See: https://github.com/WordPress/gutenberg/issues/3486 aria-disabled={ ! hasUndo } - onClick={ undo } + onClick={ hasUndo ? undo : undefined } className="editor-history__undo" /> ); @@ -24,14 +27,7 @@ export default compose( [ withSelect( ( select ) => ( { hasUndo: select( 'core/editor' ).hasEditorUndo(), } ) ), - withDispatch( ( dispatch, ownProps ) => ( { - undo: () => { - // If there are no undo levels this is a no-op, because we don't actually - // disable the button. - // See: https://github.com/WordPress/gutenberg/issues/3486 - if ( ownProps.hasUndo ) { - dispatch( 'core/editor' ).undo(); - } - }, + withDispatch( ( dispatch ) => ( { + undo: () => dispatch( 'core/editor' ).undo(), } ) ), ] )( EditorHistoryUndo ); From 0a70c82640c715d9e907f1f5a63ccc0b2cf41dfc Mon Sep 17 00:00:00 2001 From: "Matthew Riley MacPherson (tofumatt)" Date: Thu, 8 Nov 2018 03:21:39 +0000 Subject: [PATCH 2/2] Do not wrap dispatch undo/redo --- packages/editor/src/components/editor-history/redo.js | 2 +- packages/editor/src/components/editor-history/undo.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/editor/src/components/editor-history/redo.js b/packages/editor/src/components/editor-history/redo.js index 9a79d7057617c3..de6c3e46a26daf 100644 --- a/packages/editor/src/components/editor-history/redo.js +++ b/packages/editor/src/components/editor-history/redo.js @@ -28,6 +28,6 @@ export default compose( [ hasRedo: select( 'core/editor' ).hasEditorRedo(), } ) ), withDispatch( ( dispatch ) => ( { - redo: () => dispatch( 'core/editor' ).redo(), + redo: dispatch( 'core/editor' ).redo, } ) ), ] )( EditorHistoryRedo ); diff --git a/packages/editor/src/components/editor-history/undo.js b/packages/editor/src/components/editor-history/undo.js index 85010fd79f992d..6c3826cc7f1f4d 100644 --- a/packages/editor/src/components/editor-history/undo.js +++ b/packages/editor/src/components/editor-history/undo.js @@ -28,6 +28,6 @@ export default compose( [ hasUndo: select( 'core/editor' ).hasEditorUndo(), } ) ), withDispatch( ( dispatch ) => ( { - undo: () => dispatch( 'core/editor' ).undo(), + undo: dispatch( 'core/editor' ).undo, } ) ), ] )( EditorHistoryUndo );