govau / design-system-components

🛠 Component code and tests for the Australian Government design system
https://auds.service.gov.au
MIT License
739 stars 111 forks source link

Main Navigation - Screen Reader Mobile view #574

Open HerinHentry opened 5 years ago

HerinHentry commented 5 years ago

Component: https://designsystem.gov.au/components/main-nav/

Being a responsive menu, the toggle feature expands and collapses the menu. Does this feature need aria-expanded and aria-haspopup?

Screen reader mobile content does not read the status change of the menu. Can you please review this? The expectation is, as it expands on 'enter key' it announces 'expanded' and 'collapsed'.

alex-page commented 5 years ago

Thanks Herin we will review this in the new year. Thanks for bringing it up :)

sarahpulis commented 5 years ago

Totally agree with you @HerinHentry. Adding aria-expanded and aria-haspopup would follow the ARIA design pattern for menu button.

@alex-page I noticed that DTA website and DTA marketplace have implemented the mobile menu as an accordion rather than a flyout as per the design system. This would be good to talk though before the update as it could have some bearing on whether we use aria-haspopup or not.

LaurenceRLewis commented 5 years ago

I agree with aria-expanded, however, do not include aria-haspopup. I recommend adding aria-controls.

‘Authors MUST ensure that the role of the element that serves as the container for the popup content is menu, listbox, tree, grid, or dialog’.

I also recommend wrapping the menu expand/collapse buttons in a div with a role of heading and aria-level. Recent discussions with a blind screen reader user who is also an accessibility developer found ‘If role heading and aria-level is implemented, expanding the subsection it is a clear indicator that the section starts at a particular point, so the heading is helpful for that plus the user can quickly jump between them using the H and Shift+H keys with or without the sections being expanded, and even use the rotor on iOS to swipe between headings using single swipe up/down gestures.

There are major issues using. VoiceOver on IOS. When trying to navigate the expanded menu on mobile the expanded menu cannot be accessed. VoiceOver moves through the background page and not the expanded menu—which is inaccessible.

https://www.w3.org/TR/wai-aria-1.1/#aria-haspopup

sukhrajghuman commented 5 years ago

Hi @Decrepidos thanks for sharing, I tried out the main nav on voiceover IOS and you are right. Despite there being an 'overlay' div with a high z-index, VoiceOver is still able to access elements behind the slide out menu. Is there a work around for this?

I'll start working on a new implementation with the new mark up.

LaurenceRLewis commented 5 years ago

@sukhrajghuman I will try and review the code tomorrow to find a solution for the menu.

LaurenceRLewis commented 5 years ago

@sukhrajghuman

A solution is to hide the background for AT users when the menu is open:

add aria-hidden="true" to the background when the menu is expanded and the overlay is visible.

$('.au-main-navtoggle au-main-navtoggle--closed').on('click',function(){ $('.page-wrapper').attr('aria-hidden', 'true'); }; $('.au-main-navtoggle au-main-navtoggle--open').on('click',function(){ $('.page-wrapper').removeAttr('aria-hidden'); };

This effectively does the same as the overlay which hides the background elements for visual users.

Other issues identified during current testing

The 'Australian Government official website' disclosure widget, when expanded, the next swipe moves me to the MENU button.

I believe this is caused by the method used to collapse / expand by changing the CSS height from 0 to auto; which seems to be tied into the display block / none somehow. If this CSS is turned off the disclosure breaks.

.js .au-accordionbody.au-accordion--closed { display: none; height: 0; } .js .au-accordionbody.au-accordion--open { display: block; height: auto; } I cannot find the script that runs this to offer a fix. I recommend changing this a using the method for a disclosure widget described by the ARIA Authoring Practices 1.1 https://www.w3.org/TR/wai-aria-practices/examples/disclosure/disclosure-faq.html

Implementation of aria-controls

The following does not do anything and can (should) be removed because a <div> cannot control anything so aria-controls is invalid.

<div aria-controls="au-main-nav-99" class="au-main-nav__overlay" style=""></div>

The TAB component HTML | REACT

The unselected tab is missing the aria-selected attribute. aria-selected must be set to false on the unselected tabs.

Hope this helps

Cheers Laurence

sukhrajghuman commented 5 years ago

Thanks @Decrepidos for all of this. We really appreciate it.

I will try your method to see if this solves our issues with main-nav.


The following does not do anything and can (should) be removed because a

cannot control anything so aria-controls is invalid.

I agree. Will change this.

The 'Australian Government official website' disclosure widget, when expanded, the next swipe moves me to the MENU button.

This is interesting I have opened up another ticket to investigate this.

The unselected tab is missing the aria-selected attribute. aria-selected must be set to false on the unselected tabs.

Yep nice pick up, have opened a ticket for this.

LaurenceRLewis commented 5 years ago

@sukhrajghuman you are welcome. If you need anything tested at anytime; please let me know. Happy to contribute.

AtamDhillon commented 3 years ago

@sukhrajghuman This was one of the issues I was gong to share with SM site ally findings. Good to see it already resolved.

sukhrajghuman commented 3 years ago

hey @AtamDhillon, The IOS bug where screen readers are able to view elements behind the main nav is still an open issue. I haven't been working on this project for a while so I wasn't able to get around to this.

The only fix that has been applied is adding aria-expanded to the the open/close button

LaurenceRLewis commented 3 years ago

Hi All Don’t know if this helps. I was looking into modal dialog accessibility and created a test CodePen.

https://codepen.io/Deafinitive/pen/GRNZKxG

sukhrajghuman commented 3 years ago

@rossmullen @adrian-yee @monicagee99

AtamDhillon commented 3 years ago

hey @AtamDhillon, The IOS bug where screen readers are able to view elements behind the main nav is still an open issue. I haven't been working on this project for a while so I wasn't able to get around to this.

The only fix that has been applied is adding aria-expanded to the the open/close button

Hi Raj, I was away yesterday, a quick resolve may be adding "aria-hidden = true" with open of dialog and remove when closed using js to the elements behind the dialog. It may require multiple elements including main region without restructuring the whole page architecture.