Editor: Fix block highlight rendering after block is moved (#23425)

* Editor: Fix block outline rendering after block is moved

* Fix rendering for tests. Reduce Timeout from 250 -> 200 to improve UX.

* Fix rAF call with toggling highlight

* Resolve the E2E test issue, which appears to be caused by a double-mounting Toolbar

* Use reducer

Co-authored-by: Ella van Durpe <ella@vandurpe.com>

packages/block-editor/src/components/block-toolbar/index.js

@@ -6,7 +6,7 @@ import classnames from 'classnames';
 /**
  * WordPress dependencies
  */
-import { useSelect } from '@wordpress/data';
+import { useSelect, useDispatch } from '@wordpress/data';
 import { useRef } from '@wordpress/element';
 import { useViewportMatch } from '@wordpress/compose';
 import { getBlockType, hasBlockSupport } from '@wordpress/blocks';
@@ -21,7 +21,7 @@ import BlockControls from '../block-controls';
 import BlockFormatControls from '../block-format-controls';
 import BlockSettingsMenu from '../block-settings-menu';
 import BlockDraggable from '../block-draggable';
-import { useShowMoversGestures, useToggleBlockHighlight } from './utils';
+import { useShowMoversGestures } from './utils';
 
 export default function BlockToolbar( { hideDragHandle } ) {
 	const {
@@ -61,13 +61,15 @@ export default function BlockToolbar( { hideDragHandle } ) {
 		};
 	}, [] );
 
-	const toggleBlockHighlight = useToggleBlockHighlight( blockClientId );
+	const { toggleBlockHighlight } = useDispatch( 'core/block-editor' );
 	const nodeRef = useRef();
 
 	const { showMovers, gestures: showMoversGestures } = useShowMoversGestures(
 		{
 			ref: nodeRef,
-			onChange: toggleBlockHighlight,
+			onChange( isFocused ) {
+				toggleBlockHighlight( blockClientId, isFocused );
+			},
 		}
 	);
 

packages/block-editor/src/components/block-toolbar/utils.js

@@ -2,19 +2,15 @@
  * External dependencies
  */
 import { noop } from 'lodash';
+
 /**
  * WordPress dependencies
  */
-import { useDispatch } from '@wordpress/data';
-import { useState, useRef, useEffect, useCallback } from '@wordpress/element';
+import { useState, useRef, useEffect } from '@wordpress/element';
+
+const { clearTimeout, setTimeout } = window;
 
-const {
-	clearTimeout,
-	requestAnimationFrame,
-	cancelAnimationFrame,
-	setTimeout,
-} = window;
-const DEBOUNCE_TIMEOUT = 250;
+const DEBOUNCE_TIMEOUT = 200;
 
 /**
  * Hook that creates a showMover state, as well as debounced show/hide callbacks.
@@ -168,41 +164,3 @@ export function useShowMoversGestures( {
 		},
 	};
 }
-
-let requestAnimationFrameId;
-
-/**
- * Hook that toggles the highlight (outline) state of a block
- *
- * @param {string} clientId The block's clientId
- *
- * @return {Function} Callback function to toggle highlight state.
- */
-export function useToggleBlockHighlight( clientId ) {
-	const { toggleBlockHighlight } = useDispatch( 'core/block-editor' );
-
-	const updateBlockHighlight = useCallback(
-		( isFocused ) => {
-			toggleBlockHighlight( clientId, isFocused );
-		},
-		[ clientId ]
-	);
-
-	useEffect( () => {
-		// On mount, we make sure to cancel any pending animation frame request
-		// that hasn't been completed yet. Components like NavigableToolbar may
-		// mount and unmount quickly.
-		if ( requestAnimationFrameId ) {
-			cancelAnimationFrame( requestAnimationFrameId );
-		}
-		return () => {
-			// Sequences state change to enable editor updates (e.g. cursor
-			// position) to render correctly.
-			requestAnimationFrameId = requestAnimationFrame( () => {
-				updateBlockHighlight( false );
-			} );
-		};
-	}, [] );
-
-	return updateBlockHighlight;
-}

packages/block-editor/src/components/navigable-toolbar/index.js

@@ -35,11 +35,27 @@ function focusFirstTabbableIn( container ) {
 }
 
 function useIsAccessibleToolbar( ref ) {
+	/*
+	 * By default, we'll assume the starting accessible state of the Toolbar
+	 * is true, as it seems to be the most common case.
+	 *
+	 * Transitioning from an (initial) false to true state causes the
+	 * <Toolbar /> component to mount twice, which is causing undesired
+	 * side-effects. These side-effects appear to only affect certain
+	 * E2E tests.
+	 *
+	 * This was initial discovered in this pull-request:
+	 * https://github.com/WordPress/gutenberg/pull/23425
+	 */
+	const initialAccessibleToolbarState = true;
+
 	// By default, it's gonna render NavigableMenu. If all the tabbable elements
 	// inside the toolbar are ToolbarItem components (or derived components like
 	// ToolbarButton), then we can wrap them with the accessible Toolbar
 	// component.
-	const [ isAccessibleToolbar, setIsAccessibleToolbar ] = useState( false );
+	const [ isAccessibleToolbar, setIsAccessibleToolbar ] = useState(
+		initialAccessibleToolbarState
+	);
 
 	const determineIsAccessibleToolbar = useCallback( () => {
 		const tabbables = focus.tabbable.find( ref.current );

packages/block-editor/src/store/reducer.js

@@ -1630,14 +1630,21 @@ export function automaticChangeStatus( state, action ) {
  * @return {string} Updated state.
  */
 export function highlightedBlock( state, action ) {
-	const { clientId, isHighlighted } = action;
+	switch ( action.type ) {
+		case 'TOGGLE_BLOCK_HIGHLIGHT':
+			const { clientId, isHighlighted } = action;
 
-	if ( action.type === 'TOGGLE_BLOCK_HIGHLIGHT' ) {
-		if ( isHighlighted ) {
-			return clientId;
-		} else if ( state === clientId ) {
-			return null;
-		}
+			if ( isHighlighted ) {
+				return clientId;
+			} else if ( state === clientId ) {
+				return null;
+			}
+
+			return state;
+		case 'SELECT_BLOCK':
+			if ( action.clientId !== state ) {
+				return null;
+			}
 	}
 
 	return state;