Improve menu accessibility

[fuzzbomb]

I had a look at the mobile menu JS, CSS, and Twig templates, for accessibiity (the patch in Implement the Out of the Box designs as a core theme, comment 29).

Currently it works by toggling a class, which changes the height of the menu. This has some a11y problems:

1. The button only does something when JS is enabled. When JS is off, the button is there in markup, but does nothing.
2. The button is outside the navigation landmark. When closed, a screen reader user might search for the navigation landmark region, but find it empty. They should find the button inside the nav landmark region.
3. Menu links are always in the tabbing order, changing the wrapper's height just hides them visually. A keyboard user will have to tab through these even when the menu is closed. For screen reader users, it defeats the object of the toggle button. For sighted keyboard users, it's another invisible-but-still-operable problem, very confusing.
4. The open closed state of the menu is not conveyed to assistive tech. Screen reader users just know there's a button, not whether the menu is open or closed.
5. Menu is animated, some users prefer no motion. We're forming a Drupal core policy around this, see "Use "prefers-reduced-motion" media query to disable animations". It's early days for this policy so this part isn't a blocker for MVP.

Fixes (same numbering as the problems):

1. Remove the button from the Twig template, create it using JS instead
2. Move the toggle button to just inside the start of <nav>.
3. Use display: none or visibility: hidden on the <ul> to remove the links from the tabbing order when the menu is closed. 3A. If this is a problem for the animation, do it in two steps. When the menu opens, first remove the display:none, then change the height. Reverse order of these steps when closing.
4. As well as toggling the CSS class for styling, toggle aria-expanded=true|false. This attribute goes on the <button>. 4a. The <button> also wants an aria-controls=<IDREF>, pointing to the ID of the <ul> that gets toggled.
5. Use the new prefers-reduced-motion media query to allow users to turn off animation if their platform allows it (currently just Safari).

[fuzzbomb]

The menu button is currently called "Toggle the menu" using aria-label. "Menu" will be enough, once aria-expanded is present and toggling.

[thamas]

@fuzzbomb Thanks for the detailed a11y info!

I'm do not know too much about a11y so I need some more clarification / help:

1. You mention that without JS the menu button does nothing and that is a problem to have button in the markup which not operates. Is it an a11y thing to think about users won't have JS? (I think so, of course…) In this case there is an other thing I was thinking about: currently the menu is hidden by default and needs js to add the class which shows it. Should we change this behavior and show the menu by default and hide if js presented? And if it should be done this way wont we have a menu flash while js hides it?

2. Branding, Menu toggle button, Search icon and the menu itself must be siblings: to be in the same level in the HTML we should use ugly / instable and or not supported CSS solutions to create the designed layout. So the menu button must be a sibling of the menu and not a child of it. Do you have a recommendation to resolve it?

3–5. OK

FYI this issue was originally about to change the JS to ES6. That is an other problem than the accessibility of the menu so I created a new issue for that (#131) and gave a new title to this.

[cehfisher]

In case it might be helpful, here is an example of an accessible mobile button pattern with minimal JS: http://a11y-style-guide.com/style-guide/section-navigation.html#kssref-navigation-navigation-mobile

I realize it is not 100% JS free but does include important navigational/toggle elements such as: aria-expanded="false"; aria-controls="nav-menu-ex-1"; class="screen-reader-text"; and aria-hidden="true" to enhance the menu experience.

[fuzzbomb]

@thamas:

Is it an a11y thing to think about users won't have JS?

Well it's an a11y thing to think about everyone, and treat JS as progressive enhancement. Users with disabilities generally leave JS turned on, like anyone else. This isn't really about users who deliberately turn JS off; rather it's about situations where JS fails to run for any reason at all, like a badly configured proxy.

currently the menu is hidden by default and needs js to add the class which shows it. Should we change this behavior and show the menu by default and hide if js presented?

Yes, only hide the menu if JS runs. If you hide it with CSS, and JS doesn't run, then there's no way to use the links. This can impact anyone.

And if it should be done this way wont we have a menu flash while js hides it?

I didn't think about that, but the links should be visible in situations where where JS fails to run, but CSS works.

Branding, Menu toggle button, Search icon and the menu itself must be siblings: to be in the same level in the HTML we should use ugly / instable and or not supported CSS solutions to create the designed layout. So the menu button must be a sibling of the menu and not a child of it. Do you have a recommendation to resolve it?

I don't understand why the button must be a sibling of the branding and search icon? Currently the <button> is a sibling of the <nav>. Instead the <button> should be a child of the <nav>, and a sibling of the <ul>`, like this:

<!-- the navigation landmark region should always available, don't hide this wrapper-->
<nav role="navigation">
    <!-- A screen reader user who jumps to the navigation landmark should find the button inside -->
    <button aria-controls="menu-links" aria-expanded="false">menu</button>
    <!-- the hide/show behaviour only applies to this UL -->
    <ul id="menu-links'">
        <li>...</li>
    <ul>
</nav>

The important bit is that <nav> is a landmark region, with special accessibility semantics; screen readers and other assistive tech can jump from one landmark region to another. If the <button> is outside the <nav>, it won't be found by someone who jumps to the navigation landmark. The menu button on @cehfisher's style guide does this correctly.

The <nav> can certainly be a sibling of the branding and search icon, if you like. For instance it's okay for <nav role="navigation"> and <div role="search"> landmark regions to be siblings, both descendants of a <header>.

[thamas]

@fuzzbomb Thanks for the answers!

This isn't really about users who deliberately turn JS off; rather it's about situations where JS fails to run for any reason at all, like a badly configured proxy.

And is it better to not to have a button at all than have a button which do not operates? Is it less confuse?

And if it should be done this way wont we have a menu flash while js hides it?

I didn't think about that, but the links should be visible in situations where where JS fails to run, but CSS works.

Thats ok, it has to be accessible, but a site wher every page jumps a when loaded would not be to usable I think. So I'm just looking for the good solution.

I don't understand why the button must be a sibling of the branding and search icon?

As I wrote the designed layout needs that html structure. There should be other solutions to get the same look—and I'm thinking about them—but the CSS will be crappy, not so stable and more complicated (in the solutions I was thinking of already).

Isn't there a way to keep the button outside the nav element but connect it with that?

+1 Thing: this specific comment should be here I think, because it is also about the menu trigger button: https://github.com/lauriii/umami/issues/138#issuecomment-355833747 Could you provide please the right code? (Is it enough to delete aria-labelledby and the title and desc parts?)

[fuzzbomb]

@thamas, for the SVG

Could you provide please the right code? (Is it enough to delete aria-labelledby and the title and desc parts?)

Yes, that's right. Delete this metadata from the embedded SVG is right, we just need the aria-label on the button itself. These are the changes (attributes that don't need to change are omitted from this example code):

Before:

<button aria-label="Toggle the menu">
    <svg aria-labelledby="menu-toggle-title menu-toggle-desc">
        <title id="menu-toggle-title">Menu toggle icon</title>
        <desc id="menu-toggle-desc">Hamburger icon for menu toggle.</desc>
        // ... the graphic parts
    </svg>
</button>

After:

<button aria-label="Main menu" aria-expanded="true">
    <svg>
        // ... the graphic parts
    </svg>
</button>

The button label can be "Menu", or "Main menu" to be clearer. No need to say "toggle" in the aria-label, that will be conveyed by the aria-expanded property we introduce.

[fuzzbomb]

is it better to not to have a button at all than have a button which do not operates? Is it less confuse?

That's right. For a sighted user, it's easy to understand that "nothing seems to be happening when I press this". For a visually impaired user with a screen reader, the experience is more like "I pressed the button, but I have no idea if anything has happened". Various ARIA properties, such as aria-expanded, are about giving immediate feedback in a semantic, programatically-determinable way, but they likewise need to be managed via JS.

I don't understand why the button must be a sibling of the branding and search icon?

As I wrote the designed layout needs that html structure. There should be other solutions to get the same look—and I'm thinking about them—but the CSS will be crappy, not so stable and more complicated (in the solutions I was thinking of already).

Ah right, responsive CSS layouts are not my strongpoint, I was only thinking about the programatically-determinable semantics for assistive tech.

Isn't there a way to keep the button outside the nav element but connect it with that?

Sadly no. The point about the <nav> element is that it has an implicit ARIA landmark role. The purpose of these is to allow assistive tech users to identify major parts of the page and quickly jump to them.

When entering a landmark region, most modern screen readers announce the region's label and role. Some screen readers will also announce when exiting a landmark region, too.

Let's say we have this DOM state. with the button outside the nav:

<button aria-label="menu" aria-expanded="false">
<nav>
    <ul style="display:none;">
        // links not announced or focusable due to display:none
    </ul>
</nav

When a screen reader user enters the navigation landmark region, they will find it is apparently empty. Some users may give up in frustration and try a different website. Experienced or persistent users might resort to other screen reader features like "list all buttons", "list all links", or "find in page". Some savvy users might go looking for a button nearby, but outside of the landmark region (ours is currently nearby in source order, which mitigates the problem).

If the button is inside the nav, a user will hear "Navigation landmark. Menu, button, collapsed." and can quickly figure out that it's some kind of drop-down widget.

[fuzzbomb]

To move this forward, we can address points 1, 3, 4, and 5 for MVP.

Ideally the toggle button should be inside the nav element, but at this stage I don't want to disrupt the CSS styling. Let's postpone point 2 until after we get the MVP in core.

[fuzzbomb]

This can be closed here, we have an issue for it on D.O

https://www.drupal.org/project/drupal/issues/2940023