ColorPalette: partial support of color-mix() CSS colors

[Rishit30G]

Why?

Users were facing issue with ColorGradientsControl showing incorrect labels using color-mix

What ?

Related to #64195

How?

Solved this issue by adding const currentValueIsColorMix = /^color-mix\(.*\)$/.test( currentValue ); to check for color-mix and using it logically in normalizedCurrentValue instead of converting color-mix value to hex which was giving the same label as only the transparency was changing but the color was same i.e #000000

Testing Instructions

Copy the color palette snippet in this issue to the theme.json.
Go to block editor (e.g. edit a page)
Select a block and try to change its color.
Try switching between the new colors and the selected label will always show 10% of Current Text.

Screenshots or screencast

Screen.Recording.2024-08-03.at.10.46.07.AM.mov

[github-actions]

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 props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: Rishit30G <rishit30g@git.wordpress.org>
Co-authored-by: ciampo <mciampini@git.wordpress.org>
Co-authored-by: mirka <0mirka00@git.wordpress.org>
Co-authored-by: tyxla <tyxla@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[github-actions]

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @Rishit30G! In case you missed it, we'd love to have you join us in our Slack community.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

[ciampo]

Thank you for working on this!

Unfortunately, as explained in an inline comment, we're currently unable to fully support color-mix because of lack of support in parsing newer CSS color syntaxes from an internal dependency.

Having said that, we may also argue that this PR still improves the component with respect to what's on trunk, and therefore we may want to still merge such improvements.

@WordPress/gutenberg-components any thoughts?

If we were to continue work on this PR, it would be great also to add unit tests.

[ciampo]

I'd apply a less strict regex here for better matching:

Suggested change

	const currentValueIsColorMix = /^color-mix\(.*\)$/.test( currentValue );
	const currentValueIsColorMix = /color-mix\(/.test( currentValue );

[ciampo]

And potentially consider removing the "start of line" caret (^) from currentValueIsCssVariable too

[ciampo]

I'm not sure that we can fully support color-mix CSS colors at the moment.

I took a look at the component, and noticed that this PR is missing changes in the normalizeColorValue function at the bottom of this file. The normalizeColorValue function takes care of converting the chosen color option to a CSS color that is passed to the ColorPicker.

I applied the same logic in that function too:

diff --git a/packages/components/src/color-palette/utils.ts b/packages/components/src/color-palette/utils.ts
index 00438db528..43104d2084 100644
--- a/packages/components/src/color-palette/utils.ts
+++ b/packages/components/src/color-palette/utils.ts
@@ -82,9 +82,12 @@ export const normalizeColorValue = (
 	value: string | undefined,
 	element: HTMLElement | null
 ) => {
-	const currentValueIsCssVariable = /^var\(/.test( value ?? '' );
+	const valueIsCssVariable = /var\(/.test( value ?? '' );
+	const valueIsColorMix = /color-mix\(/.test( value ?? '' );
 
-	if ( ! currentValueIsCssVariable || element === null ) {
+	const valueIsSimpleColor = ! valueIsCssVariable && ! valueIsColorMix;
+
+	if ( valueIsSimpleColor || element === null ) {
 		return value;
 	}
 

But the computed styles that the browser returns from applying a color-mix() color to an element use the CSS color() function, which is not a valid input to colord (the library that we use to manipulate and normalize color strings).

And therefore, when opening the ColorPicker, the color initially chosen in the picker is not the correct color, but black (which is the default color that colord outputs when the string that it parses is not valid).

Kapture.2024-08-15.at.13.11.48.mp4

[mirka]

But the computed styles that the browser returns from applying a color-mix() color to an element use the CSS color() function, which is not a valid input to colord (the library that we use to manipulate and normalize color strings).

What are the repro steps for this by the way? I'm just trying it in the browser and not on this PR branch, but when I try it I get a string for the respective color space. So if I getComputedStyle() on an element with background-color: color-mix(in oklab, #a71e14 75%, white) for example, I will get 'oklab(0.603981 0.113291 0.0640898)'.

If we can do that, we could possibly add support for it if it's a color space that has a colord plugin. Maybe not in this PR though.

And yes I'm all for merging changes as long as it's incrementally better than trunk 👍

[ciampo]

What are the repro steps for this by the way?

For example, in latest chrome, if I apply background-color: color-mix(in srgb, indigo, white) to an element and then query its computed background-color style, I get color(srgb 0.647059 0.5 0.754902).

If we can do that, we could possibly add support for it if it's a color space that has a colord plugin. Maybe not in this PR though.

I had looked into it, and didn't spot any plugin for the color() syntax — and not even for newer color spaces like oklab and oklch

I guess it's up to the browser to decide the format of the computed style string, and since color-mix can target any color space, to support fully color-mix we'd need to be able to virtually support any color syntax.

Also, looking at colord repo, I think the project is not being maintained anymore 😱 Given that CSS continues to evolve, I think that the colord-based approach is showing its limitations. We should ideally find a browser-based way (either through CSS or native JS) that will guarantee us that whatever CSS color was passed to the component, we're able to parse it, show it, and render a working color picker.

And yes I'm all for merging changes as long as it's incrementally better than trunk 👍

👌

[ciampo]

We should ideally find a browser-based way (either through CSS or native JS) that will guarantee us that whatever CSS color was passed to the component, we're able to parse it, show it, and render a working color picker.

Actually, hold up my canvas: https://codepen.io/ciampo/pen/QWXamNe

This approach could have potential. We'd need to make sure it can't be somehow blocked by the browser, and that there are no severe performance implications (if needed, we can explore using OffscreenCanvas in a separate worker thread).

[ciampo]

Hey @Rishit30G , @mirka and I think that the improvements in this PR are still worth pursuing, despite recognising that we still won't be able to fully support color-mix() syntaxes.

Here's a list of next steps for this PR:

• Apply suggestions from ColorPalette: partial support of color-mix() CSS colors #64224 (comment)
• Apply similar changes to the normalizeColorValue (see suggested diff in ColorPalette: partial support of color-mix() CSS colors #64224 (comment))
• Consider extracting a reusing a isSimpleCSSColor utility that checks for CSS variable and color-mix()
• Add unit tests if possible

Does it sound good ?

[ciampo]

Thank you!

I left some extra feedback on the current changes.

For the unit tests, we could add the color-mix tests to this file. Let me know if you need help in writing those tests.

[ciampo]

I actually looked into writing some unit tests, but it looks like they would be a bit pointless since JSDom has very poor support for CSS custom properties and CSS color syntax — so our best bet would be to use a real browser environment to test this component, and that's not in the scope of this PR.

@Rishit30G , the last thing left before merging would be to add a CHANGELOG entry in this file. I would probably add it under the Enhancements category.

Thank you!

[ciampo]

LGTM 🚀

[ramonjd]

I believe this change has created a regression.

It's treating undefined values as false positives = non-simple values, which means it then returns defaultView?.getComputedStyle

With this changes

The default alpha value is 0, which means, by default, any custom color cannot be seen in the canvas.

Kapture.2024-09-02.at.14.46.23.mp4

With these changes reverted

The alpha is 100 so, by default, my custom color changes are visible on the canvas.

Kapture.2024-09-02.at.14.44.36.mp4

[ramonjd]

I have a fix here:

• ColorPalette utils: do not normalize undefined color values #64969

Would be good to confirm that it doesn't affect what this PR intends to do. Thanks!