materializecss / materialize

Materialize, a web framework based on Material Design
https://materializeweb.com/
MIT License
910 stars 105 forks source link

web accessibility error: mobile navigation menu #229

Open nh916 opened 2 years ago

nh916 commented 2 years ago

This tends to be a common error and something that we can easily fix.

I was testing out a website for accessibility. I turned on a screen reader and started tabbing through the website. The reader and tab was able to successfully pick up and read each navigation bar item, then it started picking up the desktop-hidden mobile navigation menu and the tab was hidden in the mobile navigation menu and was confusing to work with.

I believe we can fix this with setting the mobile navigation menu style to none when it is not mobile.

I linked to an AMAZING video from a Google web accessibility series that talks about this exact common issue.

web accessibility common issues video

DanielRuf commented 2 years ago

Thanks for catching and reporting this issue. Generally accessibility is often quite complex and hard to do it right (that's what I can at least tell as ex-Foundation long-time contributor and maintainer).

What would the required changes be?

nh916 commented 2 years ago

I was looking up w3 to see how we can fix this and I think when the sub menu or slide out menu is off screen it's supposed to be marked as display none and when it comes back on screen it's supposed to be display block.

https://www.w3.org/WAI/tutorials/menus/flyout/

We can also add some aria tags to it to make it even more awesome and accessible if we want I think

DanielRuf commented 2 years ago

aria-hidden will be probably needed too, correct.

nh916 commented 2 years ago

It should be a button I think, so that the screen reader/any assistive technologies can recognize that its a button, but I have also seen it as an a tag and I think that might be fine too.

it must have an attribute of aria-expanded="false" when collapsed and aria-expanded="true" when expanded

From what I can find, I dont think we need a aria-hidden attribute because I think that would tell the assistive technologies that this element and all its children are for decorations only and not really needed for the page.

I think this from stackoverflow really made a lot of sense after reading the w3 docs "aria-hidden means hidden to screen readers and similar tools. This is useful for components which are used purely for formatting and contain no real content, for instance."

https://stackoverflow.com/questions/31107040/whats-the-difference-between-html-hidden-and-aria-hidden-attributes

but the official definition comes from here, that is more through https://www.w3.org/TR/wai-aria/#aria-hidden

In bootstrap 5, I noticed they made a huge push towards web accessibility within their framework and I think they've done it well and we can certainly take some inspiration from them and see how they made their dropdown accessible. For example this is one of them:

<div class="dropdown">
  <button class="btn btn-secondary dropdown-toggle" type="button" id="dropdownMenuButton1" data-bs-toggle="dropdown" aria-expanded="false">
    Dropdown button
  </button>
  <ul class="dropdown-menu" aria-labelledby="dropdownMenuButton1">
    <li><a class="dropdown-item" href="#">Action</a></li>
    <li><a class="dropdown-item" href="#">Another action</a></li>
    <li><a class="dropdown-item" href="#">Something else here</a></li>
  </ul>
</div>

https://getbootstrap.com/docs/5.1/components/dropdowns/#menu-items

gselderslaghs commented 2 weeks ago

After digging into this issue I can see there quite few different setups being implemented by Materialize. From my opinion it would be a great enhancement towards web-accessibility implementing the aria-expanded feature.

Here is my opinion in reviewing the current navbar component as described in the documentation (https://materializeweb.com/navbar.html)

Dropdown buttons in menu (https://materializeweb.com/navbar.html#navbar-dropdown) Dropdown buttons as explained in the navbar component documentation are missing the aria-expanded parameter

Sidenav mobile navigation (https://materializeweb.com/navbar.html#mobile-collapse) Sidebar mobile navigation might be confusing for screen readers, as mentioned by @nh916 there is a duplicate HTML element rendering the desktop and mobile/sidenav menu (.nav-wrapper, .sidenav). It would be great if this could be adjusted for screen readers. The idea is to read only from the current activated element by toggling the aria-hidden state on both elements on load/resize. By implementing the aria-hidden state it indicates the exposure of the element to the accessibility API and would prevent duplicate exposure to the accessibility API.

gselderslaghs commented 1 week ago

Please review the addressed issues in #514 and #516