MudNavMenu: Use nav instead of div for better accessibility

[igotinfected]

Description

Fixes: #7222

The original issue requested an override on MudDrawer for the aside tag. However, according to MDN (and other frameworks), having navigation blocks inside of aside is not an issue, but navigation blocks should always be inside of a nav tag. The logical solution for me then is to have MudNavMenu translate into nav rather than div.

How Has This Been Tested?

Visually tested and confirmed with other major accessibility focused sites:

Drawer documentation post-fix:

MDN:

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

Potentially breaking if users expect a div tag in MudNavMenu and have custom styling around that expectation.

Checklist

• The PR is submitted to the correct branch (dev).
• My code follows the code style of this project.
• I've added relevant tests.

[igotinfected]

@bunkaisatori @zola-25 this change will "fix" the doc examples, can you confirm aside > nav > link is acceptable in terms of accessibility?

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.03%. Comparing base (28bc599) to head (00a5531).
Report is 39 commits behind head on dev.

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #8674      +/-   ##
==========================================
+ Coverage   89.82%   90.03%   +0.20%     
==========================================
  Files         412      418       +6     
  Lines       11878    12006     +128     
  Branches     2364     2366       +2     
==========================================
+ Hits        10670    10810     +140     
+ Misses        681      661      -20     
- Partials      527      535       +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[danielchalmers]

Should this be done to the docs footer as well?

[igotinfected]

Should this be done to the docs footer as well?

Ideally the docs should use header, main, and footer tags, but this can also be done via the role attribute. In theory adding role="navigation" to MudNavMenu would also have been a valid solution in this PR.

https://www.w3.org/WAI/ARIA/apg/practices/landmark-regions/

All navigation blocks (in the docs there can be 4 navigation blocks at once I think) should either be surrounded by a nav tag or alternatively by a container with role="navigation" (and when multiple of the same landmarks are available, they should have an aria-label or an aria-labelledby="<id to some sort of label, i.e. a heading>".

https://www.aditus.io/patterns/multiple-navigation-landmarks/

[ScarletKuro]

@bunkaisatori @zola-25 this change will "fix" the doc examples, can you confirm aside > nav > link is acceptable in terms of accessibility?

Maybe @KamilBugnoKrk could help too. That person contributed a lot in accessibility in past. (there is also a blog post https://blog.kamilbugno.com/web-accessibility). But if I read the blogpost correctly, the nav change is correct.

[henon]

@KamilBugnoKrk would you lend us your expertise on accessibility please? Would be greatly appreciated.

Edit: Haha, @ScarletKuro we thing alike :)

[zola-25]

@igotinfected @ScarletKuro

However, according to MDN (and other frameworks), having navigation blocks inside of aside is not an issue, but navigation blocks should always be inside of a nav tag. The logical solution for me then is to have MudNavMenu translate into nav rather than div.

Yes, this was a problem too, probably more significant. As you've already mentioned, the work around involved passing "role"="navigation" as UserAttributes to a <MudPaper> (so a <div>) that wrapped the <MudNavMenu>

Work around

I think the main thing to make sure with <aside> is that it's not supposed to sit within <main>? Which it isn't in the layout I'm using anyway. But apparently it's okay if within an <article>. It's very confusing, because it has been used in navigational sidebars, and it's really not clear if this is appropriate when these are main site nav links:

Stackoverflow best-html5-markup-for-sidebar
Stackoverflow sidebar-navigation-which-tag-is-semantically-correct

It also gets confusing when it is used in responsive sidebars. Since, when collapsed, the drawer/sidebar doesn't set any styles like visibility: hidden and display: none, it still shows up okay in the accessibility view in chrome, so that's good. But whether the <nav> should be inside a collapsible <aside> I'm not sure.

An <aside> without a role attribute set is a 'complementary' landmark, but an aside with role="region" equivalent of a <section> semantically:

Complementary WAI ARIA

And from Region WAI ARIA:

A region landmark is a perceivable section containing content that is relevant to a specific, author-specified purpose and sufficiently important that users will likely want to be able to navigate to the section easily and to have it listed in a summary of the page.

Does any of this matter?

Are screen readers adaptable given such variations across the web? I have no idea. Tools like Axe and Lighthouse don't complain about MudBlazor's use of <aside> in the drawer.

I set <aside> 's "role"="region" but I can't remember exactly why.

<aside> and the lack of <nav> in the MudBlazor drawer menu only became an issue when using Microsoft Playwright and its semantically-driven element locators for Web UI testing. They have gone all in on it recently and I think Selenium are also catching up. When you can't identify the <nav> section , <header> and <footer>, form inputs by <label>, that kind of thing, it becomes super hard to use these kind of semantic element locators for testing.

That's about all the info I've got. Hope it helps in some way.

But aside from these semantic HTML/ARIA issues, would just like to say thanks for this awesome library and the work you put into it. 👍💪

A few months ago I finished my hobby project with it: https://demo.explainmyenergy.net

[igotinfected]

@zola-25 thank you for the super detailed response!! I've started working on accessibility in navigation that goes beyond this issue here: #8684

That on its own should solve a few accessibility issues including keyboard navigation.

In that implementation I put the aria-label on the MudNavGroup container instead of MudNavMenu which might not be the best approach, maybe your idea to use role="region" is better.

The collapsible navigation groups are now also marked as aria-hidden when collapsed which should clean up the accessibility tree.

This will most likely be an iterative process anyway given how much there is to do and how little documentation there is on accessibility from an implementation point of view.

only became an issue when using Microsoft Playwright and its semantically-driven element locators for Web UI testing

I am happy we have dedicated test engineers for this at my work place, it seems like a pretty complicated topic.

[ScarletKuro]

I assume this one is safe to merge @henon

[henon]

A few months ago I finished my hobby project with it: https://demo.explainmyenergy.net

@zola-25 If you want you can add your project to our show-and-tell-issue: #2849

[henon]

Thanks Jason