Theme: Use valid DTCG color format for primitive values#73858
Conversation
|
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. |
aduth
added
[Type] Task
mirka
left a comment
mirka
left a comment
There was a problem hiding this comment.
I can't check Figma, but the validation is passing 👍 All the color values also seems to be preserved.
| try { | ||
| parsed = to( parse( color ), sRGB ); | ||
| } catch { | ||
| return color; | ||
| } |
There was a problem hiding this comment.
Not introduced in this PR, but it's not obvious to me what kinds of cases would error on the parse but is still safe to silently return with the original value.
There was a problem hiding this comment.
That's a good point, maybe we should avoid catching this, so that it actually disrupts the build for the unexpected input.
See: #73858 (comment) The `return` fallback will likely produce an invalid result. We should fail the build if the value can't be parsed.
Avoid function allocation for each transformed value, since this behavior is static
|
Flaky tests detected in 147f8bb. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/20108053593
|
* Theme: Use valid DTCG color format for primitive values * Avoid graceful catching of converted color value See: WordPress/gutenberg#73858 (comment) The `return` fallback will likely produce an invalid result. We should fail the build if the value can't be parsed. * Extract function for rounding hex component Avoid function allocation for each transformed value, since this behavior is static Co-authored-by: aduth <aduth@git.wordpress.org> Co-authored-by: mirka <0mirka00@git.wordpress.org> Source: WordPress/gutenberg@ce4811c
| return color; | ||
| return { | ||
| colorSpace: 'srgb', | ||
| components: parsed.coords.map( roundHexComponent ), |
There was a problem hiding this comment.
In version 0.6, when it's released (currently in beta) we'll be able to write getAll( parsed, { precision: 3 } ).
Maybe we could switch to 0.6.0-beta2 right away, the stable 0.5.2 version is 17 months old.
There was a problem hiding this comment.
Thanks for the pointer. That'll be nice to have. It sounds like the 0.6.0 release could be imminent, or at least was planned for "next week" as of about two months ago 😅
There was a problem hiding this comment.
I think how I'll handle this is set myself a reminder for about a month from now to see if the stable version is released and update both this and your suggestion below. I don't think it's pressing enough to warrant adopting the beta version, unless I'm missing something.
There was a problem hiding this comment.
Yes, it can be updated once a stable version is released, no hurry. I found this suggestion by looking at the roundHexComponent code and thinking: wait a minute, many colorjs.io functions already have a precision option that does exactly this kind of rounding, can we get the rounded coords this way? And the answer turns out to be: yes, in the next version 🙂
| }; | ||
| } | ||
| const transformColorStringToDTCGValue = ( color: string ) => { | ||
| const parsed = to( parse( color ), sRGB ); |
There was a problem hiding this comment.
Instead of parse( color ), we can almost always pass just color. Every colorjs.io function calls getColor on all color parameters before doing anything else. That's where the parsing will happen.
3 participants
What?
Updates theme package to ensure that primitive design tokens are output in a valid format, per the DTCG Color Format specification.
Why?
How?
We previously had some code to handle this, but it looks like the
ifcondition was causing all values not to be matched. The updates here align to expectations that thecolorvalue passed to the function is a hex value, generated bygetColorString.Testing Instructions
Verify that running
npm run --workspace @wordpress/theme buildproduces no local changes (seegit status).Verify that you can import the resulting
color.jsoninto Figma Variables panel, if you have a Figma subscription.Alternatively, try using an online validator to confirm validity. Note that this tool doesn't appear to expect
hexvalues, but as referenced in the specification above,hexis a valid fallback property.