-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Add ARIA support to menu toggle for improved accessibility #545
Comments
Yes please! I assume you have also checked these menus. http://a11yproject.com/patterns/ String |
@samikeijonen Great feedback! Yes, I actually took a good bit of inspiration and code from the menu listed on the Accessibility Project. I'm also looking at the toggle on the Paciello Group's recent redesign. Great reminder on localizing the string – I'll definitely incorporate that into my eventual PR. I'm also gathering some feedback from other WordPress Accessibility team members. |
After talking with other members of the WordPress Accessibility team, I've simplified my prototype a bit. I've removed the aria-label, relying on the text for the button alone instead. I used the aria-label because the majority of sites I see just have a button that says "Menu" or has an icon with no text alternative. This was my way of helping that to be better, but it is probably too heavy-handed. Also, we don't need aria-hidden, since aria-expanded takes care of things. New version: http://codepen.io/davidakennedy/pen/aqhys It's much lighter, which I think is better for a starter theme, like Underscores. |
I've been doing a little testing in Chrome with the ChromeVox screen reader and this seems to work well. I can provide more specific information on how ChromeVox handles this if it's useful. |
At the moment |
Makes it a little easier to use the primary navigation with assistive devices. See #545.
Fixed in e142af3. |
Just to note: I tested e142af3 and it works well from an accessibility standpoint. |
Makes it a little easier to use the primary navigation with assistive devices. See Automattic#545.
I'd like to propose that we add ARIA to the Primary Menu toggle and the associated
ul
that gets expanded or collapsed as a result.Here is a good primer on ARIA.
I'd like to add these attributes:
aria-controls
aria-label
aria-labeledby
aria-hidden
aria-expanded
I built a quick prototype on CodePen.
I figured a quick demo was better so we can discuss if need be, and it's probably better to wait on a PR until #540 is sorted.
Explaining the prototype:
header.php
a bit cleaner. That makes it easier to remove the toggle if a theme author does not need it.Markup:
aria-label="Toggle Primary Menu"
: Adds a label to the button. Notice it says, toggle, which is more descriptive of what it does than it's visual label. Screen readers with support will announce this rather than text. Helpful, especially if using an icon as the button.aria-labelledby="menu-toggle"
: Maps the button label of element it's label.aria-controls="menu"
: Maps buttons functionality to the thing it's controlling.aria-expanded="true"
: Or false. Is this thing expanded or not? Screen readers with support will announce this, like: "Toggle Primary Menu button expanded"aria-hidden="true"
: Or false. Is this thing hidden or not?One thing I'm still researching: Whether I need both
aria-hidden
andaria-expanded
. In this case, it may be better to use justaria-expanded
.I'm open to suggestions and improvements, and of course, whether this should be included in
_s
.I think it makes sense as long as we're providing a basic toggle. This would improve the accessibility of the toggle functionality, and provide a nice example to theme developers.
The text was updated successfully, but these errors were encountered: