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

Overflow menu needs accessibility work #666

Closed
carmacleod opened this issue Mar 20, 2018 · 6 comments · Fixed by #1115
Closed

Overflow menu needs accessibility work #666

carmacleod opened this issue Mar 20, 2018 · 6 comments · Fixed by #1115

Comments

@carmacleod
Copy link
Contributor

The Carbon Overflow Menu is not completely accessible. It still needs:

  • semantic markup to tell screen reader users that it is not just a "clickable"... it is in fact a "menu button"
  • expected keyboard behavior for a menu button, so that keyboard-only users can use the menu

See the APG menu button pattern for guidance, including either of the Action Menu Button examples (not the Navigation Menu Button example).

@carmacleod
Copy link
Contributor Author

carmacleod commented Mar 20, 2018

Here's a sample of what the html would look like with the additional roles and aria-* attributes:

<div data-overflow-menu tabindex="0" aria-label="Overflow" class="bx--overflow-menu" role="button" aria-haspopup="true" aria-expanded="false">
  <svg aria-hidden="true" class="bx--overflow-menu__icon" width="4" height="20" viewBox="0 0 4 20">
    <circle cx="2" cy="2" r="2"></circle>
    <circle cx="2" cy="10" r="2"></circle>
    <circle cx="2" cy="18" r="2"></circle>
  </svg>
  <ul class="bx--overflow-menu-options" tabindex="-1" role="menu" aria-label="Overflow">
    <li class="bx--overflow-menu-options__option" role="presentation">
      <button class="bx--overflow-menu-options__btn" data-floating-menu-primary-focus>Option 1</button>
    </li>
    <li class="bx--overflow-menu-options__option" role="presentation">
      <button class="bx--overflow-menu-options__btn" role="menuitem">Option 2</button>
    </li>
    ...
  </ul>
</div>

Notes:

  • the aria-label should just be "Overflow", and not Overflow menu description, which is misleading because (a) it is actually a label, not a description, and (b) it should not repeat the word "menu" because the screen readers already say "menu" (actually, they would say "menu button" for this).
  • the menu should open on space and optionally down and/or up arrow (not just enter).
  • consider using an actual <button> for the menu button, and not just <div role="button"> so that you get button semantics & behavior for free (you may have a bit more challenge styling it, but I think? it should be possible)
  • the menu also has an aria-label, not just the button (I found that strange at first, but it works well with screen readers - they say "Overflow menu button" when the user tabs to the button, and then when the user opens the menu, they say "Overflow menu")
  • when the menu is open, aria-expanded="true" on the button div.
  • down and up arrows should move through the menu items, and Escape should close the menu and return focus to the menu button

@tw15egan
Copy link
Collaborator

@carmacleod Thanks for taking the time to not only create the issue but fill us in on the intended vs actual behavior of the component. This really helps increase the speed in which we can get these fixes merged in 😄

@carmacleod
Copy link
Contributor Author

Oh, by the way, please do not copy the "open on hover" behavior from the APG menu button examples.
(I mentioned to them that I think they should remove that particular behavior).

@bocampbell
Copy link

Leaving comment to get my profile attached to this thread.

@carmacleod
Copy link
Contributor Author

carmacleod commented Mar 21, 2018

@tw15egan When fixed, please test with keyboard, and then test again with screen reader. If possible, please test with VoiceOver in Safari on Mac, and with JAWS and/or NVDA in Firefox and/or Chrome on Windows. I would be happy to help test - please contact me.

@carbon-bot
Copy link
Contributor

🎉 This issue has been resolved in version 9.20.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging a pull request may close this issue.

5 participants