Fix the List View focus management

packages/block-editor/src/components/list-view/block-select-button.js

@@ -44,6 +44,7 @@ function ListViewBlockSelectButton(
 		draggable,
 		isExpanded,
 		ariaDescribedBy,
+		isSelected,
 	},
 	ref
 ) {
@@ -102,6 +103,7 @@ function ListViewBlockSelectButton(
 			href={ `#block-${ clientId }` }
 			aria-describedby={ ariaDescribedBy }
 			aria-expanded={ isExpanded }
+			data-is-selected={ isSelected ? true : undefined }
 		>
 			<ListViewExpander onClick={ onToggleExpanded } />
 			<BlockIcon

packages/block-editor/src/components/list-view/index.js

@@ -179,12 +179,27 @@ function ListViewComponent(
 		ref,
 	] );
 
+	const timerIdRef = useRef();
+
 	useEffect( () => {
-		// If a blocks are already selected when the list view is initially
+		// If any blocks are already selected when the list view is initially
 		// mounted, shift focus to the first selected block.
-		if ( selectedClientIds?.length ) {
-			focusListItem( selectedClientIds[ 0 ], elementRef?.current );
-		}
+		// The ListView may render within other components that already manage
+		// initial focus via `useFocusOnMount` e.g. the `ListViewSidebar`. As
+		// `useFocusOnMount` uses a timeout internally, it runs last and may steal
+		// focus from the selected item. We use another timeout to make ListView
+		// set its own initial focus last.
+		timerIdRef.current = setTimeout( () => {
+			if ( selectedClientIds?.length ) {
+				focusListItem( selectedClientIds[ 0 ], elementRef?.current );
+			}
+		}, 0 );
+
+		return () => {
+			if ( timerIdRef.current ) {
+				clearTimeout( timerIdRef.current );
+			}
+		};
 		// Only focus on the selected item when the list view is mounted.
 	}, [] );
 

packages/components/src/tree-grid/README.md

@@ -12,6 +12,8 @@ A tree grid is a hierarchical 2 dimensional UI component, for example it could b
 
 A tree grid allows the user to navigate using arrow keys. Up/down to navigate vertically across rows, and left/right to navigate horizontally between focusables in a row.
 
+To make the keyboard navigation and roving tabindex behaviors work as expected it is important to avoid programmatically setting focus on any of the focusable items in the tree grid. In fact, `RovingTabIndexItem` handles the logic to make only one item navigable with the Tab key at a time. The other items can be navigated with the arrow keys. Triggering a focus event may conflict with the `RovingTabIndexItem` internal logic.
+
 For more information on a tree grid, see the following links:
 
 -   https://www.w3.org/TR/wai-aria-practices/examples/treegrid/treegrid-1.html

packages/editor/src/components/global-keyboard-shortcuts/index.js

@@ -91,9 +91,15 @@ export default function EditorKeyboardShortcuts() {
 		savePost();
 	} );
 
-	// Only opens the list view. Other functionality for this shortcut happens in the rendered sidebar.
+	// Only opens the list view. Other functionality for this shortcut happens
+	// in the rendered sidebar. When the `showListViewByDefault` preference is
+	// enabled, the sidebar is rendered by default. As such, we need to prevent
+	// the callback from running twice by using an additional check for
+	// `event.defaultPrevented` otherwise the shortcut:
+	// 1. It will first be invoked in the sidebar, thus closing it.
+	// 2. It will then run again here, reopening the sidebar unexpectedly.
 	useShortcut( 'core/editor/toggle-list-view', ( event ) => {
-		if ( ! isListViewOpened() ) {
+		if ( ! isListViewOpened() && ! event.defaultPrevented ) {
 			event.preventDefault();
 			setIsListViewOpened( true );
 		}

packages/editor/src/components/list-view-sidebar/index.js

@@ -8,10 +8,11 @@ import {
 import { useFocusOnMount, useMergeRefs } from '@wordpress/compose';
 import { useDispatch, useSelect } from '@wordpress/data';
 import { focus } from '@wordpress/dom';
-import { useCallback, useRef, useState } from '@wordpress/element';
+import { useCallback, useRef, useState, useEffect } from '@wordpress/element';
 import { __, _x } from '@wordpress/i18n';
 import { useShortcut } from '@wordpress/keyboard-shortcuts';
 import { ESCAPE } from '@wordpress/keycodes';
+import { store as preferencesStore } from '@wordpress/preferences';
 
 /**
  * Internal dependencies
@@ -22,29 +23,63 @@ import { store as editorStore } from '../../store';
 
 const { TabbedSidebar } = unlock( blockEditorPrivateApis );
 
+// Used to count how many times the component renders and determine the initial focus logic.
+let renderCounter = 0;
+
 export default function ListViewSidebar() {
 	const { setIsListViewOpened } = useDispatch( editorStore );
-	const { getListViewToggleRef } = unlock( useSelect( editorStore ) );
+
+	const { listViewToggleRef, showListViewByDefault } = useSelect(
+		( select ) => {
+			const { getListViewToggleRef } = unlock( select( editorStore ) );
+			const _showListViewByDefault = select( preferencesStore ).get(
+				'core',
+				'showListViewByDefault'
+			);
+
+			return {
+				listViewToggleRef: getListViewToggleRef(),
+				showListViewByDefault: _showListViewByDefault,
+			};
+		},
+		[]
+	);
 
 	// This hook handles focus when the sidebar first renders.
 	const focusOnMountRef = useFocusOnMount( 'firstElement' );
 
 	// When closing the list view, focus should return to the toggle button.
 	const closeListView = useCallback( () => {
 		setIsListViewOpened( false );
-		getListViewToggleRef().current?.focus();
-	}, [ getListViewToggleRef, setIsListViewOpened ] );
+		listViewToggleRef.current?.focus();
+	}, [ listViewToggleRef, setIsListViewOpened ] );
 
 	const closeOnEscape = useCallback(
 		( event ) => {
 			if ( event.keyCode === ESCAPE && ! event.defaultPrevented ) {
+				// Always use `event.preventDefault` before calling `closeListView`.
+				// This is important to prevent the `core/editor/toggle-list-view`
+				// shortcut callback from being twice.
 				event.preventDefault();
 				closeListView();
 			}
 		},
 		[ closeListView ]
 	);
 
+	const firstRenderCheckRef = useRef( false );
+
+	useEffect( () => {
+		// This extra check avoids duplicate updates of the counter in development
+		// mode (React.StrictMode) or because of potential re-renders triggered
+		// by components higher up the tree.
+		if ( firstRenderCheckRef.current ) {
+			return;
+		}
+		renderCounter++;
+		firstRenderCheckRef.current = true;
+	}, [] );
+
 	// Use internal state instead of a ref to make sure that the component
 	// re-renders when the dropZoneElement updates.
 	const [ dropZoneElement, setDropZoneElement ] = useState( null );
@@ -53,18 +88,29 @@ export default function ListViewSidebar() {
 
 	// This ref refers to the sidebar as a whole.
 	const sidebarRef = useRef();
-	// This ref refers to the tab panel.
+	// This ref refers to the tablist.
 	const tabsRef = useRef();
 	// This ref refers to the list view application area.
 	const listViewRef = useRef();
 
 	// Must merge the refs together so focus can be handled properly in the next function.
 	const listViewContainerRef = useMergeRefs( [
-		focusOnMountRef,
 		listViewRef,
 		setDropZoneElement,
 	] );
 
+	// focusOnMountRef ref is used to set initial focus to the first tab in the
+	// ListViewSidebar while the tabsRef is used to manage focus for the ARIA tabs UI.
+	let tabsPanelRef = useMergeRefs( [ focusOnMountRef, tabsRef ] );
+
+	// When the 'Always open List View' preference is enabled and the ListViewSidebar
+	// renders for the first time on page load, initial focus should not be managed.
+	// Rather, the tab sequence should normally start from the document root. In
+	// this case, we only pass the tabsRef and omit the focusOnMountRef.
+	if ( showListViewByDefault && renderCounter === 1 ) {
+		tabsPanelRef = tabsRef;
+	}
+
 	/*
 	 * Callback function to handle list view or outline focus.
 	 *
@@ -73,43 +119,68 @@ export default function ListViewSidebar() {
 	 * @return void
 	 */
 	function handleSidebarFocus( currentTab ) {
-		// Tab panel focus.
-		const tabPanelFocus = focus.tabbable.find( tabsRef.current )[ 0 ];
+		// Active tab in the tablist.
+		const activeTab = focus.tabbable.find( tabsRef.current )[ 0 ];
 		// List view tab is selected.
 		if ( currentTab === 'list-view' ) {
-			// Either focus the list view or the tab panel. Must have a fallback because the list view does not render when there are no blocks.
-			const listViewApplicationFocus = focus.tabbable.find(
-				listViewRef.current
-			)[ 0 ];
-			const listViewFocusArea = sidebarRef.current.contains(
-				listViewApplicationFocus
+			// Either focus the list view selected item or the active tab in the
+			// tablist. Must have a fallback because the list view does not
+			// render when there are no blocks.
+			// Important: The `core/editor/toggle-list-view` keyboard shortcut
+			// callback runs when the `keydown` event fires. At that point the
+			// ListView hasn't received focus yet and its internal mechanism to
+			// handle the tabindex attribute hasn't run yet. As such, there may
+			// be an additional item that is 'tabbable' but it's not the
+			// selected item. Filtering based on the `data-is-selected` attribute
+			// attribute makes sure to target the selected item.
+			const listViewSelectedItem = focus.tabbable
+				.find( listViewRef.current )
+				.filter( ( item ) =>
+					item.hasAttribute( 'data-is-selected' )
+				)[ 0 ];
+			const listViewFocusTarget = sidebarRef.current.contains(
+				listViewSelectedItem
 			)
-				? listViewApplicationFocus
-				: tabPanelFocus;
-			listViewFocusArea.focus();
+				? listViewSelectedItem
+				: activeTab;
+
+			listViewFocusTarget.focus();
 			// Outline tab is selected.
 		} else {
-			tabPanelFocus.focus();
+			activeTab.focus();
 		}
 	}
 
-	const handleToggleListViewShortcut = useCallback( () => {
-		// If the sidebar has focus, it is safe to close.
-		if (
-			sidebarRef.current.contains(
-				sidebarRef.current.ownerDocument.activeElement
-			)
-		) {
-			closeListView();
-		} else {
-			// If the list view or outline does not have focus, focus should be moved to it.
-			handleSidebarFocus( tab );
-		}
-	}, [ closeListView, tab ] );
+	const handleToggleListViewShortcut = useCallback(
+		( event ) => {
+			// If the sidebar has focus, it is safe to close.
+			if (
+				sidebarRef.current.contains(
+					sidebarRef.current.ownerDocument.activeElement
+				)
+			) {
+				// Always use `event.preventDefault` before calling `closeListView`.
+				// This is important to prevent the `core/editor/toggle-list-view`
+				// shortcut callback from running twice.
+				event.preventDefault();
+				closeListView();
+			} else {
+				// If the list view or outline does not have focus, focus should be moved to it.
+				handleSidebarFocus( tab );
+			}
+		},
+		[ closeListView, tab ]
+	);
 
 	// This only fires when the sidebar is open because of the conditional rendering.
-	// It is the same shortcut to open but that is defined as a global shortcut and only fires when the sidebar is closed.
-	useShortcut( 'core/editor/toggle-list-view', handleToggleListViewShortcut );
+	// It is the same shortcut to open the sidebar but that is defined as a global
+	// shortcut. However, when the `showListViewByDefault` preference is enabled,
+	// the sidebar is open by default and the shortcut callback would be invoked
+	// twice (here and in the global shortcut). To prevent that, we pass the event
+	// for some additional logic in the global shortcut based on `event.defaultPrevented`.
+	useShortcut( 'core/editor/toggle-list-view', ( event ) => {
+		handleToggleListViewShortcut( event );
+	} );
 
 	return (
 		// eslint-disable-next-line jsx-a11y/no-static-element-interactions
@@ -147,7 +218,7 @@ export default function ListViewSidebar() {
 				onClose={ closeListView }
 				onSelect={ ( tabName ) => setTab( tabName ) }
 				defaultTabId="list-view"
-				ref={ tabsRef }
+				ref={ tabsPanelRef }
 				closeButtonLabel={ __( 'Close' ) }
 			/>
 		</div>

test/e2e/specs/editor/various/block-hierarchy-navigation.spec.js

@@ -240,6 +240,9 @@ test.describe( 'Navigating the block hierarchy', () => {
 				name: 'Block navigation structure',
 			} )
 		).toBeVisible();
+		// Move focus to the first item in the List view,
+		// which happens to be the Group block.
+		await page.keyboard.press( 'Tab' );
 		await page.keyboard.press( 'Enter' );
 
 		await expect(

test/e2e/specs/site-editor/list-view.spec.js

@@ -69,6 +69,9 @@ test.describe( 'Site Editor List View', () => {
 		} );
 		await expect( listView ).toBeVisible();
 
+		// Move focus to the first item in the List view,
+		// which happens to be the site title block.
+		await page.keyboard.press( 'Tab' );
 		// The site title block should have focus.
 		await expect(
 			listView.getByRole( 'link', {