Style engine: export util to compile CSS custom var from preset string #64490
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] Code Quality
|
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. |
ramonjd
commented
Aug 14, 2024
ramonjd
commented
Aug 14, 2024
4 tasks
ramonjd
requested review from
noisysocks,
talldan,
kevin940726,
andrewserong and
aaronrobertshaw
ramonjd
commented
Aug 14, 2024
| @@ -127,7 +127,12 @@ export function generateBoxRules( | |||
| } | |||
|
|
|||
| /** | |||
| * Returns a CSS var value from incoming style value following the pattern `var:description|context|slug`. | |||
| * Returns a WordPress CSS custom var value from incoming style preset value. | |||
There was a problem hiding this comment.
Side note: I think the style engine in general could subsume a lot of style utility functions, even in the backend. E.g., ref resolution, CSS var building and more.
I see the benefit as reducing WP_Theme_JSON bloat and grouping 'like' concerns.
|
Size Change: -14 B (0%) Total Size: 1.77 MB
ℹ️ View Unchanged
|
aaronrobertshaw
approved these changes
Aug 14, 2024
There was a problem hiding this comment.
Thanks for the follow-up here @ramonjd 👍
This is testing well for me. I didn't spot any regressions. I focused mainly on color, spacing, and typography preset values using the TT4 and Stewart themes.
✅ All references to removed util have been updated
✅ Global Styles using preset vars continue to work
✅ Block Instance styles using presets work
✅ Custom colors added to Global Styles palette are generated correctly
✅ Issue resolved in #43116 remains fixed
✅ Unit tests are passing
I only have one minor nit or concern around the naming of the util as we're adding it to the public API.
To me, it is rather misleading. getCSSVarFromStyleValue sort of suggests it will only return a CSS variable. The readme and doc block comment also say this for the return value description despite the util actually returning the original value if no preset is detected.
Having said that, I'm terrible at naming so don't have any good alternatives 😅
Perhaps some of these might spark better ideas. If not, we could always stick with the status quo but update the documentation to better reflect functionality.
maybeGetCSSVarFromStyleValuegetCSSValueFromRawStylegetCSSVarOrValueFromRawStylegetCSSVarOrValueFromStyleValue
I don't want to block this PR if others are happy with the current naming, so I'll proactively approve this PR from a functionality and testing standpoint.
|
Thanks for testing, folks! I'll make those test/type/naming changes.
That's a good point. I like your suggestion of |
Use style engine getCSSVarFromStyleValue Update style engine tests to cover bugfix in #43116 Moves style engine utils tests into correct directory
Redefine types Update tests
ramonjd
force-pushed
the
update/export-style-utils
branch
from
e049c40 to
3fadf97
Compare
kevin940726
reviewed
Aug 15, 2024
| export function getCSSVarFromStyleValue( styleValue: string ): string { | ||
|
|
||
| export function getCSSValueFromRawStyle( | ||
| styleValue: string | any |
There was a problem hiding this comment.
Nit: I think using any here effectively disables the type checking. I believe we can use = to communicate that we primarily accept string, something like below:
There was a problem hiding this comment.
Oh dang, thanks for the tip. I'll create a quick follow up if you don't mind taking a look.
fabiankaegy
added
the
Needs Dev Note
97 tasks
|
Hey @ramonjd 👋 Would you be able to help write a dev note for this for the 6.7 release? We are planning to have this as part of a larger Miscellaneous Editor Updates note. We are hoping to get all drafts in by October 13th to leave some time for reviews before the RC1. All Dev Notes get tracked in #65784 so feel free to leave a note there or ping me directly :) Please let us know if you can assist with that. Thanks in advance :) |
|
I've taken a quick stab at a dev note for this one. Ramon can give it a once over and check it's accurate before including it. Dev Note: New style engine util for compiling CSS custom varsWordPress 6.7 introduces a new public utility, Example usage: // Example 1: Standard CSS remains unchanged.
getCSSValueFromRawStyle('min(40%, 400px)');
// Returns: 'min(40%, 400px)'
// Example 2: CSS variable for preset color.
getCSSValueFromRawStyle('var:preset|color|wordpress-blue');
// Returns: 'var(--wp--preset--color--wordpress-blue)' |
|
@aaronrobertshaw thanks for this :) I do think it would be useful to include a quick example how to use the API / what it may be used for to this 🤔 |
|
Thanks for the nudge @fabiankaegy, I meant to add examples but got distracted yesterday. I've updated the note with a couple of quick examples that illustrate the input/output for the util. |
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?
Follow up on #64128
This PR:
compileStyleValue()from global style output functionsgetCSSVarFromStyleValue()in its placeWhy?
getCSSVarFromStyleValue()does the same thing ascompileStyleValue()This PR ensures that it's more widely available throughout the app as a public API.
How?
Shuffling stuff around.
Testing Instructions
There should be no regressions.
Smoke test in the site editor by applying some global styles presets to block styles, e.g., colors or font sizes.
Add a custom color in Styles > Color > Palette and make sure the CSS custom var is generated correctly, e.g.,
Run the tests!