cds-snc / notification-planning

Project planning for GC Notify Team
5 stars 0 forks source link

Mobile menu. Name Role Value #1466

Closed amazingphilippe closed 3 months ago

amazingphilippe commented 9 months ago

Description of issue

Pop-up menus There are several buttons that use the aria-haspopup attribute. A few things to note about aria-haspopup: When aria-haspopup is present, Safari does not communicate the value of the aria-expanded attribute (aria-haspopup attribute support). This can be observed in the “Your account” menu. For mobile navigation, which opens a dialog as the navigation menu, an aria-haspopup="dialog" is more semantically accurate than aria-haspopup="true" which by default communicates this attribute as having a value of “menu”.

Rather than making these changes, though, it might be worth considering changing the mobile navigation to use the ARIA disclosure menu pattern: a button with the aria-expanded attribute can be used to toggle the display of the menu, and the aria-expanded attribute will convey the state of the menu. This also makes it simpler to open/close the menu, as the same button can be used for both operations.

Potential fix

Resources

YedidaZalik commented 5 months ago

Will continuing to migrate from overlay pattern to new menu pattern for mobile

whabanks commented 5 months ago

Phil reviewed the PR, left some comments to address. Need to figure out what is going wrong in the Cypress tests and this should be good to merge / complete.

amazingphilippe commented 4 months ago

QA'd and noticed we're missing focus styles on menu buttons:

The screenshots show some kind of faint default focus style. We'll just need to apply our usual thick yellow outline or a yellow background with focus:bg-yellow or focus:outline-yellow

Capture d’écran, le 2024-05-28 à 15.13.43.png Capture d’écran, le 2024-05-28 à 15.13.48.png

I can write up a PR to neaten things up!