Important: the initial proposal wasn't appropriate. Eventually an aria-expanded disclosure pattern was implemented, see comments #5 onwards.

The primary menu in the PoC is using the .has-children class on the list item to apply styling to the dropdown menu. In case of accessibility, this should be changed to applying the aria-haspopup=true to the list item.

An example and instructions on how this should work can be found here.

Also, it can be thought of adding aria-controls to the link which opens it on hover and add aria-labelledby to the expanding menu.

The current markup looks like this:

<ul class="primary-nav--level-1">
  <li><a href="#"><span>Home</span></a></li>
  <li class="has-children">
    <a href="#"><span>Products</span></a>
    <button class="primary-nav__button-expand">Expand sub-navigation</button>
    <ul class="primary-nav--level-2">
      <li><a href="#"><span>Black &amp; White Photos</span></a></li>
      <li><a href="#"><span>Stone Statues</span></a></li>
      <li><a href="#"><span>Antiques &amp; Artifacts</span></a></li>
      <li><a href="#"><span>Replicas</span></a></li>
    </ul>
  </li>
  <li><a href="#"><span>About</span></a></li>
  <li><a href="#"><span>Blog</span></a></li>
  <li><a href="#"><span>Contact</span></a></li>
</ul>

The corrected markup should look like this:

<ul class="primary-nav--level-1">
  <li><a href="#"><span>Home</span></a></li>
  <li aria-haspopup="true" aria-controls="nav--level-2">
    <a href="#"><span>Products</span></a>
    <button id="menubutton" class="primary-nav__button-expand">Expand sub-navigation</button>
    <ul id="nav--level-2" aria-labelledby="menubutton" class="primary-nav--level-2">
      <li><a href="#"><span>Black &amp; White Photos</span></a></li>
      <li><a href="#"><span>Stone Statues</span></a></li>
      <li><a href="#"><span>Antiques &amp; Artifacts</span></a></li>
      <li><a href="#"><span>Replicas</span></a></li>
    </ul>
  </li>
  <li><a href="#"><span>About</span></a></li>
  <li><a href="#"><span>Blog</span></a></li>
  <li><a href="#"><span>Contact</span></a></li>
</ul>

Screenshot of current implementation:

screenshot of olivero primary menu

CommentFileSizeAuthor
Bildschirmfoto 2019-10-28 um 15.34.18.png71.79 KBfhaeberle
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

fhaeberle created an issue. See original summary.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
CreditAttribution: mherchel commented
Assigned: Unassigned » mherchel
mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
CreditAttribution: mherchel commented
Status: Active » Needs review

This is at https://github.com/Lullabot/olivero-poc/pull/3/files

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
CreditAttribution: mherchel at Lullabot commented
Status: Needs review » Fixed

Merged.

andrewmacpherson’s picture

andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer commented
Status: Fixed » Needs work

Please revert this change. The aria-haspopup="true" introduced here (a) isn't appropriate here, and (b) isn't correctly implemented.

The ARIA menu button pattern you linked to is very controversial in the accessibility community. The upshot is that ARIA menus are inappropriate for website navigation links. Some useful reading:

The PR mentioned in #3 isn't correct implementation of the pattern anyway. The presence of aria-haspopup="true" means there needs to be an associated container with the ARIA menu role. This in turn can only contain interactive children with the menuitem role. These in turn are not conveyed to assistive tech as links.

Please don't use ARIA properties in a piecemeal way like this. A HTML class doesn't affect semantics, doesn't imply a defined content model, and doesn't affect the accessibility tree. ARIA does all three, and if used incorrectly it can have disastrous results. See No ARIA is better than bad ARIA. If using one of the patterns in the ARIA Authoring Practices, be sure to implement it completely. I also recommend seeking guidance on which patterns are appropriate; they aren't suitable for all occasions. ARIA menus in particular are best suited to application behaviour, rather than website navigation.

Instead, I recommend a simple submenu disclosure pattern:

  • Use aria-expanded and aria-controls on the button.
  • No other ARIA roles are needed; the HTML semantics of button, ul, li, a elements are sufficient.
  • The button should be displayed in both the wide and narrow breakpoints.
  • The submenu should be hidden completely with display:none;. (I saw some animation, which may complicate this. A good approach is open/close the menu in two steps. One step animates visually-hidden, another does display:none; when visually hidden. Umami ran into this problem too.)
  • The Link + Disclosure Widget Navigation is very good.
fhaeberle’s picture

fhaeberle
he/him
Location 🇩🇪 Stuttgart
CreditAttribution: fhaeberle commented

@andrewmacpherson I apologize for my fast written issue summary where I didn't outline the complete and correct way, this was created in a hurry to have a few actionable issues for DrupalCon sprinting. Thanks for pointing us in the right direction.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
CreditAttribution: mherchel at Lullabot commented

The button should be displayed in both the wide and narrow breakpoints.

I'm breaking this out into its own discussion at #3093693: Discussion on showing drop-menu button at large widths..

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
CreditAttribution: mherchel at Lullabot commented

@andrewmacpherson:

Regarding

The submenu should be hidden completely with display:none;. (I saw some animation, which may complicate this. A good approach is open/close the menu in two steps. One step animates visually-hidden, another does display:none; when visually hidden. Umami ran into this problem too.)

My understanding is that visibility: hidden removes the DOM node from the accessibility tree. As you said, it's useful to use this because it respects CSS transitions (which display: none does not). Is this not sufficient? The DOM node isn't available to assistive devices (let me know if i'm wrong on this), and is not focusable. What additional accessibility benefit does display: none add?

shaal’s picture

shaal
he/him
Primary language English
Location Boca Raton, FL
CreditAttribution: shaal as a volunteer and at Palantir.net commented

In OOTB Umami, we created 2 separate markups for the menus, one for desktop, and another for mobile. CSS determines which should be displayed.

We also had accessibility issues with the mobile menu and the language switcher (menu).

After many iterations we accomplished building an accessible and smooth dropdown menu.
It's a public function that is being imported to both mobile-menu and language switcher.

#3042417: Accessible dropdown for Umami's language-switcher and mobile main-menu

andrewmacpherson’s picture

andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer commented

Yes, visibility:hidden or display:none will do equally well. Both of there remove the element and it's children from the tab order, and from the accessibility tree. That's what I meant by completely hidden.

The problem is, that the menu isn't actually hidden this way when it matters.

I can see that visibility:hidden is being used when the focus is somewhere else, and menu is collapsed. However the menu is being revealed when the user tabs to the products link. The problem here is the focus-within pseudo-class is being used do reveal the submenu, by setting visibility: visible.

If a keyboard user wants to operate the "About" link, they have to tab through all of the submenu items under products. The user has no choice in this; the disclosure button may as well not be there. They can't tab from "Products" to "About".

A sighted pointer user is allowed to ignore the submenus. A keyboard user should be allowed to do the same.

Stop revealing the submenu using focus-within, and use the submenu button at both breakpoints. That way keyboard users don't have to tab through the submenu, unless they want to open it.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
CreditAttribution: mherchel at Lullabot commented
Status: Needs work » Needs review

Reverted this at https://github.com/Lullabot/olivero-poc/pull/17/files

Note that the discussion for implementing subnav toggle buttons has been moved to #3093693: Discussion on showing drop-menu button at large widths..

vulcanr’s picture

vulcanr CreditAttribution: vulcanr as a volunteer and at Annertech commented

Should the aria attributes be on the button instead of the <li> element? And I think instead of aria-haspopup should be used aria-expanded

andrewmacpherson’s picture

andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer commented

The <button>:

  • should be used at both breakpoints
  • should use aria-expanded to convey the state
  • should have aria-controlswith an IDREF pointing at the <ul>
  • Optionally, aria-labelledyby or aria-label

No other ARIA attributes are needed for this.

aria-haspopup isn't appropriate for navigation lists, and should not be used.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
CreditAttribution: mherchel at Lullabot commented

@vulcanr and @andrewmacpherson:

All items have been addressed (except the aria-controls, which I'll take care of now). Please see the comment in #11.

vulcanr’s picture

vulcanr CreditAttribution: vulcanr as a volunteer and at Annertech commented
Status: Needs review » Reviewed & tested by the community

This is looking good now according to https://github.com/Lullabot/olivero-poc/pull/17 and tested on https://deploy-preview-17--olivero-poc.netlify.com/.

I'll mark this into RTBC

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
CreditAttribution: mherchel at Lullabot commented
Status: Reviewed & tested by the community » Fixed

Thanks! Merged!

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
CreditAttribution: mherchel at Lullabot commented
andrewmacpherson’s picture

andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer commented
Title: Implement aria-haspopup=true instead of using the has-children CSS class on the primary menu » Convey behaviour of navigation submenu to assisitive tech.
Issue summary: View changes

I tested the changes, they are correct.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.