From 57f0a131b47e784aee29805e6bdf3ab88d59b9c3 Mon Sep 17 00:00:00 2001 From: Tetsuaki Hamano Date: Sat, 23 Dec 2023 20:06:23 +0900 Subject: [PATCH 1/3] Navigation Block: Use dom.focus for focus control --- .../class-wp-navigation-block-renderer.php | 17 ++++++++++------- packages/block-library/src/navigation/view.js | 19 ++++--------------- 2 files changed, 14 insertions(+), 22 deletions(-) diff --git a/lib/compat/wordpress-6.5/class-wp-navigation-block-renderer.php b/lib/compat/wordpress-6.5/class-wp-navigation-block-renderer.php index 189e5a695c23a..ea94128e1dde2 100644 --- a/lib/compat/wordpress-6.5/class-wp-navigation-block-renderer.php +++ b/lib/compat/wordpress-6.5/class-wp-navigation-block-renderer.php @@ -428,11 +428,11 @@ private static function get_responsive_container_markup( $attributes, $inner_blo $responsive_dialog_directives = ''; $close_button_directives = ''; if ( $should_load_view_script ) { - $open_button_directives = ' + $open_button_directives = ' data-wp-on--click="actions.openMenuOnClick" data-wp-on--keydown="actions.handleMenuKeydown" '; - $responsive_container_directives = ' + $responsive_container_directives = ' data-wp-class--has-modal-open="state.isMenuOpen" data-wp-class--is-menu-open="state.isMenuOpen" data-wp-watch="callbacks.initMenu" @@ -440,15 +440,17 @@ private static function get_responsive_container_markup( $attributes, $inner_blo data-wp-on--focusout="actions.handleMenuFocusout" tabindex="-1" '; - $responsive_dialog_directives = ' + $responsive_dialog_directives = ' data-wp-bind--aria-modal="state.ariaModal" data-wp-bind--aria-label="state.ariaLabel" data-wp-bind--role="state.roleAttribute" - data-wp-watch="callbacks.focusFirstElement" '; - $close_button_directives = ' + $close_button_directives = ' data-wp-on--click="actions.closeMenuOnClick" '; + $responsive_container_content_directives = ' + data-wp-watch="callbacks.focusFirstElement" + '; } return sprintf( @@ -457,7 +459,7 @@ private static function get_responsive_container_markup( $attributes, $inner_blo
-
+
%2$s
@@ -475,7 +477,8 @@ private static function get_responsive_container_markup( $attributes, $inner_blo $open_button_directives, $responsive_container_directives, $responsive_dialog_directives, - $close_button_directives + $close_button_directives, + $responsive_container_content_directives ); } diff --git a/packages/block-library/src/navigation/view.js b/packages/block-library/src/navigation/view.js index ba8e6d1a6683a..90a5ea388a136 100644 --- a/packages/block-library/src/navigation/view.js +++ b/packages/block-library/src/navigation/view.js @@ -1,18 +1,9 @@ /** * WordPress dependencies */ +import { focus } from '@wordpress/dom'; import { store, getContext, getElement } from '@wordpress/interactivity'; -const focusableSelectors = [ - 'a[href]', - 'input:not([disabled]):not([type="hidden"]):not([aria-hidden])', - 'select:not([disabled]):not([aria-hidden])', - 'textarea:not([disabled]):not([aria-hidden])', - 'button:not([disabled]):not([aria-hidden])', - '[contenteditable]', - '[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. @@ -169,8 +160,7 @@ const { state, actions } = store( 'core/navigation', { const ctx = getContext(); const { ref } = getElement(); if ( state.isMenuOpen ) { - const focusableElements = - ref.querySelectorAll( focusableSelectors ); + const focusableElements = focus.tabbable.find( ref ); ctx.modal = ref; ctx.firstFocusableElement = focusableElements[ 0 ]; ctx.lastFocusableElement = @@ -180,9 +170,8 @@ const { state, actions } = store( 'core/navigation', { focusFirstElement() { const { ref } = getElement(); if ( state.isMenuOpen ) { - ref.querySelector( - '.wp-block-navigation-item > *:first-child' - ).focus(); + const [ firstTabbable ] = focus.tabbable.find( ref ); + firstTabbable?.focus(); } }, }, From 3eb28c71f15b93fd23558c20b8ec612f56b76d2f Mon Sep 17 00:00:00 2001 From: Tetsuaki Hamano Date: Sun, 24 Dec 2023 14:09:26 +0900 Subject: [PATCH 2/3] Try adding a focus trap test for Safari --- .../editor/blocks/navigation-frontend-interactivity.spec.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/e2e/specs/editor/blocks/navigation-frontend-interactivity.spec.js b/test/e2e/specs/editor/blocks/navigation-frontend-interactivity.spec.js index ac6094c3d3eac..1d54f25b1a7cc 100644 --- a/test/e2e/specs/editor/blocks/navigation-frontend-interactivity.spec.js +++ b/test/e2e/specs/editor/blocks/navigation-frontend-interactivity.spec.js @@ -112,7 +112,11 @@ test.describe( 'Navigation block - Frontend interactivity', () => { // Test: overlay menu focuses on first element after opening await expect( overlayMenuFirstElement ).toBeFocused(); - // Not Tested: overlay menu traps focus + // Test: overlay menu traps focus + await pageUtils.pressKeys( 'Tab', { times: 2, delay: 50 } ); + await expect( closeMenuButton ).toBeFocused(); + await pageUtils.pressKeys( 'Shift+Tab', { times: 2, delay: 50 } ); + await expect( overlayMenuFirstElement ).toBeFocused(); // Test: overlay menu closes on click on close menu button await closeMenuButton.click(); From 31fca4a2e9fe9ec58de021f2e40095df165ebf84 Mon Sep 17 00:00:00 2001 From: Tetsuaki Hamano Date: Tue, 26 Dec 2023 17:42:59 +0900 Subject: [PATCH 3/3] Don't use dom package --- packages/block-library/src/navigation/view.js | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/packages/block-library/src/navigation/view.js b/packages/block-library/src/navigation/view.js index 90a5ea388a136..fb3919168a267 100644 --- a/packages/block-library/src/navigation/view.js +++ b/packages/block-library/src/navigation/view.js @@ -1,9 +1,18 @@ /** * WordPress dependencies */ -import { focus } from '@wordpress/dom'; import { store, getContext, getElement } from '@wordpress/interactivity'; +const focusableSelectors = [ + 'a[href]', + 'input:not([disabled]):not([type="hidden"]):not([aria-hidden])', + 'select:not([disabled]):not([aria-hidden])', + 'textarea:not([disabled]):not([aria-hidden])', + 'button:not([disabled]):not([aria-hidden])', + '[contenteditable]', + '[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. @@ -160,7 +169,8 @@ const { state, actions } = store( 'core/navigation', { const ctx = getContext(); const { ref } = getElement(); if ( state.isMenuOpen ) { - const focusableElements = focus.tabbable.find( ref ); + const focusableElements = + ref.querySelectorAll( focusableSelectors ); ctx.modal = ref; ctx.firstFocusableElement = focusableElements[ 0 ]; ctx.lastFocusableElement = @@ -170,8 +180,9 @@ const { state, actions } = store( 'core/navigation', { focusFirstElement() { const { ref } = getElement(); if ( state.isMenuOpen ) { - const [ firstTabbable ] = focus.tabbable.find( ref ); - firstTabbable?.focus(); + const focusableElements = + ref.querySelectorAll( focusableSelectors ); + focusableElements?.[ 0 ]?.focus(); } }, },