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.

Sign up for GitHub

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

Jump to bottom

Navigation Block: Use dom.focus for focus control #57362

Merged
merged 4 commits into from
Dec 28, 2023
Merged

Navigation Block: Use dom.focus for focus control #57362

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
57f0a13
Navigation Block: Use dom.focus for focus control
t-hamano Dec 23, 2023
3eb28c7
Try adding a focus trap test for Safari
t-hamano Dec 24, 2023
363a096
Merge branch 'trunk' into navigation/focus-first-element
t-hamano Dec 26, 2023
31fca4a
Don't use dom package
t-hamano Dec 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 10 additions & 7 deletions lib/compat/wordpress-6.5/class-wp-navigation-block-renderer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -428,27 +428,29 @@ private static function get_responsive_container_markup( $attributes, $inner_blo
$responsive_dialog_directives = '';
$close_button_directives = '';
if ( $should_load_view_script ) {
$open_button_directives = '
Copy link
Contributor

Choose a reason for hiding this comment

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

I checked and these changes are still included in the WP_Navigation_Block_Renderer which has now been moved to the block-library package and thus will be automatically synced to Core as part of the packages sync process.

$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"
data-wp-on--keydown="actions.handleMenuKeydown"
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(
Expand All @@ -457,7 +459,7 @@ private static function get_responsive_container_markup( $attributes, $inner_blo
<div class="wp-block-navigation__responsive-close" tabindex="-1">
<div class="wp-block-navigation__responsive-dialog" %12$s>
<button %4$s class="wp-block-navigation__responsive-container-close" %13$s>%9$s</button>
<div class="wp-block-navigation__responsive-container-content" id="%1$s-content">
<div class="wp-block-navigation__responsive-container-content" %14$s id="%1$s-content">
%2$s
</div>
</div>
Expand All @@ -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
);
}

Expand Down
19 changes: 4 additions & 15 deletions packages/block-library/src/navigation/view.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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 =
Expand All @@ -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();
}
},
},
Expand Down
6 changes: 5 additions & 1 deletion test/e2e/specs/editor/blocks/navigation-frontend-interactivity.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Comment on lines -115 to +119
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't know why this test wasn't done in Safari, but let's try it and see if it works.


// Test: overlay menu closes on click on close menu button
await closeMenuButton.click();
Expand Down