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

Navigation block accessibility: Understanding the different submenu configuration options #51226

Closed
luisherranz opened this issue Jun 5, 2023 · 16 comments · Fixed by #52251
Closed
Assignees
Labels
[Block] Navigation Affects the Navigation Block [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Type] Question Questions about the design or development of the editor.

Comments

@luisherranz
Copy link
Member

I'm no expert in accessibility but, right now, for reasons that I don't understand, the showSubmenuIcons are forced to be true when openOnClick is true. In that scenario, the link label becomes a button that contains aria-expanded, so there is no real need for arrows, other than for aesthetical reasons. But when openClick is false, the link remains a link (<a>) and without the arrows, there is no button and therefore no aria-expanded.

  • openOnClick = true
    • showSubmenuIcons = true
      • Link label: a button element with aria-expanded
      • Arrow: a clickable span element
      • Submenu opens on click on the link label or arrow
    • showSubmenuIcons = false
      • Link label: a button element with aria-expanded
      • Arrow: not rendered
      • It's not possible to choose this configuration, but the submenu would open on click on the link label.
  • openOnClick = false
    • showSubmenuIcons = false
      • Link label: an a element (with no aria-expanded)
      • Arrow: not rendered
      • Submenu opens on hover and focus.
    • showSubmenuIcons = true
      • Link label: an a element (with no aria-expanded)
      • Arrow: a button element with aria-expanded
      • Submenu opens on hover or click on the arrow

The openOnClick=false && showSubmenuIcons=false option seems bad not only for accessibility but also for mobile UX because there's no way to open a submenu on tap.

So maybe I'm focusing on the wrong things, but I'd like to understand why it's not possible to set openOnClick=true && showSubmenuIcons=true (which to me seems fine), and why it's possible to set openOnClick=false && showSubmenuIcons=false (which to me seems wrong).

cc: @tellthemachines, @jeryj, @alexstine

@luisherranz luisherranz added [Type] Question Questions about the design or development of the editor. [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Block] Navigation Affects the Navigation Block labels Jun 5, 2023
@alexstine
Copy link
Contributor

I can't really speak to the specific settings of the navigation block, I think the block has been pretty confusing from the start. Here is the simple version:

  1. If there is a submenu, button with aria-expanded should exist in the DOM.
  2. If there are no submenus, top-level links should exist in the DOM but no submenu toggle button.

Very simple. If there is ever an event where a submenu can exist without a trigger button, this is a bug and should be fixed now.

Hope this helps. Thanks.

@tellthemachines
Copy link
Contributor

the showSubmenuIcons are forced to be true when openOnClick is true. In that scenario, the link label becomes a button that contains aria-expanded, so there is no real need for arrows, other than for aesthetical reasons.

I don't fully recall the ins and outs of this; the implementation was in #33775 but there may have been previous discussion spread across other issues. The gist of it is: open on click-only submenus do need some visual indication that the menu item is a button that opens a submenu, as opposed to a simple link. Whether that should be an arrow or not, there should be some guard rails in place so that users can't create a menu where buttons are visually indistinguishable from links. I think that the arrow was considered to be a reliable visual indicator of open on click functionality (in that it's a recurring pattern on websites).

The openOnClick=false && showSubmenuIcons=false option seems bad not only for accessibility but also for mobile UX because there's no way to open a submenu on tap.

This was recognised at the time, but provided mainly for back-compat reasons, because that's the standard behaviour of navigation menus in classic themes (most older default themes have it). Iirc at this point back compat was a concern because we were working on the Navigation screen, which was to be a block-based rebuild of the classic theme menus screen, so would be dealing primarily with classic menus. That project ended up being scrapped, so perhaps if we were building this today things would have gone differently.

I hope this helps! @jasmussen might have further context to add here.

@alexstine
Copy link
Contributor

@tellthemachines Is there anyway to detect that old legacy configuration and force users into a more accessible configuration?

@luisherranz
Copy link
Member Author

open on click-only submenus do need some visual indication that the menu item is a button that opens a submenu, as opposed to a simple link

Ok, I think this is what we were missing. Thanks, @tellthemachines.


The only way that I can think of to maintain the option to set showSubmenuIcons = false when open on click is false and solve both the keyboard accessibility and the mobile UX problems at the same time would be to use the first click/keyboard-input/tap to open the menu, and the second click/keyboard-input/tap to navigate to the link. Is that a common pattern?

Another option could be to use hidden buttons for the screen reader, similar to the "Skip to button" link. But that doesn't solve the mobile UX problem.

@alexstine
Copy link
Contributor

@luisherranz What I am saying is, if we spot that those options are both set to false, we should upgrade to a more accessible experience and disallow users from having these settings. WordPress should not allow this easily for users to create inaccessible websites. I understand this is legacy so my thought is to somehow fix the experience going forward. Thoughts from @joedolson or @afercia might help us figure out how to move forward. We also cannot have a hidden button to screen readers because it will be an empty tab stop and as you already mentioned, this wouldn't help mobile users anyway. Having a click on the link would be okay if the link itself also did not go to a page. Trying to communicate to a user that a link opens a submenu and goes to a page is too confusing, that's why we use trigger buttons for the submenus.

Remember:

  • Button: In-page action such as opening a submenu.
  • Link: Go to another page, change of context, with the exception of skip links. :)

Thanks.

@tellthemachines
Copy link
Contributor

@tellthemachines Is there anyway to detect that old legacy configuration and force users into a more accessible configuration?

That legacy configuration only exists in classic themes, so it's not a concern for the Navigation block. We only thought it was at the time because we were trying to build a block-based screen to edit classic theme menus with, but that's not happening any more.

However, now that the Navigation block has been in WP for a few versions, some folks will be using that openOnClick=false && showSubmenuIcons=false configuration on their websites already so we have to provide back compat for existing instances even if we decide to remove the option for new menus. Fwiw that configuration still technically keyboard accessible (submenus open on Tab and then you have to Tab through all the links in them) but the experience is terrible.

@alexstine
Copy link
Contributor

@tellthemachines

However, now that the Navigation block has been in WP for a few versions, some folks will be using that openOnClick=false && showSubmenuIcons=false configuration on their websites already so we have to provide back compat for existing instances even if we decide to remove the option for new menus.

Why can't we deprecate the option and force users to a recommended configuration? If devs want it around so bad, they can use a hook.

I understand it's technically accessible but that would be using accessible very loosely.

It is my thinking that the navigation block should not allow this behavior. If we do allow it, we're really no better then all the page builders that let users do terrible things with their sites. This should have never been an option in Core but we can't change the passed, we can only change the future.

Thoughts?

@joedolson
Copy link
Contributor

I don't think it's trivial to remove support for that combination of characteristics; the back compatibility could be pretty significant. Essentially, because all menus pre-block editor had those characteristics, removing them could break those menus in significant ways - either layout (by adding an icon that takes up additional space) or by breaking existing scripting that does it's own changes to structure (which could include changing the top level item into a button or injecting an additional button, etc.)

Any change to output without direct user intervention could be disastrous. The best we can do is probably add a flag in the block that lets somebody know if the settings they're using aren't recommend, allows them to switch them, and then allows them to restore them if it causes problems on their site.

We can't forget that WordPress didn't support either of these options natively until extremely recently, so users and developers have implemented dozens of variations on menus to make them work during that time. Any or all of those customizations could break if we make those changes.

@jasmussen
Copy link
Contributor

One thing we could do is show a big scary warning in context of the option, like we do for missing color contrast.

@alexstine
Copy link
Contributor

That would be a nice compromise.

@luisherranz
Copy link
Member Author

Perfect. Thanks, folks. I'll take care of that 🙂

like we do for missing color contrast

@jasmussen, may I know where that is located to take a look and do something similar in terms of tone and style? Thanks!

@jasmussen
Copy link
Contributor

If you open the post editor (though the warning is present also in the site editor, anywhere you can mess up color), and set as in the following example, a gray text color on a light gray background, then a contrast warning will show up inside the Color panel:

screenshot of the post editor showing the inspector with a color contrast warning after applying gray text on a light background

Here's what the warning says:

This color combination may be hard for people to read. Try using a brighter background color and/or a darker text color.

For the navigation menu, we can be similar or even more verbose, ideally explaining precisely why it's a bad option to choose. I would appreciate advice on the exact phrasing here.

@luisherranz
Copy link
Member Author

Perfect, thank you!

I would appreciate advice on the exact phrasing here.

Me too 🙂

@joedolson
Copy link
Contributor

Rough draft; could certainly use refinement.

"The current menu options offer reduced accessibility for users and are not recommended, Enabling either "Open on Click" or "Show arrow" offers enhanced accessibility by allowing keyboard users to browse submenus selectively."

In writing this, I realized that we might want to modify the text of the 'Show arrow' label, as well. There's nothing about that to help users understand that the arrow is a button, and not just a graphic element. It's really about showing the submenu button, which happens to use an arrow.

@luisherranz
Copy link
Member Author

There you go:

I'd appreciate feedback on when the screen reader text should be triggered.

@luisherranz
Copy link
Member Author

I've just merged the PR with the notice. Thanks all for your feedback and ideas.

@priethor priethor removed the [Status] In Progress Tracking issues with work in progress label Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Navigation Affects the Navigation Block [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Type] Question Questions about the design or development of the editor.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants