-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Global styles menu: Avoid visible labels and accessible names mismatch #65124
base: trunk
Are you sure you want to change the base?
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Size Change: -280 B (-0.02%) Total Size: 1.77 MB
ℹ️ View Unchanged
|
fa6ba5d
to
a1c3577
Compare
Happy to help with the code review, but I think this also needs design feedback. cc @WordPress/gutenberg-design |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Special case: Adds a visible label to the 'Palette' button. Note: this is the only visual change in this PR.
This does not following existing UI patterns. I'd encourage not introducing new UI for existing concepts.
The current label made of 'palette icons' of the control is already a unique case that doesn't provide a choesive experience for users. It's not accessible and to me it's not a great design. I see two options:
I'd like to be very clear that controls labeled with only icons must be avoided, especially when the icons are not recognizable and can't be mentally associate to the name of the control.
Any feedback is welcomed but saying 'No' without providing an alternative solution to a valid issue is not acceptable and not welcomed. I think I already pointed out on other issues and PRs that this kind of feedback doesn't help in any way to make WordPress better and doesn't help collaboration in an open source project. Thanks. |
d4c3e20
to
401ea48
Compare
In the last commit I'm entirely removing the 'palette icons' and only using visible text for the label. To recap: these icons used as label illustrated in the screenshot below are not OK. This is not the kind of accessible, inclusive, design I'd liek to see in WordPress: The control is now labeled with visible text only: As a consequence, the 'Theme colors randomizer' button that is shown when enabling the 'Color randomizer' Gutenberg experiment is now moved one level deeper into the 'edit palette' panel. Screenshot: |
One more good reason to move the 'Randomize color' button to the actual Edit palette panel is that the colors icons shown inside the 'Edit palette' button are only the first five colors provided by the theme. Depending on the theme palette, clicking 'Randomize color' may not show any visual change in the five colors icons. For example, in Twenty Twenty-Four the first five colors are all white, black or grays and clicking the Randomize button won't show any change in the colors icons. |
401ea48
to
b4f1f83
Compare
Fixes #65112
What?
Removes most aria-label that:
Why?
In such cases, the labels are unnecessary or even harmful for accessibility. Always prefer the visible label to determine the accessible name.
How?
title
attribute.Screenshot of the Palette butto, before and after:
Testing Instructions
aria-label
attribute.Testing Instructions for Keyboard
Screenshots or screencast