Pass keydown listener into navigable toolbar instead of removing bubb…

…lesVirtually from the toolbar group slot 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: #19242
WordPress · Oct 4, 2023 · 7aa9752 · 7aa9752
1 parent 300bc60
commit 7aa9752
Show file tree

Hide file tree

Showing 3 changed files with 78 additions and 34 deletions.
diff --git 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 = <Slot { ...props } fillProps={ fillProps } />;
+	const slot = <Slot { ...props } bubblesVirtually fillProps={ fillProps } />;
 
 	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
@@ -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 }
 		>
 			<BlockToolbar hideDragHandle={ isFixed } />

diff --git 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 (
 			<Toolbar label={ props[ 'aria-label' ] } ref={ ref } { ...props }>