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

Made menubar with nested dropdowns keyboard accessible based on W3's menubar design pattern #35462

Closed
wants to merge 7 commits into from

Conversation

FluffyPorkBuns
Copy link

@FluffyPorkBuns FluffyPorkBuns commented Dec 3, 2021

Made it so a menubar with nested dropdowns will be keyboard accessible according to W3's menubar design pattern: https://www.w3.org/TR/wai-aria-practices/examples/menubar/menubar-1/menubar-1.html

  • Top-level dropdowns can be navigated through with the left and right arrows or tab keys.
  • Top-level dropdowns are opened with spacebar, down arrow, or enter key. First item in dropdown is selected automatically.
  • Dropdowns can be navigated with up and down arrows or tab key and selection wraps from beginning to end / end to beginning.
  • Sub-dropdowns can be opened with right arrow, spacebar, or enter key. First item in sub-dropdown is selected automatically.
  • Dropdowns and sub-dropdowns can be closed with the left arrow or escape key. Only the currently open dropdown or sub-dropdown will close (will not bubble up to all parents and close everything). The item that opened that dropdown is selected automatically.
  • Dropdown and sub-dropdown links can be opened with spacebar or enter key and closes entire menubar.
  • Only one dropdown can be open at a time in a menubar.
  • Fixed a bug where the spacebar didn't open dropdown items (dropdown.js line 39).

I'm guessing my changes may not fit the existing code gracefully (this was created for a specific menubar for a project and I'm not that familiar with this codebase) but I'm interested in hearing what others think about adding these accessibility features to the existing dropdown code.

@FluffyPorkBuns FluffyPorkBuns requested review from a team as code owners December 3, 2021 00:35
@ffoodd
Copy link
Member

ffoodd commented Dec 3, 2021

AFAIK the ARIA menu design pattern is not meant to be used for standard website navigation, is it?

@patrickhlauke any argument on whether we should implement this or not?

Copy link
Member

@XhmikosR XhmikosR left a comment

Choose a reason for hiding this comment

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

You have unrelated changes in the PR, please drop them

@patrickhlauke
Copy link
Member

@ffoodd correct, ARIA menu bar patterns are generally meant for application-like menus, not for navigation menus.

The use of an ARIA menu for site navigation has long been a controversial topic. Most recent active discussion can be found here w3c/aria-practices#353. The general consensus among most practitioners today is that ARIA menus should really only be used for "application-like" menus (in web applications that mimic apps, and have menus that trigger commands like Copy/Paste/etc. (however, that ARIA Practices example for navigation continues to persist in version 1.2, much to the chagrin of many people involved).

Note that for a proper ARIA menu implementation, you'd also need all the relevant role="..." attributes etc as per the pattern - from a quick skim over the file changes, I don't think this PR does that - does it just add the keyboard pattern?

In any case, I think we'd pass on this. We used to have a basic part of ARIA menu stuff in Bootstrap years ago, but we removed it because it's not universal/generic enough - we know that authors use bootstrap's dropdowns for things that are simply not ARIA menus (a real ARIA menu only allows you to have menu items, menu radio button items, menu checkbox items, and submenus - as soon as you put in anything else, like a search box or even just a bit of static text, you are unable to express that correctly with ARIA roles since that simply won't translate to correct/sensible structures in the accessibility tree that browsers pass on to assistive technologies).

I'd suggest not merging this, but an idea may be for the author to release this as some sort of extension/plugin that others can use when/if their menus do fit the pattern.

@FluffyPorkBuns
Copy link
Author

@ffoodd correct, ARIA menu bar patterns are generally meant for application-like menus, not for navigation menus.

The use of an ARIA menu for site navigation has long been a controversial topic. Most recent active discussion can be found here w3c/aria-practices#353. The general consensus among most practitioners today is that ARIA menus should really only be used for "application-like" menus (in web applications that mimic apps, and have menus that trigger commands like Copy/Paste/etc. (however, that ARIA Practices example for navigation continues to persist in version 1.2, much to the chagrin of many people involved).

Note that for a proper ARIA menu implementation, you'd also need all the relevant role="..." attributes etc as per the pattern - from a quick skim over the file changes, I don't think this PR does that - does it just add the keyboard pattern?

In any case, I think we'd pass on this. We used to have a basic part of ARIA menu stuff in Bootstrap years ago, but we removed it because it's not universal/generic enough - we know that authors use bootstrap's dropdowns for things that are simply not ARIA menus (a real ARIA menu only allows you to have menu items, menu radio button items, menu checkbox items, and submenus - as soon as you put in anything else, like a search box or even just a bit of static text, you are unable to express that correctly with ARIA roles since that simply won't translate to correct/sensible structures in the accessibility tree that browsers pass on to assistive technologies).

I'd suggest not merging this, but an idea may be for the author to release this as some sort of extension/plugin that others can use when/if their menus do fit the pattern.

Thanks for weighing in! The menubar I worked on did follow the rest of the ARIA guidelines in the pattern such as the "role=" property and providing some sample html probably would have helped. That's interesting this pattern is non-standard for a nav, I had no idea when I found the documentation and implemented it. Following the pattern did allow the nav on the website I was working on to pass a third party accessibility certification (dropdowns and sub-dropdowns had to be fully keyboard accessible) but maybe a simpler pattern would have worked as well.

I'll go ahead and close this PR and consider creating an extension that fits this particular use case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants