From 21f8cdbe17af1786f42506739e0233ca6fc628a9 Mon Sep 17 00:00:00 2001 From: Alex Stine Date: Mon, 13 Feb 2023 21:38:50 -0600 Subject: [PATCH] List view: Modify the shortcut to focus while open (#45135) * Modify shortcut to focus list view when it is open. * Remove extra state and figure out activeElement from ref. * Try a sneaky way of making shortcut work even on close button focus. * Fix static checks. * Modify selector. * Export isListViewOpen function. * Add E2E test for new functionality. * Fix focus events now that list view sidebar contains outline tab. * Update E2E. * Update E2E. * Do not render ListView if there are no blocks. * Fix focus for no blocks. * Fix focus on mount. * Move functionality to callback function entirely. * Fix E2E. * Remove merge artifact. * Fix tests. * Prevent scroll on focus. * Rework code. --- package-lock.json | 31 +++++++ .../src/components/list-view/index.js | 5 ++ packages/e2e-test-utils/README.md | 4 + packages/e2e-test-utils/src/index.js | 2 +- packages/e2e-test-utils/src/list-view.js | 2 +- .../specs/editor/various/list-view.test.js | 84 +++++++++++++++++++ packages/edit-post/package.json | 1 + .../components/keyboard-shortcuts/index.js | 9 +- .../secondary-sidebar/list-view-sidebar.js | 59 ++++++++++++- 9 files changed, 191 insertions(+), 6 deletions(-) diff --git a/package-lock.json b/package-lock.json index 1749ed01a7cc2..58914db037710 100644 --- a/package-lock.json +++ b/package-lock.json @@ -17723,6 +17723,7 @@ "@wordpress/core-data": "file:packages/core-data", "@wordpress/data": "file:packages/data", "@wordpress/deprecated": "file:packages/deprecated", + "@wordpress/dom": "^3.20.0", "@wordpress/editor": "file:packages/editor", "@wordpress/element": "file:packages/element", "@wordpress/hooks": "file:packages/hooks", @@ -17744,6 +17745,36 @@ "lodash": "^4.17.21", "memize": "^1.1.0", "rememo": "^4.0.0" + }, + "dependencies": { + "@wordpress/dom": { + "version": "3.20.0", + "resolved": "https://registry.npmjs.org/@wordpress/dom/-/dom-3.20.0.tgz", + "integrity": "sha512-Q35qCW8jj/JXTTujcC0wDDaLNNdzivkWkvCQj9FTyb+SoT8ZMwcwipnNvhyC0qprPlEREdQtNODmlSm4Ur4YkA==", + "requires": { + "@babel/runtime": "^7.16.0", + "@wordpress/deprecated": "^3.20.0" + }, + "dependencies": { + "@wordpress/deprecated": { + "version": "3.20.0", + "resolved": "https://registry.npmjs.org/@wordpress/deprecated/-/deprecated-3.20.0.tgz", + "integrity": "sha512-JruZLx74uP9uI5qi7uTANMXwLBNtGHvw/pYCtWBaisFTUutGm1fF1tGWFtIgTy5j1SjHtN0PMkTFlvdpJ+HASQ==", + "requires": { + "@babel/runtime": "^7.16.0", + "@wordpress/hooks": "^3.20.0" + } + }, + "@wordpress/hooks": { + "version": "3.20.0", + "resolved": "https://registry.npmjs.org/@wordpress/hooks/-/hooks-3.20.0.tgz", + "integrity": "sha512-OMOJwmbubrKueXhXEyBNU8CXBycawmtXCWbhqgYYbihgecB7cSZ1kAAPz+Oi/5j+3+XDfSlZXgWM1lCwvfnzPQ==", + "requires": { + "@babel/runtime": "^7.16.0" + } + } + } + } } }, "@wordpress/edit-site": { diff --git a/packages/block-editor/src/components/list-view/index.js b/packages/block-editor/src/components/list-view/index.js index 7166b594c276c..5425290809a18 100644 --- a/packages/block-editor/src/components/list-view/index.js +++ b/packages/block-editor/src/components/list-view/index.js @@ -174,6 +174,11 @@ function ListView( [ isMounted.current, draggedClientIds, expandedState, expand, collapse ] ); + // If there are no blocks to show, do not render the list view. + if ( ! clientIdsTree.length ) { + return null; + } + return ( { // selector .edit-post-header-toolbar__list-view-toggle is still required because the performance tests also execute against older versions that still use that selector. return !! document.querySelector( diff --git a/packages/e2e-tests/specs/editor/various/list-view.test.js b/packages/e2e-tests/specs/editor/various/list-view.test.js index dfb3caf1d8aa4..ade8bcf2359a5 100644 --- a/packages/e2e-tests/specs/editor/various/list-view.test.js +++ b/packages/e2e-tests/specs/editor/various/list-view.test.js @@ -5,6 +5,7 @@ import { createNewPost, insertBlock, getEditedPostContent, + isListViewOpen, openListView, pressKeyWithModifier, pressKeyTimes, @@ -327,4 +328,87 @@ describe( 'List view', () => { ); await expect( listViewGroupBlockRight ).toHaveFocus(); } ); + + async function getActiveElementLabel() { + return page.evaluate( + () => + document.activeElement.getAttribute( 'aria-label' ) || + document.activeElement.textContent + ); + } + + // If list view sidebar is open and focus is not inside the sidebar, move focus to the sidebar when using the shortcut. If focus is inside the sidebar, shortcut should close the sidebar. + it( 'ensures the list view global shortcut works properly', async () => { + // Insert some blocks of different types. + await insertBlock( 'Image' ); + await insertBlock( 'Paragraph' ); + await page.keyboard.type( 'Paragraph text.' ); + + // Open list view sidebar. + await pressKeyWithModifier( 'access', 'o' ); + + // Navigate to the image block. + await page.keyboard.press( 'ArrowUp' ); + // Check if image block link in the list view has focus by XPath selector. + const listViewImageBlock = await page.waitForXPath( + '//a[contains(., "Image")]' + ); + await expect( listViewImageBlock ).toHaveFocus(); + // Select the image block in the list view to move focus to it in the canvas. + await page.keyboard.press( 'Enter' ); + + // Check if image block upload button has focus by XPath selector. + const imageBlockUploadButton = await page.waitForXPath( + '//button[contains(text(), "Upload")]' + ); + await expect( imageBlockUploadButton ).toHaveFocus(); + + // Since focus is now at the image block upload button in the canvas, pressing the list view shortcut should bring focus back to the image block in the list view. + await pressKeyWithModifier( 'access', 'o' ); + await expect( listViewImageBlock ).toHaveFocus(); + + // Since focus is now inside the list view, the shortcut should close the sidebar. + await pressKeyWithModifier( 'access', 'o' ); + // Focus should now be on the paragraph block since that is where we opened the list view sidebar. This is not a perfect solution, but current functionality prevents a better way at the moment. Get the current block aria-label and compare. + await expect( await getActiveElementLabel() ).toEqual( + 'Paragraph block' + ); + // List view sidebar should be closed. + await expect( await isListViewOpen() ).toBeFalsy(); + + // Open list view sidebar. + await pressKeyWithModifier( 'access', 'o' ); + + // Focus the list view close button and make sure the shortcut will close the list view. This is to catch a bug where elements could be out of range of the sidebar region. Must shift+tab 3 times to reach cclose button before tabs. + await pressKeyWithModifier( 'shift', 'Tab' ); + await pressKeyWithModifier( 'shift', 'Tab' ); + await pressKeyWithModifier( 'shift', 'Tab' ); + await expect( await getActiveElementLabel() ).toEqual( + 'Close Document Overview Sidebar' + ); + + // Close the list view sidebar. + await pressKeyWithModifier( 'access', 'o' ); + // List view sidebar should be closed. + await expect( await isListViewOpen() ).toBeFalsy(); + + // Open list view sidebar. + await pressKeyWithModifier( 'access', 'o' ); + + // Focus the outline tab and select it. This test ensures the outline tab receives similar focus events based on the shortcut. + await pressKeyWithModifier( 'shift', 'Tab' ); + await expect( await getActiveElementLabel() ).toEqual( 'Outline' ); + await page.keyboard.press( 'Enter' ); + + // From here, tab in to the editor so focus can be checked on return to the outline tab in the sidebar. + await pressKeyTimes( 'Tab', 2 ); + // Focus should be placed on the outline tab button since there is nothing to focus inside the tab itself. + await pressKeyWithModifier( 'access', 'o' ); + await expect( await getActiveElementLabel() ).toEqual( 'Outline' ); + + // Close the list view sidebar. + await pressKeyWithModifier( 'access', 'o' ); + // List view sidebar should be closed. + await expect( await isListViewOpen() ).toBeFalsy(); + } ); } ); diff --git a/packages/edit-post/package.json b/packages/edit-post/package.json index aaec62928573e..b5045c96bfcf0 100644 --- a/packages/edit-post/package.json +++ b/packages/edit-post/package.json @@ -37,6 +37,7 @@ "@wordpress/core-data": "file:../core-data", "@wordpress/data": "file:../data", "@wordpress/deprecated": "file:../deprecated", + "@wordpress/dom": "^3.20.0", "@wordpress/editor": "file:../editor", "@wordpress/element": "file:../element", "@wordpress/hooks": "file:../hooks", diff --git a/packages/edit-post/src/components/keyboard-shortcuts/index.js b/packages/edit-post/src/components/keyboard-shortcuts/index.js index d3d6569e398a4..f3a8c15addb5b 100644 --- a/packages/edit-post/src/components/keyboard-shortcuts/index.js +++ b/packages/edit-post/src/components/keyboard-shortcuts/index.js @@ -250,9 +250,12 @@ function KeyboardShortcuts() { } } ); - useShortcut( 'core/edit-post/toggle-list-view', () => - setIsListViewOpened( ! isListViewOpened() ) - ); + // Only opens the list view. Other functionality for this shortcut happens in the rendered sidebar. + useShortcut( 'core/edit-post/toggle-list-view', () => { + if ( ! isListViewOpened() ) { + setIsListViewOpened( true ); + } + } ); useShortcut( 'core/block-editor/transform-heading-to-paragraph', diff --git a/packages/edit-post/src/components/secondary-sidebar/list-view-sidebar.js b/packages/edit-post/src/components/secondary-sidebar/list-view-sidebar.js index 447fcbd1a70db..21c3885f590a7 100644 --- a/packages/edit-post/src/components/secondary-sidebar/list-view-sidebar.js +++ b/packages/edit-post/src/components/secondary-sidebar/list-view-sidebar.js @@ -14,10 +14,12 @@ import { useMergeRefs, } from '@wordpress/compose'; import { useDispatch } from '@wordpress/data'; +import { focus } from '@wordpress/dom'; +import { useRef, useState } from '@wordpress/element'; import { __ } from '@wordpress/i18n'; import { closeSmall } from '@wordpress/icons'; +import { useShortcut } from '@wordpress/keyboard-shortcuts'; import { ESCAPE } from '@wordpress/keycodes'; -import { useState } from '@wordpress/element'; /** * Internal dependencies @@ -31,6 +33,7 @@ export default function ListViewSidebar() { const focusOnMountRef = useFocusOnMount( 'firstElement' ); const headerFocusReturnRef = useFocusReturn(); const contentFocusReturnRef = useFocusReturn(); + function closeOnEscape( event ) { if ( event.keyCode === ESCAPE && ! event.defaultPrevented ) { event.preventDefault(); @@ -40,12 +43,63 @@ export default function ListViewSidebar() { const [ tab, setTab ] = useState( 'list-view' ); + // This ref refers to the sidebar as a whole. + const sidebarRef = useRef(); + // This ref refers to the list view tab button. + const listViewTabRef = useRef(); + // This ref refers to the outline tab button. + const outlineTabRef = useRef(); + // This ref refers to the list view application area. + const listViewRef = useRef(); + + /* + * Callback function to handle list view or outline focus. + * + * @param {string} currentTab The current tab. Either list view or outline. + * + * @return void + */ + function handleSidebarFocus( currentTab ) { + // List view tab is selected. + if ( currentTab === 'list-view' ) { + // Either focus the list view or the list view tab button. 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 + ) + ? listViewApplicationFocus + : listViewTabRef.current; + listViewFocusArea.focus(); + // Outline tab is selected. + } else { + outlineTabRef.current.focus(); + } + } + + // 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/edit-post/toggle-list-view', () => { + // If the sidebar has focus, it is safe to close. + if ( + sidebarRef.current.contains( + sidebarRef.current.ownerDocument.activeElement + ) + ) { + setIsListViewOpened( false ); + // If the list view or outline does not have focus, focus should be moved to it. + } else { + handleSidebarFocus( tab ); + } + } ); + return ( // eslint-disable-next-line jsx-a11y/no-static-element-interactions