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

The ContentOnlySettingsMenu breaks the ARIA menu pattern #65514

Open
2 tasks done
afercia opened this issue Sep 20, 2024 · 5 comments
Open
2 tasks done

The ContentOnlySettingsMenu breaks the ARIA menu pattern #65514

afercia opened this issue Sep 20, 2024 · 5 comments
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Components /packages/components [Package] Editor /packages/editor [Type] Regression Related to a regression in the latest release

Comments

@afercia
Copy link
Contributor

afercia commented Sep 20, 2024

Description

#62354 introduced a ContentOnlySettingsMenu component that is then used in a few places in the editor. This component adds a menu item and a paragraph with some extr ainformation inside a few dropdown menus.

The dropdown menus are ARIA menus. They can only contain specific roles and they cannot contain paragraphs.

There have been ohter cases in the past where extraneous content was added inside ARIA menus and they all have been raised as issues and fixed. I'm surprised extraneous content has been added to ARIA menus again as this has been reported a few times already for previous cases. See for example:
#58313
#13390
#12505
#5953

It's worth reminding:

Important

An ARIA menu can only contain elements with role group, separator, menuitem and the menuitems variants. It can not contain anything else.

Reference:

https://www.w3.org/TR/wai-aria-1.2/#menubar
Required Owned Elements:

  • group → menuitem
  • group → menuitemradio
  • group → menuitemcheckbox
  • menuitem
  • menuitemcheckbox
  • menuitemradio

The presence of the paragraph with extra information:

  • Breaks the ARIA pattern.
  • Screen reader users can't reach this paragraph. As such, this information isn't available to them. Tabbing or arrows navigation skips the paragraph and only navigates through the menu items. In order to reach the paragraph, screen reader users would have to exit 'focus mode' and use hte built-in arrows navigation of the screen reader virtual cursor mode, which is totally unexpected.

Note: For the ones that only test with VoiceOver on macOS, the behavior described above only happens with Windows screen readers. VoiceOv er has a different interaction model which is, in a way, non-standard.

Also important:
I seem to recall there were checks in place to make sure an UI with role=menu only contains the expected roles. Not sure if something has changed in that regard or maybe the checks I'm thinking at are specific to some other component. Cc @youknowriad

Regardless, this extraneous content inside the ARIA menus must be removed as it totally breaks the expected semantics and interaction.

From a design perspective, I'd argue this isn't a great design pattern as some important information is buried down inside a dropdown menu, which is just a good way to hide important information,

@WordPress/gutenberg-components given this happened a few times already, it appeaers to me there should be some kind of mechanism to ensure an ARIA menu doesn't contain extraneous content. Is there any way to make sure an ARIA menu only contains menu items? Thanks.

Cc @ramonjd @talldan @kevin940726

Step-by-step reproduction instructions

To see an example of this pattern follo the steps below. Other example are in the screenshot in the original PR #62354

  • Edit a post and add a Query Loop block.
  • Choose a pattern for the Query Loop block e.g. 'Standard`.
  • Once the block renders itis content, click 'Options' in the block toolbar.
  • Observe the dropdown menu contains a paragraph with some extra information.

Screenshots, screen recording, code snippet

Screenshot 2024-09-20 at 09 18 07

Environment info

No response

Please confirm that you have searched existing issues in the repo.

  • Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

  • Yes
@afercia afercia added [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Type] Regression Related to a regression in the latest release [Package] Components /packages/components [Package] Editor /packages/editor labels Sep 20, 2024
@ciampo
Copy link
Contributor

ciampo commented Sep 20, 2024

given this happened a few times already, it appeaers to me there should be some kind of mechanism to ensure an ARIA menu doesn't contain extraneous content. Is there any way to make sure an ARIA menu only contains menu items? Thanks.

This kind of runtime checks are not easy to write, can be costly and they are not perfect — developers who really want to do something usually find a way regardless. Especially if the "invalid" items are injected via a slot/fill.

The best solution, IMO, is to add e2e tests that check these menus

@kevin940726
Copy link
Member

The accessibility improvement is meant to be an immediate follow-up, but I guess we never get to it 🤦. I think we could implement some special cases for screen readers, if we want to keep the design intact.

The best solution, IMO, is to add e2e tests that check these menus

I think we can run an automatic accessibility check via Axe (if they can catch this sort of bug). We might actually use to do this with Puppeteer, but probably left out in Playwright 🤔.

@afercia
Copy link
Contributor Author

afercia commented Sep 23, 2024

I think we could implement some special cases for screen readers, if we want to keep the design intact.

The design is just wrong and it's not accessible. It needs to be changed. The arguments in this report are valid. The UI needs to be designed in an accessible way to start with, and breaking patterns isn't the best way to do that.

@kevin940726
Copy link
Member

Curious to hear what @jameskoster thinks about the alternative UI 🙏.

@jameskoster
Copy link
Contributor

This feature doesn't seem to be working as intended any more. I see the paragraph and associated actions spread throughout the menu:

Screenshot 2024-09-26 at 12 24 14

This makes the experience more confusing.

It may be simplest to remove the text for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Components /packages/components [Package] Editor /packages/editor [Type] Regression Related to a regression in the latest release
Projects
None yet
Development

No branches or pull requests

4 participants