ProgressBar: Use the theme system accent for indicator color

[tyxla]

What?

This PR updates the ProgressBar indicator color to use the theme system accent color. Changes are only observable when the ProgressBar component is used in conjunction with the Theme component.

Why?

Necessary to be able to properly and reliably style the progress bar for #53032 by using the @wordpress/components Theme system.

How?

We're just changing the ProgressBar indicator color to use the theme system accent color with priority, and only optionally fall back to the COLORS.ui.theme color.

Testing Instructions

• Run npm run storybook:dev
• Navigate to http://localhost:50240/?path=/docs/components-experimental-progressbar--default
• Verify the progress bar works and looks well.
• Play with the Theme controls (the pencil icon) and verify the combinations work well.
• Test it with Edit Site: Use progress bar for loading screen #53032.

Testing Instructions for Keyboard

None

Screenshots or screencast

None - there are no observable visual changes. They're observed only with #53032.

[mirka]

Curious, was this not working? The value is supposed to be pretty much the same:

gutenberg/packages/components/src/utils/colors-values.js

Line 34 in 904fd24

theme: 'var(--wp-components-color-accent, var(--wp-admin-theme-color, #3858e9))',

[tyxla]

Hmm, today I can no longer reproduce the scenario. But I'd still prefer to keep it consistent with the other color styles for this component. I realize it's very unlikely the underlying COLORS constant to change, but still, I appreciate that when looking at the styles of the progress bar component it's immediately clear what colors we're using, without having to peek at the COLORS definition. Will go ahead and merge, but let me know if you have any second thoughts about it.

[mirka]

Would it be better if we renamed the COLORS.ui.theme variable to something that evokes the underlying CSS variable, like COLORS.theme.accent? For example, in Sass we have it set up like this.

In any case I don't think it makes sense to do var( --wp-components-color-accent, ${ COLORS.ui.theme } ) because that renders an entirely unnecessary level of nesting:

var( --wp-components-color-accent, var( --wp-components-color-accent, var( --wp-admin-theme-color, #3858e9 ) ) )

[tyxla]

You're right about the level of nesting, I'd be happy to follow up and remove it. I just want to discuss the best approach to do it.

So, I think the problem comes from the fact that in the components package, we call it accent color, but historically we've also called that color just "admin theme color".

I'd be happy with the rename as you suggested it, but would it make sense for all the pre-existing COLORS.ui.theme use cases? With that concern in mind, should we just create an alias and keep both COLORS.ui.theme and COLORS.theme.accent? Or you'd be fine with just renaming all instances to the latter?

[mirka]

in the components package, we call it accent color, but historically we've also called that color just "admin theme color".

Right. It was intentional on our part to decouple it from the wp-admin theme color. And the components Theme system involves other colors like background color and grayscales as well, so we can't just call it "the theme color".

I'd be happy with the rename as you suggested it, but would it make sense for all the pre-existing COLORS.ui.theme use cases?

Yes 🎉 We haven't done the grayscale stuff yet, but the accent colors used in all our components are Theme-ready (#45289). Meaning, every use of COLORS.ui.theme and COLORS.ui.themeDark10 in our codebase is already hand-verified to look as expected in a Theme context. So it would be perfectly ok to rename them all to COLORS.theme.accent and COLORS.theme.accentDarker10 to better reflect the underlying CSS variable names.

FYI the colors-values.js file is not completely cleaned up yet. It's likely that we'll eventually remove most of the things in the COLORS.ui object, and move the rest of them to COLORS.theme.

[tyxla]

Sounds good, thanks for confirming @mirka! I'm planning to follow up on this next week!

[tyxla]

@mirka I'm renaming the color values as you suggested in #53631. Furthermore, I'm removing the duplication you outlined above in #53632. Let me know what you think!