From 7aa97522f18646c202299574e1e72480028d9c74 Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Tue, 3 Oct 2023 11:35:09 -0500 Subject: [PATCH] Pass keydown listener into navigable toolbar instead of removing bubblesVirtually from the toolbar group slot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removing bubblesVirtually from the toolbar group slot prevents any dynamically added toolbar buttons from firing their click event. This can be seen when cropping an image, you press Crop, then the Apply and Cancel buttons get added to the toolbar. Those button events don’t fire properly and is mentioned as an issue in the bubblesVirtually slot fill PR: https://github.com/WordPress/gutenberg/pull/19242 --- .../src/components/block-controls/slot.js | 2 +- .../block-tools/block-contextual-toolbar.js | 80 +++++++++++-------- .../src/components/navigable-toolbar/index.js | 30 +++++++ 3 files changed, 78 insertions(+), 34 deletions(-) diff --git a/packages/block-editor/src/components/block-controls/slot.js b/packages/block-editor/src/components/block-controls/slot.js index fb2ace0ba17a10..ad800b49ab40db 100644 --- a/packages/block-editor/src/components/block-controls/slot.js +++ b/packages/block-editor/src/components/block-controls/slot.js @@ -42,7 +42,7 @@ export default function BlockControlsSlot( { group = 'default', ...props } ) { return null; } - const slot = ; + const slot = ; if ( group === 'default' ) { return slot; diff --git a/packages/block-editor/src/components/block-tools/block-contextual-toolbar.js b/packages/block-editor/src/components/block-tools/block-contextual-toolbar.js index 68a52a2e65910b..5a16b77f254a44 100644 --- a/packages/block-editor/src/components/block-tools/block-contextual-toolbar.js +++ b/packages/block-editor/src/components/block-tools/block-contextual-toolbar.js @@ -9,6 +9,7 @@ import classnames from 'classnames'; import { __ } from '@wordpress/i18n'; import { hasBlockSupport, store as blocksStore } from '@wordpress/blocks'; import { useSelect } from '@wordpress/data'; +import { ESCAPE } from '@wordpress/keycodes'; /** * Internal dependencies @@ -19,40 +20,47 @@ import { store as blockEditorStore } from '../../store'; import { useHasAnyBlockControls } from '../block-controls/use-has-block-controls'; function BlockContextualToolbar( { focusOnMount, isFixed, ...props } ) { - const { blockType, blockEditingMode, hasParents, showParentSelector } = - useSelect( ( select ) => { - const { - getBlockName, - getBlockParents, - getSelectedBlockClientIds, - getBlockEditingMode, - } = select( blockEditorStore ); - const { getBlockType } = select( blocksStore ); - const selectedBlockClientIds = getSelectedBlockClientIds(); - const _selectedBlockClientId = selectedBlockClientIds[ 0 ]; - const parents = getBlockParents( _selectedBlockClientId ); - const firstParentClientId = parents[ parents.length - 1 ]; - const parentBlockName = getBlockName( firstParentClientId ); - const parentBlockType = getBlockType( parentBlockName ); + const { + blockType, + blockEditingMode, + lastFocus, + hasParents, + showParentSelector, + } = useSelect( ( select ) => { + const { + getBlockName, + getBlockParents, + getLastFocus, + getSelectedBlockClientIds, + getBlockEditingMode, + } = select( blockEditorStore ); + const { getBlockType } = select( blocksStore ); + const selectedBlockClientIds = getSelectedBlockClientIds(); + const _selectedBlockClientId = selectedBlockClientIds[ 0 ]; + const parents = getBlockParents( _selectedBlockClientId ); + const firstParentClientId = parents[ parents.length - 1 ]; + const parentBlockName = getBlockName( firstParentClientId ); + const parentBlockType = getBlockType( parentBlockName ); - return { - blockType: - _selectedBlockClientId && - getBlockType( getBlockName( _selectedBlockClientId ) ), - blockEditingMode: getBlockEditingMode( _selectedBlockClientId ), - hasParents: parents.length, - showParentSelector: - parentBlockType && - getBlockEditingMode( firstParentClientId ) === 'default' && - hasBlockSupport( - parentBlockType, - '__experimentalParentSelector', - true - ) && - selectedBlockClientIds.length <= 1 && - getBlockEditingMode( _selectedBlockClientId ) === 'default', - }; - }, [] ); + return { + blockType: + _selectedBlockClientId && + getBlockType( getBlockName( _selectedBlockClientId ) ), + blockEditingMode: getBlockEditingMode( _selectedBlockClientId ), + lastFocus: getLastFocus(), + hasParents: parents.length, + showParentSelector: + parentBlockType && + getBlockEditingMode( firstParentClientId ) === 'default' && + hasBlockSupport( + parentBlockType, + '__experimentalParentSelector', + true + ) && + selectedBlockClientIds.length <= 1 && + getBlockEditingMode( _selectedBlockClientId ) === 'default', + }; + }, [] ); const isToolbarEnabled = ! blockType || @@ -77,6 +85,12 @@ function BlockContextualToolbar( { focusOnMount, isFixed, ...props } ) { className={ classes } /* translators: accessibility text for the block toolbar */ aria-label={ __( 'Block tools' ) } + onChildrenKeyDown={ ( event ) => { + if ( event.keyCode === ESCAPE && lastFocus?.current ) { + event.preventDefault(); + lastFocus.current.focus(); + } + } } { ...props } > diff --git a/packages/block-editor/src/components/navigable-toolbar/index.js b/packages/block-editor/src/components/navigable-toolbar/index.js index d26476a866b7aa..f5826e2797bc06 100644 --- a/packages/block-editor/src/components/navigable-toolbar/index.js +++ b/packages/block-editor/src/components/navigable-toolbar/index.js @@ -200,6 +200,7 @@ function NavigableToolbar( { shouldUseKeyboardFocusShortcut = true, __experimentalInitialIndex: initialIndex, __experimentalOnIndexChange: onIndexChange, + onChildrenKeyDown, ...props } ) { const ref = useRef(); @@ -214,6 +215,35 @@ function NavigableToolbar( { shouldUseKeyboardFocusShortcut ); + useEffect( () => { + const navigableToolbarRef = ref.current; + const toolbarButtons = navigableToolbarRef.querySelectorAll( + '[data-toolbar-item="true"]' + ); + + if ( onChildrenKeyDown ) { + const handleChildrenKeyDown = ( event ) => { + onChildrenKeyDown( event ); + }; + + toolbarButtons.forEach( ( toolbarButton ) => { + toolbarButton.addEventListener( + 'keydown', + handleChildrenKeyDown + ); + } ); + + return () => { + toolbarButtons.forEach( ( toolbarButton ) => { + toolbarButton.removeEventListener( + 'keydown', + handleChildrenKeyDown + ); + } ); + }; + } + }, [ onChildrenKeyDown, children ] ); + if ( isAccessibleToolbar ) { return (