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

Improve the various Alignment controls props handling #63649

Open
2 tasks done
afercia opened this issue Jul 17, 2024 · 0 comments · May be fixed by #63696
Open
2 tasks done

Improve the various Alignment controls props handling #63649

afercia opened this issue Jul 17, 2024 · 0 comments · May be fixed by #63696
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Block editor /packages/block-editor [Type] Bug An existing feature does not function as intended

Comments

@afercia
Copy link
Contributor

afercia commented Jul 17, 2024

Description

Splitting this out from #63486 (comment)

Applies to:

These three components are very similar but they handle their props differently. which seems to introduce some confusion about how the label and description props are expected to be used. Also, these props aren't documented in the components readme and other props aren't documented as well.

AlignmentControl
It allows to pass both the label and describedBy props, to change the default values.

BlockAlignmentUI
Both the label and description are hardcoded. The values are Align / Change alignment, which seems redundant.
Also, the label Align doesn't communicate what will be aligned. It's inconsistent with the AlignmentControl which does clarify Align text. As such, it should be Align block.

BlockVerticalAlignmentUI
Used for xample in the Media & Text block, where the label Change vertical alignment is hardcoded and there's no description. The label is inconsistent wiht Align and Align text. It should be Align vertically.

Misleading descriptions

I'd think that if these controls would have clear and meaningful labels, they wouldn't need a visually hidden description in the first place. The lack of documentation and the fact the description is visually hidden leads to incorrect implementations. For example, in the Table block the alignment control passes a label prop with value Change column alignment. That makes sense. Unfortunately, the describedBy is still the default one: Change text alignment. Honestly, it's sad to see how the development process in this project often misses to check and test anything that is 'non visual'. The consequence is that the alignment button in the Table block has:

  • a label that says Change column alignment
  • a description that says Change text alignment

Screen readers will announce both the label and description, thus providing a very confusing information to usdrs. Screenshots:

Screenshot 2024-07-17 at 09 36 48

Screenshot 2024-07-17 at 09 31 31

Step-by-step reproduction instructions

  • Select a paragraph block.
  • Inspect the Align text button in the block toolbar.
  • Observe the button has an aria-describedby attribute that points to an element with the description Change text alignment. This description is a bit redundant.
  • Select a Table block.
  • Inspect the Change column alignment button in the block toolbar.
  • Observe the button has an aria-describedby attribute that points to an element with the description Change text alignment. This description is a misleading.
  • Select a Group block.
  • Inspect the Align button in the block toolbar.
  • Observe the button has an aria-describedby attribute that points to an element with the description Change alignment. Both the label and the description don't clarify what to align. The description Change alignment is redundant.
  • Select a Media & Text block.
  • Inspect the Change vertical alignment button in the block toolbar.
  • Observe the button does not have an aria-describedby attribute.
  • Observe the label Change vertical alignment is inconsistent with the other labels, to be more consistent it should say 'Align vertically'.

Screenshots, screen recording, code snippet

AlignmentControl in a Paragraph:

align text

AlignmentControl in a Table:

table ealign column

BlockAlignmentUI in a Group:

align block

BlockVerticalAlignmentUI in a Media & Text:

Screenshot 2024-07-17 at 12 36 38

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). [Package] Block editor /packages/block-editor labels Jul 17, 2024
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] Block editor /packages/block-editor [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant