Resolve "Edit palette" visual regression in Global Styles > Colors #66481
+19
−21
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
richtabor
added
[Type] Regression
|
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: 0 B Total Size: 1.81 MB ℹ️ View Unchanged
|
|
I don't understand why this PR is labeled as a regression. The reasons for needing a visual label seem to be explained well enough in #65112. It seems like a broader discussion is needed to improve this button, such as whether we need a five-color icon in the first place, where the randomizer should be, etc. |
I agree this PR should not be labeled as a regression. The changes from #65124 have been broadly discussed and agreed upon. @richtabor you're kindly asked to create an issue or reopen the existing one to discuss with other contributors any change you would like to propose. Thanks. |
afercia
added
[Type] Iteration
|
I changed this PR labels accordingly. |
afercia
reviewed
Oct 28, 2024
| </ZStack> | ||
| <FlexItem isBlock> | ||
| { __( 'Edit palette' ) } | ||
| </FlexItem> |
There was a problem hiding this comment.
Icon buttons do not have visible text.
afercia
requested changes
Oct 28, 2024
There was a problem hiding this comment.
This button must have some visible text, please see https://github.com/WordPress/gutenberg/pull/66481/files#r1818985456
How is this different from an Icon button, which has no visible text? |
|
I think it's clearly explained in the linked comment and linked referencesd there. |
|
I disagree. Please detail. |
cc @afercia. Can you provide appropriate context? |
I already pointed to the relevant context. I'm not sure repeating the same concepts over and over again is any useful. Specifically:
More specifically, this comment on the issue: #65112 (comment):
Icon buttons in Gutenberg visually expose their accessible name by the means of a tooltip. As mentioned previously in many issue and PRs, visually exposing the accessible name is required. Still, it's a compromise that has been implemented long time ago as a remediation to the inherent lack of accessibility of icon-only buttons. I wouldn't call it a full solution to the icon-only buttons problem, but at least it's something. I would appreciate contributors in this project to make an effort and actually document themselves about the history of the project, the decisions that have been made in the past, and the recommended best practices to be able to make informed decisions. It's not feasible that for every single issue other contributors are asked to provide arguments for their feedback when the same feedback has been already provided several times only because other contributors don't have the knowledge to even understand the issue that is being discussed.
Overall, I think this PR should be closed as there is no 'regression' and I'd recommend to focus on the removal of the color swatches instead, as proposed in #66169 |
Why wouldn't we add system-level affordances like a tooltip, to the button then? If that's the issue? We're serially over-complicating and over-engineering, rather than exploring simple improvements. |
I would appreciate a much higher bar for design across this application. Arbitrary decisions don't get far. Yes, I actually do expect every contributor to provide arguments, examples, and any other necessary details, to support their ideas. How can we ensure quality work otherwise? Everyone should be held to this standard, regardless. And last, please avoid assuming that others lack knowledge or don’t care enough to engage thoughtfully. Comments like these often come across as dismissive and unproductive. |
|
We actually agree on many points, but often the proposed solutions lack cohesive design integrity. While the intent behind your solutions might be good, the execution often is inconsistent, or misaligned with the overall design system. A more thoughtful approach to design would elevate the quality of the application and ensure that solutions are not only functional (i.e. accessible) but also experientially cohesive. I recommend pairing with a designer early in the process, conducting research to validate ideas, and leveraging established design systems or guidelines to ensure consistency. Regular design reviews and collaborating with cross-functional teams can also help refine solutions and align them with the broader vision of the application. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
A visual regression from #65124. This is a non-standard pattern that does not scale wise, especially considering i18n. The previous state is an improvement.
Testing Instructions
Screenshots or screencast
Before
After