Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Focus submenu button when clicked #55198

Merged
merged 12 commits into from
Oct 18, 2023
7 changes: 7 additions & 0 deletions packages/block-library/src/navigation/index.php
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
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
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,51 @@
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 @@
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: 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: 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 @@
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 all the menus with click on a non-body element outside the menu
await page.getByText( 'Just another Gutenberg site' ).click();

Check failure on line 336 in test/e2e/specs/editor/blocks/navigation-frontend-interactivity.spec.js

View workflow job for this annotation

GitHub Actions / Playwright - 1

[chromium] › editor/blocks/navigation-frontend-interactivity.spec.js:260:3 › Navigation block - Frontend interactivity › Submenu mouse and keyboard interactions › Submenu interactions on Safari (@webkit)

1) [chromium] › editor/blocks/navigation-frontend-interactivity.spec.js:260:3 › Navigation block - Frontend interactivity › Submenu mouse and keyboard interactions › Submenu interactions on Safari (@WebKit) TimeoutError: locator.click: Timeout 10000ms exceeded. =========================== logs =========================== waiting for getByText('Just another Gutenberg site') ============================================================ 334 | 335 | // Close all the menus with click on a non-body element outside the menu > 336 | await page.getByText( 'Just another Gutenberg site' ).click(); | ^ 337 | await expect( firstLevelElement ).toBeHidden(); 338 | 339 | // Tests not covered: Tabbing to close menus at /home/runner/work/gutenberg/gutenberg/test/e2e/specs/editor/blocks/navigation-frontend-interactivity.spec.js:336:58

Check failure on line 336 in test/e2e/specs/editor/blocks/navigation-frontend-interactivity.spec.js

View workflow job for this annotation

GitHub Actions / Playwright - 1

[chromium] › editor/blocks/navigation-frontend-interactivity.spec.js:260:3 › Navigation block - Frontend interactivity › Submenu mouse and keyboard interactions › Submenu interactions on Safari (@webkit)

1) [chromium] › editor/blocks/navigation-frontend-interactivity.spec.js:260:3 › Navigation block - Frontend interactivity › Submenu mouse and keyboard interactions › Submenu interactions on Safari (@WebKit) Retry #1 ─────────────────────────────────────────────────────────────────────────────────────── TimeoutError: locator.click: Timeout 10000ms exceeded. =========================== logs =========================== waiting for getByText('Just another Gutenberg site') ============================================================ 334 | 335 | // Close all the menus with click on a non-body element outside the menu > 336 | await page.getByText( 'Just another Gutenberg site' ).click(); | ^ 337 | await expect( firstLevelElement ).toBeHidden(); 338 | 339 | // Tests not covered: Tabbing to close menus at /home/runner/work/gutenberg/gutenberg/test/e2e/specs/editor/blocks/navigation-frontend-interactivity.spec.js:336:58

Check failure on line 336 in test/e2e/specs/editor/blocks/navigation-frontend-interactivity.spec.js

View workflow job for this annotation

GitHub Actions / Playwright - 1

[chromium] › editor/blocks/navigation-frontend-interactivity.spec.js:260:3 › Navigation block - Frontend interactivity › Submenu mouse and keyboard interactions › Submenu interactions on Safari (@webkit)

1) [chromium] › editor/blocks/navigation-frontend-interactivity.spec.js:260:3 › Navigation block - Frontend interactivity › Submenu mouse and keyboard interactions › Submenu interactions on Safari (@WebKit) Retry #2 ─────────────────────────────────────────────────────────────────────────────────────── TimeoutError: locator.click: Timeout 10000ms exceeded. =========================== logs =========================== waiting for getByText('Just another Gutenberg site') ============================================================ 334 | 335 | // Close all the menus with click on a non-body element outside the menu > 336 | await page.getByText( 'Just another Gutenberg site' ).click(); | ^ 337 | await expect( firstLevelElement ).toBeHidden(); 338 | 339 | // Tests not covered: Tabbing to close menus at /home/runner/work/gutenberg/gutenberg/test/e2e/specs/editor/blocks/navigation-frontend-interactivity.spec.js:336:58

Check failure on line 336 in test/e2e/specs/editor/blocks/navigation-frontend-interactivity.spec.js

View workflow job for this annotation

GitHub Actions / Playwright - 4

[webkit] › editor/blocks/navigation-frontend-interactivity.spec.js:260:3 › Navigation block - Frontend interactivity › Submenu mouse and keyboard interactions › Submenu interactions on Safari (@webkit)

1) [webkit] › editor/blocks/navigation-frontend-interactivity.spec.js:260:3 › Navigation block - Frontend interactivity › Submenu mouse and keyboard interactions › Submenu interactions on Safari (@WebKit) TimeoutError: locator.click: Timeout 10000ms exceeded. =========================== logs =========================== waiting for getByText('Just another Gutenberg site') ============================================================ 334 | 335 | // Close all the menus with click on a non-body element outside the menu > 336 | await page.getByText( 'Just another Gutenberg site' ).click(); | ^ 337 | await expect( firstLevelElement ).toBeHidden(); 338 | 339 | // Tests not covered: Tabbing to close menus at /home/runner/work/gutenberg/gutenberg/test/e2e/specs/editor/blocks/navigation-frontend-interactivity.spec.js:336:58

Check failure on line 336 in test/e2e/specs/editor/blocks/navigation-frontend-interactivity.spec.js

View workflow job for this annotation

GitHub Actions / Playwright - 4

[webkit] › editor/blocks/navigation-frontend-interactivity.spec.js:260:3 › Navigation block - Frontend interactivity › Submenu mouse and keyboard interactions › Submenu interactions on Safari (@webkit)

1) [webkit] › editor/blocks/navigation-frontend-interactivity.spec.js:260:3 › Navigation block - Frontend interactivity › Submenu mouse and keyboard interactions › Submenu interactions on Safari (@WebKit) Retry #1 ─────────────────────────────────────────────────────────────────────────────────────── TimeoutError: locator.click: Timeout 10000ms exceeded. =========================== logs =========================== waiting for getByText('Just another Gutenberg site') ============================================================ 334 | 335 | // Close all the menus with click on a non-body element outside the menu > 336 | await page.getByText( 'Just another Gutenberg site' ).click(); | ^ 337 | await expect( firstLevelElement ).toBeHidden(); 338 | 339 | // Tests not covered: Tabbing to close menus at /home/runner/work/gutenberg/gutenberg/test/e2e/specs/editor/blocks/navigation-frontend-interactivity.spec.js:336:58

Check failure on line 336 in test/e2e/specs/editor/blocks/navigation-frontend-interactivity.spec.js

View workflow job for this annotation

GitHub Actions / Playwright - 4

[webkit] › editor/blocks/navigation-frontend-interactivity.spec.js:260:3 › Navigation block - Frontend interactivity › Submenu mouse and keyboard interactions › Submenu interactions on Safari (@webkit)

1) [webkit] › editor/blocks/navigation-frontend-interactivity.spec.js:260:3 › Navigation block - Frontend interactivity › Submenu mouse and keyboard interactions › Submenu interactions on Safari (@WebKit) Retry #2 ─────────────────────────────────────────────────────────────────────────────────────── TimeoutError: locator.click: Timeout 10000ms exceeded. =========================== logs =========================== waiting for getByText('Just another Gutenberg site') ============================================================ 334 | 335 | // Close all the menus with click on a non-body element outside the menu > 336 | await page.getByText( 'Just another Gutenberg site' ).click(); | ^ 337 | await expect( firstLevelElement ).toBeHidden(); 338 | 339 | // Tests not covered: Tabbing to close menus at /home/runner/work/gutenberg/gutenberg/test/e2e/specs/editor/blocks/navigation-frontend-interactivity.spec.js:336:58
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 @@
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 @@

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
Loading