Handle Escape on Toolbar Option 2: Use addEventListener on the select…

…edblock toolbar, attached to the NavigableToolbar This method requires forwarding a ref from the editor level down to the navigable toolbar so that the escape unselect shortcut can be blocked and the navigable toolbar event listener will still fire. Blocking the global escape event shouldn't be necessary, but we have a combination of a few things all combining to create a situation where: - Because the block toolbar uses createPortal to populate the block toolbar fills, we can't rely on the React event bubbling to hit the onKeyDown listener for the block toolbar - Since we can't use the React tree, we use the DOM tree which _should_ handle the event bubbling correctly from a `createPortal` element. - This bubbles via the React tree, which hits this `unselect` escape keypress before the block toolbar DOM event listener has access to it.
WordPress · Sep 6, 2023 · b5f5248 · b5f5248
1 parent 9acb93d
commit b5f5248
Show file tree

Hide file tree

Showing 4 changed files with 86 additions and 60 deletions.
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
@@ -7,7 +7,7 @@ import classnames from 'classnames';
  * WordPress dependencies
  */
 import { __ } from '@wordpress/i18n';
-import { useEffect, useRef, useState } from '@wordpress/element';
+import { forwardRef, useEffect, useRef, useState } from '@wordpress/element';
 import { hasBlockSupport, store as blocksStore } from '@wordpress/blocks';
 import { useSelect } from '@wordpress/data';
 import {
@@ -27,7 +27,10 @@ import BlockToolbar from '../block-toolbar';
 import { store as blockEditorStore } from '../../store';
 import { useHasAnyBlockControls } from '../block-controls/use-has-block-controls';
 
-function BlockContextualToolbar( { focusOnMount, isFixed, ...props } ) {
+function UnforwardBlockContextualToolbar(
+	{ focusOnMount, isFixed, ...props },
+	ref
+) {
 	// When the toolbar is fixed it can be collapsed
 	const [ isCollapsed, setIsCollapsed ] = useState( false );
 	const toolbarButtonRef = useRef();
@@ -175,11 +178,12 @@ function BlockContextualToolbar( { focusOnMount, isFixed, ...props } ) {
 
 	return (
 		<NavigableToolbar
+			ref={ ref }
 			focusOnMount={ focusOnMount }
 			className={ classes }
 			/* translators: accessibility text for the block toolbar */
 			aria-label={ __( 'Block tools' ) }
-			onChildrenKeyDown={ ( event ) => {
+			handleOnKeyDown={ ( event ) => {
 				if ( event.keyCode === ESCAPE && lastFocus?.current ) {
 					event.preventDefault();
 					lastFocus.current.focus();
@@ -216,4 +220,4 @@ function BlockContextualToolbar( { focusOnMount, isFixed, ...props } ) {
 	);
 }
 
-export default BlockContextualToolbar;
+export default forwardRef( UnforwardBlockContextualToolbar );
diff --git a/packages/block-editor/src/components/block-tools/index.js b/packages/block-editor/src/components/block-tools/index.js
@@ -62,6 +62,8 @@ export default function BlockTools( {
 		moveBlocksDown,
 	} = useDispatch( blockEditorStore );
 
+	const selectedBlockToolsRef = useRef( null );
+
 	function onKeyDown( event ) {
 		if ( event.defaultPrevented ) return;
 
@@ -104,6 +106,15 @@ export default function BlockTools( {
 				insertBeforeBlock( clientIds[ 0 ] );
 			}
 		} else if ( isMatch( 'core/block-editor/unselect', event ) ) {
+			if ( selectedBlockToolsRef.current.contains( event.target ) ) {
+				// This shouldn't be necessary, but we have a combination of a few things all combining to create a situation where:
+				// - Because the block toolbar uses createPortal to populate the block toolbar fills, we can't rely on the React event bubbling to hit the onKeyDown listener for the block toolbar
+				// - Since we can't use the React tree, we use the DOM tree which _should_ handle the event bubbling correctly from a `createPortal` element.
+				// - This bubbles via the React tree, which hits this `unselect` escape keypress before the block toolbar DOM event listener has access to it.
+				// An alternative would be to remove the addEventListener on the navigableToolbar and use this event to handle it directly right here. That feels hacky too though.
+				return;
+			}
+
 			const clientIds = getSelectedBlockClientIds();
 			if ( clientIds.length ) {
 				event.preventDefault();
@@ -140,7 +151,10 @@ export default function BlockTools( {
 					__unstableContentRef={ __unstableContentRef }
 				/>
 				<Fill name="__experimentalSelectedBlockTools">
-					<SelectedBlockTools isFixed={ hasFixedToolbar } />
+					<SelectedBlockTools
+						ref={ selectedBlockToolsRef }
+						isFixed={ hasFixedToolbar }
+					/>
 				</Fill>
 				{ /* Used for the inline rich text toolbar. */ }
 				<Popover.Slot name="block-toolbar" ref={ blockToolbarRef } />

diff --git a/packages/block-editor/src/components/block-tools/selected-block-tools.js b/packages/block-editor/src/components/block-tools/selected-block-tools.js
@@ -6,7 +6,7 @@ import classnames from 'classnames';
 /**
  * WordPress dependencies
  */
-import { useRef, useEffect } from '@wordpress/element';
+import { forwardRef, useRef, useEffect } from '@wordpress/element';
 import { isUnmodifiedDefaultBlock } from '@wordpress/blocks';
 import { useDispatch, useSelect } from '@wordpress/data';
 import { useShortcut } from '@wordpress/keyboard-shortcuts';
@@ -41,13 +41,10 @@ function selector( select ) {
 	};
 }
 
-function SelectedBlockTools( {
-	clientId,
-	rootClientId,
-	isEmptyDefaultBlock,
-	isFixed,
-	capturingClientId,
-} ) {
+function WrappedSelectedBlockTools(
+	{ clientId, rootClientId, isEmptyDefaultBlock, isFixed, capturingClientId },
+	ref
+) {
 	const { editorMode, hasMultiSelection, isTyping, lastClientId } = useSelect(
 		selector,
 		[]
@@ -118,6 +115,7 @@ function SelectedBlockTools( {
 	if ( isFixed || ! isLargeViewport ) {
 		return (
 			<BlockContextualToolbar
+				ref={ ref }
 				isFixed={ true }
 				__experimentalInitialIndex={
 					initialToolbarItemIndexRef.current
@@ -152,6 +150,7 @@ function SelectedBlockTools( {
 			>
 				{ shouldShowContextualToolbar && (
 					<BlockContextualToolbar
+						ref={ ref }
 						// If the toolbar is being shown because of being forced
 						// it should focus the toolbar right after the mount.
 						focusOnMount={ isToolbarForced.current }
@@ -179,6 +178,8 @@ function SelectedBlockTools( {
 	return null;
 }
 
+const SelectedBlockTools = forwardRef( WrappedSelectedBlockTools );
+
 function wrapperSelector( select ) {
 	const {
 		getSelectedBlockClientId,
@@ -221,7 +222,8 @@ function wrapperSelector( select ) {
 	};
 }
 
-export default function WrappedSelectedBlockTools( { isFixed } ) {
+const UnforwardWrappedSelectedBlockTools = ( props, ref ) => {
+	const { isFixed } = props;
 	const selected = useSelect( wrapperSelector, [] );
 
 	if ( ! selected ) {
@@ -242,11 +244,14 @@ export default function WrappedSelectedBlockTools( { isFixed } ) {
 
 	return (
 		<SelectedBlockTools
+			ref={ ref }
 			clientId={ clientId }
 			rootClientId={ rootClientId }
 			isEmptyDefaultBlock={ isEmptyDefaultBlock }
 			isFixed={ isFixed }
 			capturingClientId={ capturingClientId }
 		/>
 	);
-}
+};
+
+export default forwardRef( UnforwardWrappedSelectedBlockTools );
diff --git a/packages/block-editor/src/components/navigable-toolbar/index.js b/packages/block-editor/src/components/navigable-toolbar/index.js
@@ -3,6 +3,7 @@
  */
 import { NavigableMenu, Toolbar } from '@wordpress/components';
 import {
+	forwardRef,
 	useState,
 	useRef,
 	useLayoutEffect,
@@ -38,7 +39,7 @@ function focusFirstTabbableIn( container ) {
 	}
 }
 
-function useIsAccessibleToolbar( ref ) {
+function useIsAccessibleToolbar( toolbarRef ) {
 	/*
 	 * By default, we'll assume the starting accessible state of the Toolbar
 	 * is true, as it seems to be the most common case.
@@ -62,7 +63,7 @@ function useIsAccessibleToolbar( ref ) {
 	);
 
 	const determineIsAccessibleToolbar = useCallback( () => {
-		const tabbables = focus.tabbable.find( ref.current );
+		const tabbables = focus.tabbable.find( toolbarRef.current );
 		const onlyToolbarItem = hasOnlyToolbarItem( tabbables );
 		if ( ! onlyToolbarItem ) {
 			deprecated( 'Using custom components as toolbar controls', {
@@ -73,23 +74,26 @@ function useIsAccessibleToolbar( ref ) {
 			} );
 		}
 		setIsAccessibleToolbar( onlyToolbarItem );
-	}, [] );
+	}, [ toolbarRef ] );
 
 	useLayoutEffect( () => {
 		// Toolbar buttons may be rendered asynchronously, so we use
 		// MutationObserver to check if the toolbar subtree has been modified.
 		const observer = new window.MutationObserver(
 			determineIsAccessibleToolbar
 		);
-		observer.observe( ref.current, { childList: true, subtree: true } );
+		observer.observe( toolbarRef.current, {
+			childList: true,
+			subtree: true,
+		} );
 		return () => observer.disconnect();
-	}, [ isAccessibleToolbar ] );
+	}, [ isAccessibleToolbar, determineIsAccessibleToolbar, toolbarRef ] );
 
 	return isAccessibleToolbar;
 }
 
 function useToolbarFocus(
-	ref,
+	toolbarRef,
 	focusOnMount,
 	isAccessibleToolbar,
 	defaultIndex,
@@ -101,8 +105,8 @@ function useToolbarFocus(
 	const [ initialIndex ] = useState( defaultIndex );
 
 	const focusToolbar = useCallback( () => {
-		focusFirstTabbableIn( ref.current );
-	}, [] );
+		focusFirstTabbableIn( toolbarRef.current );
+	}, [ toolbarRef ] );
 
 	const focusToolbarViaShortcut = () => {
 		if ( shouldUseKeyboardFocusShortcut ) {
@@ -121,7 +125,7 @@ function useToolbarFocus(
 
 	useEffect( () => {
 		// Store ref so we have access on useEffect cleanup: https://legacy.reactjs.org/blog/2020/08/10/react-v17-rc.html#effect-cleanup-timing
-		const navigableToolbarRef = ref.current;
+		const navigableToolbarRef = toolbarRef.current;
 		// If initialIndex is passed, we focus on that toolbar item when the
 		// toolbar gets mounted and initial focus is not forced.
 		// We have to wait for the next browser paint because block controls aren't
@@ -150,22 +154,27 @@ function useToolbarFocus(
 			const index = items.findIndex( ( item ) => item.tabIndex === 0 );
 			onIndexChange( index );
 		};
-	}, [ initialIndex, initialFocusOnMount ] );
+	}, [ initialIndex, initialFocusOnMount, toolbarRef, onIndexChange ] );
 }
 
-function NavigableToolbar( {
-	children,
-	focusOnMount,
-	onChildrenKeyDown = () => {},
-	shouldUseKeyboardFocusShortcut = true,
-	__experimentalInitialIndex: initialIndex,
-	__experimentalOnIndexChange: onIndexChange,
-	...props
-} ) {
-	const ref = useRef();
-	const isAccessibleToolbar = useIsAccessibleToolbar( ref );
+function UnforwardNavigableToolbar(
+	{
+		children,
+		focusOnMount,
+		handleOnKeyDown,
+		shouldUseKeyboardFocusShortcut = true,
+		__experimentalInitialIndex: initialIndex,
+		__experimentalOnIndexChange: onIndexChange,
+		...props
+	},
+	ref
+) {
+	const maybeRef = useRef();
+	// If a ref was not forwarded, we create one.
+	const toolbarRef = ref || maybeRef;
+	const isAccessibleToolbar = useIsAccessibleToolbar( toolbarRef );
 	useToolbarFocus(
-		ref,
+		toolbarRef,
 		focusOnMount,
 		isAccessibleToolbar,
 		initialIndex,
@@ -174,37 +183,31 @@ function NavigableToolbar( {
 	);
 
 	useEffect( () => {
-		const navigableToolbarRef = ref.current;
-		const toolbarButtons = navigableToolbarRef.querySelectorAll(
-			'[data-toolbar-item="true"]'
-		);
+		const navigableToolbarRef = toolbarRef.current;
 
-		if ( onChildrenKeyDown ) {
-			const handleChildrenKeyDown = ( event ) => {
-				onChildrenKeyDown( event );
+		if ( handleOnKeyDown ) {
+			const handleKeyDown = ( event ) => {
+				handleOnKeyDown( event );
 			};
 
-			toolbarButtons.forEach( ( toolbarButton ) => {
-				toolbarButton.addEventListener(
-					'keydown',
-					handleChildrenKeyDown
-				);
-			} );
+			navigableToolbarRef.addEventListener( 'keydown', handleKeyDown );
 
 			return () => {
-				toolbarButtons.forEach( ( toolbarButton ) => {
-					toolbarButton.removeEventListener(
-						'keydown',
-						handleChildrenKeyDown
-					);
-				} );
+				navigableToolbarRef.removeEventListener(
+					'keydown',
+					handleKeyDown
+				);
 			};
 		}
-	}, [ onChildrenKeyDown, children ] );
+	}, [ handleOnKeyDown, toolbarRef ] );
 
 	if ( isAccessibleToolbar ) {
 		return (
-			<Toolbar label={ props[ 'aria-label' ] } ref={ ref } { ...props }>
+			<Toolbar
+				label={ props[ 'aria-label' ] }
+				ref={ toolbarRef }
+				{ ...props }
+			>
 				{ children }
 			</Toolbar>
 		);
@@ -214,12 +217,12 @@ function NavigableToolbar( {
 		<NavigableMenu
 			orientation="horizontal"
 			role="toolbar"
-			ref={ ref }
+			ref={ toolbarRef }
 			{ ...props }
 		>
 			{ children }
 		</NavigableMenu>
 	);
 }
 
-export default NavigableToolbar;
+export default forwardRef( UnforwardNavigableToolbar );