Use admin colors in CanvasLoader ProgressBar #54611
Closed
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
changed the title
|
Size Change: -65 B (0%) Total Size: 1.62 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 00c48bc. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6237651901
|
|
Thanks for the PR, as noted this was originally my instinct as well, however I think relying on system colors is going to be equally frail once we get the real style color to consistently load as the background color for the preview. To that end, before we land this I would love to hear from @tyxla and @mtias who originally requested themecolors for this particular instance. |
I think the only way this works, would be to use variations of the text color as the theming values for track and fill. The accent color doesn’t quite work for TT3 or TT4 themes, which are technically using the correct background color (as they’re white(ish)). Think that’s something we can get in for 6.4? |
tyxla
reviewed
Sep 21, 2023
There was a problem hiding this comment.
I personally prefer the colors as they are right now because the admin colors don't necessarily look well in the theme context in the background. There is no solution that works and looks great 100% of the time and in all use cases, and in that case, I prefer the one that matches the context. This is just my personal opinion of course, and I'm happy to disagree and commit if the general design direction suggests otherwise.
At the same time, one can argue that the progress bar is part of the interface, and as such, it should use the interface colors. It's a fair claim, although, in my experience, I don't find it so appealing to the eye when testing it with various themes and style variations.
I recall in a conversation with Matias that the motivation for using theme colors was that they fit better with the theme backgrounds. I also recall him saying something along the lines that we can afford to use theme colors with a new component (which ProgressBar essentially is), but for Spinner we had to use the admin colors, because it was already being utilized in other places.
I'd prefer leaving the final call to @mtias if he can chime in on this one since I mostly translated his ideas into code for the site editor loading the progress bar.
|
I would echo to not change to admin colors until we have a chance to try and fix the current system as it was intended. I think there's a path forward as described here: #54202 (comment) |
|
Closed in lieu of #55285 |
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?
Closes #54202.
Instead of pulling colors from Global Styles (which is unpredictable, as noted in the original issue), editor interface elements should be using editor colors.
I also noted that the 1.5px height bar was shifting between 1.5 and 1px at varying viewport sizes, so I added a quick fix here to set it to 2px. Makes it more visible as well.
How?
Testing Instructions
Screenshots or screencast