Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revisit accessibility of main nav menu #137

Open
terrill opened this issue Feb 10, 2020 · 6 comments · Fixed by #149
Open

Revisit accessibility of main nav menu #137

terrill opened this issue Feb 10, 2020 · 6 comments · Fixed by #149

Comments

@terrill
Copy link
Contributor

terrill commented Feb 10, 2020

The main "Dawgdrops" navigation menu was build with accessibility in mind, but accessibility methods related to navigation menus have evolved since then. The current menu needs the following changes, which I've grouped into two phases:

Phase 1
The following changes don't require any changes to HTML structure of the menu; therefore should be fairly easy to make I think.

  • Remove role="application" from div.dawgdrops-inner
  • Remove aria-haspopup="true" from each top-level menu item (<a>)
  • Remove role="group" and aria-expanded from the <ul> that contains each submenu
  • Currently, when a top-level menu item has keyboard focus, the corresponding submenu can be opened by pressing either Enter or down arrow. Remove support for down arrow for this purpose. If no submenu is open, users should be able to use the down arrow for its default browser function (which for most browsers is scrolling the page down within the viewport).
  • If a user presses the Escape key when focus is anywhere within the <nav>, all open submenus should close, regardless of whether they were opened using the keyboard or mouse. (Currently, if a submenu was opened with a mouse, it's impossible to close it with the keyboard).
  • If a user opens a submenu by hovering with the mouse, all previously open submenus should close, even those that were opened using the keyboard.
  • If a user opens a submenu by pressing the Enter key on a top-level submenu, all previously open submenus should close, even those that were opened using the mouse.
  • When a submenu opens, the aria-expanded attribute on the top-level menu item should change to "true". Conversely, when the submenu closes, the aria-expanded attribute should change to "false". This change should occur regardless of whether the submenu was opened/closed with keyboard or mouse (currently the attribute value only changes on keyboard events).

Phase 2
Currently, the top-level menu item is serving two roles. It serves the role of a link (<a>) that leads to a new page, but also as a button that triggers the submenu. This isn't allowed in HTML, and there are only two ways to solve it:

  1. Change the top-level links to <button> elements, so they serve the button role, but no longer link to other pages, or
  2. Add a <button>, adjacent to the link. There's already a visual indicator to the right of each link, so this could be accomplished with little or no change to the visual design. That visual indicator just needs to be a <button>.

I've been told in the past that option 1 is not an option: Those top-level links are not expendable. If that's still true, option 2 would be the one to implement. The following additional steps assume there is both a link and a button for each top-level menu item:

  • Move the aria-expanded and aria-controls attributes from the link to the button.
  • Add aria-labelledby to the button, with a value that matches the id of the link (e.g., aria-labelledby="academics").
  • Currently if the top-level menu item (the link) has focus, pressing Enter triggers the submenu, whereas pressing Space follows the link. Remove the Enter key event listener, so Enter and Space both follow the link.
  • Add keydown event listeners on the button, so pressing Enter or space when the button has focus both trigger the submenu.
lcaple added a commit that referenced this issue Jun 25, 2020
Dropdowns accessibility updates, resolves #137
@lcaple
Copy link
Contributor

lcaple commented Jun 25, 2020

(didn't realize Github would auto-close this)

Hi Terrill,

I've addressed most of this in the most recent pull request. I'm satisfied with how it functions for now, but the following needs to be revisited:

  • Toggle any/all open menus when another menu opens
  • Fine-tune the above behavior between the mouse and the enter button

There's some UX issues with overriding default functionality (i.e. don't undo a hover-state if your mouse is still actually hovering, etc.) but I think we can come to a happy medium with additional testing.

Thank you for your guidance in resolving these accessibility issues. It was extremely helpful and a huge improvement to the navigation.

@lcaple lcaple reopened this Jun 25, 2020
@lcaple
Copy link
Contributor

lcaple commented Oct 8, 2020

After a round of bug fixes I am marking this as resolved. We're noting the guidance on using buttons vs links for the main navigation and keep it in mind for future development.

@lcaple lcaple closed this as completed Oct 8, 2020
@terrill
Copy link
Contributor Author

terrill commented Oct 23, 2020

I discovered some additional bugs with keyboard interaction, hopefully easy to fix:

  1. If the menu has focus, and no submenu is open, right and left arrows should move between top-level menu items. It does so if focus is on a button, but not if it's on a link.
  2. If a sub-menu is open, pressing the first letter of any menu item should jump to the next item that starts with that letter. It currently works for the first letter only (for example, try "C" twice from the Academics menu on the home page).
  3. Related to the previous item: After you jump to an item with a letter, an up or down arrow key should take you to the previous or next item, relative to the item you just jumped to. That's currently broken.

@lcaple lcaple reopened this Nov 3, 2020
@terrill
Copy link
Contributor Author

terrill commented Nov 6, 2020

Specific steps for reproducing the latest issues (it's the same in all browsers, confirmed in Firefox on Mac OS and Chrome in both Mac OS and Windows):

Issue 1: Right/Left arrows not working when submenu is collapsed

  1. On the UW home page, tab to the "Academics" link in the nav menu (not the button).
  2. Press left arrow. Focus should move to the previous button (the one associated with "About"). Instead, nothing happens.
  3. Press right arrow. Focus should move to the next button (the one associated with "Academics"). Instead, nothing happens.

Issues 2-3: Up/Down arrows not moving to the correct target after jumping with a letter in submenu

  1. On the UW home page, tab to the button associated with the Academics menu.
  2. Press Enter to trigger the submenu.
  3. Press the letter "C". You should jump to "Colleges and schools". This works.
  4. Press the letter "C" again. You should jump to "Course descriptions". This does not work. Focus remains on "Colleges and Schools".
  5. Press the up arrow. You should move to the previous item ("Academic departments"). This does not work. Instead, focus jumps to another item in the menu.

What seems to be happening is the pointer to the user's current location in the menu does not change when the user jumps to a new item by pressing a letter. For example, if the user's focus is on the second item in the menu, then they jump somewhere else (e.g., by pressing "C"), the pointer remains on the second item. So when the user presses up arrow, they move to the first item in the menu, rather than the item immediately before their new location. The down arrow behaves the same way.

@terrill
Copy link
Contributor Author

terrill commented Feb 8, 2022

Some new accessibility bugs seem to have slipped into the dawgdrop menus, as observed in the new theme on both the Admissions and CISO websites. Here's an up-to-date list of problems and solutions:

  • A dropdown menu can be triggered with Enter or down arrow (this is correct). However, with a submenu visible, users should be able to navigate the submenu with up/down arrow keys. This no longer works.
  • With a submenu visible, users should not be able to tab or shift+tab through submenu items. The tab key should be reserved for navigating across the top.
  • Users can arrow left/right through the top-level menu items, which is good. However, if a submenu is open when they do this, the submenu should close.
  • Each top level menu item is coded as an <a>. Since it is used to trigger a submenu, it should be a button. For screen reader users, this can be accomplished by adding role="button". In both of the sites tested, the top-level menu is also a link (with an href attribute), but clicking on it triggers the submenu; it doesn't follow the link. So, the link itself is not accessible to mouse users either, which in my opinion is ok. The top level menu item cannot be both a link and a button simultaneously, and its role as a button is arguably the most important role (i.e., triggering the submenu).
  • For any top-level menu item (<a>) that has a submenu, it should have an aria-expanded attribute, and does (nice job). However, it currently also has aria-haspopup="true" but should not. That attribute is only applicable if this were coded with role="menubar", which it isn't (and shouldn't be).
  • Each sub-menu is coded as a <ul> with role="menu". The role attribute should be removed. It overrides the default role of the <ul>, which means all the list items inside it are orphaned and invalid. A submenu is really just a list of links; no additional ARIA required.

@lcaple
Copy link
Contributor

lcaple commented Feb 8, 2022

Hi Terrill,

Noting this is related to the new theme and adding it to our list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants