Skip to content

Commit

Permalink
List view: Modify the shortcut to focus while open (#45135)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
alexstine committed Feb 14, 2023
1 parent 79103f1 commit 21f8cdb
Show file tree
Hide file tree
Showing 9 changed files with 191 additions and 6 deletions.
31 changes: 31 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions packages/block-editor/src/components/list-view/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
<AsyncModeProvider value={ true }>
<ListViewDropIndicator
Expand Down
4 changes: 4 additions & 0 deletions packages/e2e-test-utils/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,10 @@ _Returns_

- `Promise`: Promise resolving with a boolean indicating if the focused block is the default block.

### isListViewOpen

Undocumented declaration.

### isOfflineMode

Undocumented declaration.
Expand Down
2 changes: 1 addition & 1 deletion packages/e2e-test-utils/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ export {
rest as __experimentalRest,
batch as __experimentalBatch,
} from './rest-api';
export { openListView, closeListView } from './list-view';
export { isListViewOpen, openListView, closeListView } from './list-view';
export {
disableSiteEditorWelcomeGuide,
getCurrentSiteEditorContent,
Expand Down
2 changes: 1 addition & 1 deletion packages/e2e-test-utils/src/list-view.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ async function toggleListView() {
);
}

async function isListViewOpen() {
export async function isListViewOpen() {
return await page.evaluate( () => {
// 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(
Expand Down
84 changes: 84 additions & 0 deletions packages/e2e-tests/specs/editor/various/list-view.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
createNewPost,
insertBlock,
getEditedPostContent,
isListViewOpen,
openListView,
pressKeyWithModifier,
pressKeyTimes,
Expand Down Expand Up @@ -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();
} );
} );
1 change: 1 addition & 0 deletions packages/edit-post/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
9 changes: 6 additions & 3 deletions packages/edit-post/src/components/keyboard-shortcuts/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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();
Expand All @@ -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
<div
aria-label={ __( 'Document Overview' ) }
className="edit-post-editor__document-overview-panel"
onKeyDown={ closeOnEscape }
ref={ sidebarRef }
>
<div
className="edit-post-editor__document-overview-panel-header components-panel__header edit-post-sidebar__panel-tabs"
Expand All @@ -59,6 +113,7 @@ export default function ListViewSidebar() {
<ul>
<li>
<Button
ref={ listViewTabRef }
onClick={ () => {
setTab( 'list-view' );
} }
Expand All @@ -73,6 +128,7 @@ export default function ListViewSidebar() {
</li>
<li>
<Button
ref={ outlineTabRef }
onClick={ () => {
setTab( 'outline' );
} }
Expand All @@ -91,6 +147,7 @@ export default function ListViewSidebar() {
ref={ useMergeRefs( [
contentFocusReturnRef,
focusOnMountRef,
listViewRef,
] ) }
className="edit-post-editor__list-view-container"
>
Expand Down

1 comment on commit 21f8cdb

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flaky tests detected in 21f8cdb.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4170225931
📝 Reported issues:

Please sign in to comment.