Skip to content

Commit

Permalink
Focus submenu button when clicked (#55198)
Browse files Browse the repository at this point in the history 
* Focus element manually when open submenu on click

* Try using `tabindex="-1"`

* Use `tabindex="-1"` also in body when a submenu is opened

* Replace tabindex with event listener

* Explain the tabindex on <li>

* Don't store the element on hover to restore the focus later

* Improve explanations

* Add tests to cover webkit frontend menu interactions

Safari doesn't place focus on a clicked button as expected. These tests verify that when a submenu chevron button is clicked, focus is correctly placed on that button. It also verifies that the click on the body correctly closes any open submenus, which was failing in Safari.

* Focus clicked button on Safari

Combining the tabindex -1 on the parent li and focusing the button on Safari, and also checking that the relatedTarget is null inside the handleMenuFocusout seems to contain the focus within the menu to not fire the handleMenuFocusout as often, and still works to click on the body to close the menu.

* Added the document.addEventListener body click back in

Authored by Luis Herranz <luisherranz@gmail.com>. I'm just re-applying the change.

* Remove tab keypresses from webkit menu interaction tests

Tab keypreses on webkit playwright are really flakey (or it's something in our code that we haven't isolated) so I've split out the webkit tests to test everything I can without using a tab keypress.

* Use body click instead for consistency across environments

---------

Co-authored-by: Luis Herranz <luisherranz@gmail.com>
Co-authored-by: Jerry Jones <jones.jeremydavid@gmail.com>
  • Loading branch information
@SantosGuillamot @luisherranz @jeryj
3 people authored and Siobhan committed Oct 19, 2023
1 parent 9ef7560 commit 042dfed
Show file tree
Hide file tree
Showing 3 changed files with 203 additions and 12 deletions.
7 changes: 7 additions & 0 deletions packages/block-library/src/navigation/index.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,13 @@ function block_core_navigation_add_directives_to_submenu( $w, $block_attributes
$w->set_attribute( 'data-wp-effect', 'effects.core.navigation.initMenu' );
$w->set_attribute( 'data-wp-on--focusout', 'actions.core.navigation.handleMenuFocusout' );
$w->set_attribute( 'data-wp-on--keydown', 'actions.core.navigation.handleMenuKeydown' );

// This is a fix for Safari. Without it, Safari doesn't change the active
// element when the user clicks on a button. It can be removed once we add
// an overlay to capture the clicks, instead of relying on the focusout
// event.
$w->set_attribute( 'tabindex', '-1' );

if ( ! isset( $block_attributes['openSubmenusOnClick'] ) || false === $block_attributes['openSubmenusOnClick'] ) {
$w->set_attribute( 'data-wp-on--mouseenter', 'actions.core.navigation.openMenuOnHover' );
$w->set_attribute( 'data-wp-on--mouseleave', 'actions.core.navigation.closeMenuOnHover' );
Expand Down
24 changes: 18 additions & 6 deletions packages/block-library/src/navigation/view.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,14 @@ const focusableSelectors = [
'[tabindex]:not([tabindex^="-"])',
];

// This is a fix for Safari in iOS/iPadOS. Without it, Safari doesn't focus out
// when the user taps in the body. It can be removed once we add an overlay to
// capture the clicks, instead of relying on the focusout event.
document.addEventListener( 'click', () => {} );

const openMenu = ( store, menuOpenedOn ) => {
const { context, ref, selectors } = store;
const { context, selectors } = store;
selectors.core.navigation.menuOpenedBy( store )[ menuOpenedOn ] = true;
context.core.navigation.previousFocus = ref;
if ( context.core.navigation.type === 'overlay' ) {
// Add a `has-modal-open` class to the <html> root.
document.documentElement.classList.add( 'has-modal-open' );
Expand All @@ -33,7 +37,7 @@ const closeMenu = ( store, menuClosedOn ) => {
window.document.activeElement
)
) {
context.core.navigation.previousFocus.focus();
context.core.navigation.previousFocus?.focus();
}
context.core.navigation.modal = null;
context.core.navigation.previousFocus = null;
Expand Down Expand Up @@ -130,6 +134,8 @@ wpStore( {
closeMenu( store, 'hover' );
},
openMenuOnClick( store ) {
const { context, ref } = store;
context.core.navigation.previousFocus = ref;
openMenu( store, 'click' );
},
closeMenuOnClick( store ) {
Expand All @@ -140,13 +146,16 @@ wpStore( {
openMenu( store, 'focus' );
},
toggleMenuOnClick: ( store ) => {
const { selectors } = store;
const { selectors, context, ref } = store;
// Safari won't send focus to the clicked element, so we need to manually place it: https://bugs.webkit.org/show_bug.cgi?id=22261
if ( window.document.activeElement !== ref ) ref.focus();
const menuOpenedBy =
selectors.core.navigation.menuOpenedBy( store );
if ( menuOpenedBy.click || menuOpenedBy.focus ) {
closeMenu( store, 'click' );
closeMenu( store, 'focus' );
} else {
context.core.navigation.previousFocus = ref;
openMenu( store, 'click' );
}
},
Expand Down Expand Up @@ -194,11 +203,14 @@ wpStore( {
// event.relatedTarget === The element receiving focus (if any)
// When focusout is outsite the document,
// `window.document.activeElement` doesn't change.

// The event.relatedTarget is null when something outside the navigation menu is clicked. This is only necessary for Safari.
if (
! context.core.navigation.modal?.contains(
event.relatedTarget === null ||
( ! context.core.navigation.modal?.contains(
event.relatedTarget
) &&
event.target !== window.document.activeElement
event.target !== window.document.activeElement )
) {
closeMenu( store, 'click' );
closeMenu( store, 'focus' );
Expand Down
184 changes: 178 additions & 6 deletions test/e2e/specs/editor/blocks/navigation-frontend-interactivity.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,51 @@ test.describe( 'Navigation block - Frontend interactivity', () => {
await expect( overlayMenuFirstElement ).toBeHidden();
await expect( openMenuButton ).toBeFocused();
} );

/**
* These are already tested within the Overlay Interactions test above, but Safari is flakey on the Tab
* keypresses (passes 50 - 70% of the time). Tab keypresses are testing fine manually in Safari, but not
* in the test. nce we figure out why the Tab keypresses are flakey in the test, we can
* remove this test and only rely on the Overlay Interactions test above and add a (@firefox, @webkit)
* directive to the describe() statement. https://github.com/WordPress/gutenberg/pull/55198
*/
test( 'Overlay menu interactions in Safari (@webkit)', async ( {
page,
pageUtils,
} ) => {
await page.goto( '/' );
const overlayMenuFirstElement = page.getByRole( 'link', {
name: 'Item 1',
} );
const openMenuButton = page.getByRole( 'button', {
name: 'Open menu',
} );

const closeMenuButton = page.getByRole( 'button', {
name: 'Close menu',
} );

// Test: overlay menu opens on click on open menu button
await expect( overlayMenuFirstElement ).toBeHidden();
await openMenuButton.click();
await expect( overlayMenuFirstElement ).toBeVisible();

// Test: overlay menu focuses on first element after opening
await expect( overlayMenuFirstElement ).toBeFocused();

// Not Tested: overlay menu traps focus

// Test: overlay menu closes on click on close menu button
await closeMenuButton.click();
await expect( overlayMenuFirstElement ).toBeHidden();

// Test: overlay menu closes on ESC key
await openMenuButton.click();
await expect( overlayMenuFirstElement ).toBeVisible();
await pageUtils.pressKeys( 'Escape' );
await expect( overlayMenuFirstElement ).toBeHidden();
await expect( openMenuButton ).toBeFocused();
} );
} );

test.describe( 'Submenu mouse and keyboard interactions', () => {
Expand Down Expand Up @@ -133,10 +178,14 @@ test.describe( 'Navigation block - Frontend interactivity', () => {
const secondLevelElement = page.getByRole( 'link', {
name: 'Nested Submenu Link 1',
} );
const lastFirstLevelElement = page.getByRole( 'link', {
name: 'Complex Submenu Link 2',
} );

// Test: submenu opens on click
await expect( innerElement ).toBeHidden();
await simpleSubmenuButton.click();
await expect( simpleSubmenuButton ).toBeFocused();
await expect( innerElement ).toBeVisible();

// Test: submenu closes on click outside submenu
Expand All @@ -145,10 +194,12 @@ test.describe( 'Navigation block - Frontend interactivity', () => {

// Test: nested submenu opens on click
await complexSubmenuButton.click();
await expect( complexSubmenuButton ).toBeFocused();
await expect( firstLevelElement ).toBeVisible();
await expect( secondLevelElement ).toBeHidden();

await nestedSubmenuButton.click();
await expect( nestedSubmenuButton ).toBeFocused();
await expect( firstLevelElement ).toBeVisible();
await expect( secondLevelElement ).toBeVisible();

Expand All @@ -160,6 +211,7 @@ test.describe( 'Navigation block - Frontend interactivity', () => {
// Test: submenu opens on Enter keypress
await simpleSubmenuButton.focus();
await pageUtils.pressKeys( 'Enter' );
await expect( simpleSubmenuButton ).toBeFocused();
await expect( innerElement ).toBeVisible();

// Test: submenu closes on ESC key and focuses parent link
Expand All @@ -168,36 +220,127 @@ test.describe( 'Navigation block - Frontend interactivity', () => {
await expect( simpleSubmenuButton ).toBeFocused();

// Test: submenu closes on tab outside submenu
await simpleSubmenuButton.focus();
await pageUtils.pressKeys( 'Enter' );
await expect( simpleSubmenuButton ).toBeFocused();
await expect( innerElement ).toBeVisible();
// Tab to first element, then tab outside the submenu.
await pageUtils.pressKeys( 'Tab', { times: 2, delay: 50 } );
await expect( innerElement ).toBeHidden();
await expect( complexSubmenuButton ).toBeFocused();
await expect( innerElement ).toBeHidden();

// Test: only nested submenu closes on tab outside
await complexSubmenuButton.focus();
await pageUtils.pressKeys( 'Enter' );
await expect( complexSubmenuButton ).toBeFocused();
await expect( firstLevelElement ).toBeVisible();
await expect( secondLevelElement ).toBeHidden();

await nestedSubmenuButton.click();
await expect( nestedSubmenuButton ).toBeFocused();
await expect( firstLevelElement ).toBeVisible();
await expect( secondLevelElement ).toBeVisible();

// Tab to nested submenu first element, then tab outside the nested
// submenu.
await pageUtils.pressKeys( 'Tab', { times: 2, delay: 50 } );
await expect( lastFirstLevelElement ).toBeFocused();
await expect( firstLevelElement ).toBeVisible();
await expect( secondLevelElement ).toBeHidden();
// Tab outside the complex submenu.
await page.keyboard.press( 'Tab' );
await expect( firstLevelElement ).toBeHidden();
} );

/**
* These are already tested within the Submenu Interactions test above, but Safari is flakey on the
* Tab keypresses (passes 50 - 70% of the time). Tab keypresses are testing fine manually in Safari,
* but not in the test. Once we figure out why the Tab keypresses are flakey in the test, we can
* remove this test and only rely on the Submenu interactions test above and add a (@firefox, @webkit)
* directive to the describe() statement. https://github.com/WordPress/gutenberg/pull/55198
*/
test( 'Submenu interactions on Safari (@webkit)', async ( {
page,
pageUtils,
} ) => {
await page.goto( '/' );
const simpleSubmenuButton = page.getByRole( 'button', {
name: 'Simple Submenu',
} );
const innerElement = page.getByRole( 'link', {
name: 'Simple Submenu Link 1',
} );
const complexSubmenuButton = page.getByRole( 'button', {
name: 'Complex Submenu',
} );
const nestedSubmenuButton = page.getByRole( 'button', {
name: 'Nested Submenu',
} );
const firstLevelElement = page.getByRole( 'link', {
name: 'Complex Submenu Link 1',
} );
const secondLevelElement = page.getByRole( 'link', {
name: 'Nested Submenu Link 1',
} );

// Test: submenu opens on click and focuses the button
await expect( innerElement ).toBeHidden();
await simpleSubmenuButton.click();
await expect( simpleSubmenuButton ).toBeFocused();
await expect( innerElement ).toBeVisible();

// Test: a second click closes the submenu
await simpleSubmenuButton.click();
await expect( simpleSubmenuButton ).toBeFocused();
await expect( innerElement ).toBeHidden();

// Test: submenu opens on Enter keypress
await simpleSubmenuButton.focus();
await pageUtils.pressKeys( 'Enter' );
await expect( simpleSubmenuButton ).toBeFocused();
await expect( innerElement ).toBeVisible();

// Test: submenu closes on second Enter keypress
await pageUtils.pressKeys( 'Enter' );
await expect( innerElement ).toBeHidden();
await expect( simpleSubmenuButton ).toBeFocused();

// Test: inner submenu opens on click and focuses the button
await complexSubmenuButton.click();
await expect( complexSubmenuButton ).toBeFocused();
await expect( firstLevelElement ).toBeVisible();
await expect( secondLevelElement ).toBeHidden();
// Click the inner menu button and check it opens the third level menu
await nestedSubmenuButton.click();
await expect( nestedSubmenuButton ).toBeFocused();
await expect( firstLevelElement ).toBeVisible();
await expect( secondLevelElement ).toBeVisible();

// Click the inner menu button and check it closes the third level menu
await nestedSubmenuButton.click();
await expect( nestedSubmenuButton ).toBeFocused();
await expect( firstLevelElement ).toBeVisible();
await expect( secondLevelElement ).toBeHidden();

// Do the same with Enter keypresses: open the third level menu
await pageUtils.pressKeys( 'Enter' );
await expect( nestedSubmenuButton ).toBeFocused();
await expect( firstLevelElement ).toBeVisible();
await expect( secondLevelElement ).toBeVisible();

// Close the third level menu
await pageUtils.pressKeys( 'Enter' );
await expect( nestedSubmenuButton ).toBeFocused();
await expect( firstLevelElement ).toBeVisible();
await expect( secondLevelElement ).toBeHidden();

// Close the menu via click on the body
await page.click( 'body' );
await expect( firstLevelElement ).toBeHidden();

// Tests not covered: Tabbing to close menus
} );
} );

test.describe( 'Submenus (Arrow setting)', () => {
test.describe( 'Submenus (Arrow setting) (@firefox, @webkit)', () => {
test.beforeEach( async ( { admin, editor, requestUtils } ) => {
await admin.visitSiteEditor( {
postId: 'emptytheme//header',
Expand All @@ -222,7 +365,7 @@ test.describe( 'Navigation block - Frontend interactivity', () => {
await editor.saveSiteEditorEntities();
} );

test( 'submenu opens on click in the arrow', async ( { page } ) => {
test( 'submenu click on the arrow interactions', async ( { page } ) => {
await page.goto( '/' );
const arrowButton = page.getByRole( 'button', {
name: 'Submenu submenu',
Expand All @@ -239,19 +382,48 @@ test.describe( 'Navigation block - Frontend interactivity', () => {

await expect( firstLevelElement ).toBeHidden();
await expect( secondLevelElement ).toBeHidden();
// Open first submenu level
await arrowButton.click();
await expect( arrowButton ).toBeFocused();
await expect( firstLevelElement ).toBeVisible();
await expect( secondLevelElement ).toBeHidden();

// Close first submenu level, check that it closes and focus is on the arrow button
await arrowButton.click();
await expect( arrowButton ).toBeFocused();
// Move the mouse so the hover on the button doesn't keep the menu open
await page.mouse.move( 400, 400 );
await expect( firstLevelElement ).toBeHidden();
await expect( secondLevelElement ).toBeHidden();

// Open first submenu level one more time so we can test the nested submenu
await arrowButton.click();
await expect( arrowButton ).toBeFocused();
await expect( firstLevelElement ).toBeVisible();
await expect( secondLevelElement ).toBeHidden();

// Nested submenu open
await nestedSubmenuArrowButton.click();
await expect( nestedSubmenuArrowButton ).toBeFocused();
await expect( firstLevelElement ).toBeVisible();
await expect( secondLevelElement ).toBeVisible();

// Nested submenu close
await nestedSubmenuArrowButton.click();
await expect( nestedSubmenuArrowButton ).toBeFocused();
// Move the mouse so the hover on the button doesn't keep the menu open
await page.mouse.move( 400, 400 );
await expect( firstLevelElement ).toBeVisible();
await expect( secondLevelElement ).toBeHidden();

// Close menu via click on the body
await page.click( 'body' );
await expect( firstLevelElement ).toBeHidden();
await expect( secondLevelElement ).toBeHidden();
} );
} );

test.describe( 'Page list block', () => {
test.describe( 'Page list block (@firefox, @webkit)', () => {
test.beforeEach( async ( { admin, editor, requestUtils } ) => {
const parentPage = await requestUtils.createPage( {
title: 'Parent Page',
Expand Down

0 comments on commit 042dfed

Please sign in to comment.