brand icon indicating copy to clipboard operation
brand copied to clipboard

[a11y] `SubNav` accessibility issues

Open stamat opened this issue 5 months ago • 1 comments

  1. Focus trap activates even when there is no submenu or popup https://github.com/primer/brand/issues/761
  2. On mobile, once you enter into a focus trap in a submenu, there is no way to reach the close button if you are using your keyboard. The only way to exit the focus trap is by pressing Esc. Fixing the focus trap issue and rethinking the layout will provide a solution for this. I think focus trap needs to include the item to close the menu.
  3. aria-current="page" should not be a link if it has a submenu it should be a button, if it doesn't have a submenu, if it doesn't have a submenu it can remain a link since it has the aria-current="page". Submenu toggles have a button that activates them which is separate from the link. I think it should all be one button. We should basically discourage the usage of # value for the href.
  4. Submenus don't have their own focus trap and you cannot exit them by pressing Esc.
  5. Mobile dropdown menu toggle button maybe needs an aria-haspopup="menu"? Maybe also aria-controls="ID" if we perform the restructure.
  6. Mobile menu pushes the content down on tablet sizes when toggled on, but then on mobile the whole SubNav becomes absolutely positioned - which is an inconsistent behavior. I suggest that the nav items only are always absolutely positioned on tablet/mobile sizes and the SubNav containing element is positioned relatively. We can always change it's positioning in custom scenarios.
  7. We can maybe have the ul element or nav labeled by the nav heading with aria-labeledby="ID"? Or maybe this is automatically understood since the heading is inside the nav element? We should check how the screen reader reads this. This might be useful if we restructure the SubNav a bit to accommodate for other issues.
  8. Not entirely an a11y issue, but can we have an option for the heading to choose the element type like with an optional attribute as like we have on some other components. Could be useful! ✨

stamat avatar Sep 19 '24 10:09 stamat