Global styles revisions: update text color contrast #58340
Merged
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
ramonjd
added
[Type] Enhancement
|
Should we use a solid hex color, e.g., ? I can't seem to find a suitable background color from the existing base CSS vars. But we could add one. |
|
Size Change: +47 B (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
|
The alpha is used in a variety of places. If we replace it with a var (solid color or not) that might be one to handle in a dedicated PR?
Do we need this text? Isn't it implied that the most recent item in the timeline is the one that's applied? |
It's also possible to have other revisions that match current styles, e.g., if folks are using theme style variations or resetting and saving a lot. There was nothing there before, and I think the text was added to explain the absence of an "Apply" button. Then we tried a disabled Apply button, which seemed useless and confusing. Is there some semiotic or other way to indicate this? Happy to get a PR up to remove the text if it doesn't provide any value. Would we still need some accessible label to indicate to folks using screen readers that the revisions can't be applied because it matches the current editor styles?
Good plan! |
|
Ah, thanks for explaining. Given the timeline-esque nature of the UI, I find it a little confusing that it's possible for an 'older' item to essentially be marked as 'current'. It's also strange how clicking "Apply" sends you back to the Styles panel home rather than staying in revisions. These are all separate issues though, sorry for the distraction :) |
All good! All ideas are welcome. 😄 Thank you!
Yeah, fair point. You know what, another way we could do it is to allow "Apply" for all revisions (except the first one at the top of the list). The consequence however is that "Apply"(ing) an identical version won't have any effect visually, assuming the user expects something to happen. 🤷🏻
"Apply" is really saying "Reinstate/restore this version into the editor" - I took a bit of inspiration from Google docs revisions in that regard. Google docs however does show a confirmation modal before redirecting back to the main document. I could try that. |
Playing around here: |
We should probably discuss in a dedicated issue, but I don't think that would be a problem so long as there was some confirmation that the styles had been applied (Snackbar?). |
jameskoster
approved these changes
Jan 31, 2024
I'll get an issue up. Thanks again 🙇🏻 |
ramonjd
added
the
Backport to WP 6.7 Beta/RC
youknowriad
removed
the
Backport to WP 6.7 Beta/RC
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?
Starter PR that one day resolves #58121
These styles are already applied to your site.Why?
Testing Instructions
Screenshots or screencast