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

Block bindings: Fix accessibility of the 'Connected' visual indicator in the block toolbar #60810

Open
afercia opened this issue Apr 17, 2024 · 6 comments
Labels
[Feature] Block bindings [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Type] Bug An existing feature does not function as intended

Comments

@afercia
Copy link
Contributor

afercia commented Apr 17, 2024

Description

Spltting this out from https://github.saobby.my.eu.orgsically/WordPress/gutenberg/issues/58595#issuecomment-2060646354

The 'connected' visual indicator added to the block toolbar is not OK. It's a focusable div element with an aria-label attribute, which is not OK and it's something that was already pointed out several times as a bad practice. Never use a focusable div unless it reproduces all the native semantics and behaviors of the corresponding HTML element that is supposed to replace.

Additionally:

  • It doesn't show a focus style, although it is focusable.
  • It doesn't show a Tooltip.

It should be changed to an aria-disabled, nooped, button. It should show a tooltip.

As mentioned in several other issues and PRs, please remind:

Important

Never ever use focusable div elements.

Step-by-step reproduction instructions

  • Use the code snippets provided at Block Bindings: Improve accessibility for non-editable, bound fields #58595 (comment) to have a few bound blocks in your post.
  • Select one of the bound Paragraphs.
  • Use a keyboard and press Shift + Tab to move focus to the block toolbar.
  • Use the left and arrow keys to move through the block toolbar buttons.
  • Observe the 'Connected' indicator receives focus but it doesn't show a focus style.
  • Observe the 'Connected' indicator doesn't show a tooltip.

Animated GIF to illustrate:

connected div button

Screenshots, screen recording, code snippet

No response

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 [Type] Bug An existing feature does not function as intended [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Feature] Block bindings labels Apr 17, 2024
@SantosGuillamot
Copy link
Contributor

The "Connected" icon is not used anymore and instead, the block icon is reused adding a message saying "Block is connected". I'm pretty sure that there are more accessibility improvements we need to work on, but should we close this specific issue as it doesn't seem to apply anymore?

Screenshot 2024-06-27 at 13 24 05

@afercia
Copy link
Contributor Author

afercia commented Jun 27, 2024

Late to the party but I'm afraid the text This block is connected placed at the end fo the menu breaks the ARIA menu pattern.

A role=menu is only expected to contain children with role menuitem, menuitemcheckbox, menuitemradio and, optionally, groups with separators.

The textThis block is connected. is just a <span> element placed within a role="group". A menu group is not expected to contain plain text

Also, the arrows keyboard interaction, which is the expected interaction for a menu, will never reach this text as it works only for items in the menu. Screen reader users will have very few chances to discover this text at the bottom of the menu as they wouldn't expect it to be there.

I haven't followed the discussion where this design was provided but there have been already other cases of extraneous content in a role=menu and they have been removed. I'd kindly ask to remove this text from the menu to preserve the ARIA menu pattern expected semantics.

I'd argue that also from a visuals perspective, burying down in a menu such important information isn't ideal
I'd tend to think it's unlikely users will ever expect the 'connected' information to be inside the 'Transform to' It can't be added in the block toolbar as well. As an alternative solutoin, I'd sugget to explore placing this information in the inspetor block card.

@SantosGuillamot
Copy link
Contributor

Thanks a lot for the feedback and sorry for introducing those issues.

If I am not mistaken, that was introduced in this pull request by @kevin940726 and the discussion happened here.

@richtabor @jasmussen As you were involved in that conversation, maybe you can help here. Any alternative to the "The block is connected" text, which is causing accessibility issues?

@jasmussen
Copy link
Contributor

It sounds like it's the placement in the DOM of the text that's causing issues, not the content of the text. The pattern is similar to this one:

Screenshot 2024-07-03 at 09 47 06

Should that menu be fixed too?

@kevin940726
Copy link
Member

As an alternative solutoin, I'd sugget to explore placing this information in the inspetor block card.

Happy to explore this if we think it'd be better for accessibility. It'd be better to have a unified way to display such text as shown in the edit mode popover too though!

@afercia
Copy link
Contributor Author

afercia commented Jul 4, 2024

The pattern is similar to this one:

Visually, it's similar. Semantically, it is not. That text is placed outside of the role=menu:

Screenshot 2024-07-04 at 09 55 52

It's unlikely screen reader users will ever get that text though, unless they switch their scren reader to browse mode and manually explore the content of the popover.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Block bindings [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

4 participants