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

Header: Switch to submenu blocks for navigation items with submenus #210

Merged
merged 4 commits into from
Jun 28, 2022

Conversation

ryelle
Copy link
Contributor

@ryelle ryelle commented Jun 16, 2022

Fixes #207 - Menu items with submenus should use the navigation-submenu block, this provides the expected markup for accessibility:

  • Submenus don't open on focus.
  • Items with a submenu (Support & Get Involved) have buttons following that say "Support submenu"/"Get Involved submenu".
  • The button has aria-expanded="false" by default, and true when open

The remaining item on the list in #207 is this:

If inside a sub menu, pressing Escape should close the sub menu and move focus to the trigger that opened it.

This menu doesn't seem to do that, but this doesn't seem to be core behavior either. I'd recommend opening an issue in gutenberg for the navigation block if this is required.

Props @alexstine.

Screenshots

In both of the videos below, I tab through the menu. In "before" you can see I tab down into the submenus with no other action. After, I need to activate the 🔽 button. There's no visual change, and no change in behavior with a mouse.

before.mp4
after.mp4
Before After

@ryelle ryelle added the Accessibility Issues related to keyboard navigation, screen readers, etc label Jun 16, 2022
@ryelle ryelle self-assigned this Jun 16, 2022
@ryelle ryelle requested a review from StevenDufresne June 16, 2022 20:02
@StevenDufresne
Copy link
Contributor

When the menu is truncated, the submenu is not reachable:

menu-updates.mp4

);
}

$ouput = sprintf(
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is a typo: $output maybe?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤦🏻

@alexstine
Copy link
Contributor

Thanks @ryelle ! I opened a PR upstream in Gutenberg which should fix up Escape key functionality.

@ryelle
Copy link
Contributor Author

ryelle commented Jun 20, 2022

When the menu is truncated, the submenu is not reachable

Ah, I bet when the menu item is cloned, the toggle looses the event listener. I've updated the style on submenus in the overflow menu so they'll always display to work around that. Also because 3rd-level flyouts are awkward :)

I also updated the ... link when the menu is truncated to be a button and use the same toggle-on-hover/click pattern. This fixes an issue where screen readers would just announce the link as "dot" (at least, VoiceOver did), now it's "More menu" with the expanded context.

Copy link
Contributor

@StevenDufresne StevenDufresne left a comment

Choose a reason for hiding this comment

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

Great, looks good.

@ryelle ryelle merged commit 023c9c7 into trunk Jun 28, 2022
@ryelle ryelle deleted the update/menu-a11y branch June 28, 2022 16:43
bazza pushed a commit to WordPress/wordpress.org that referenced this pull request Jun 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accessibility Issues related to keyboard navigation, screen readers, etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Navigation sub menus not totally accessible via the keyboard
3 participants