Refactor: Add extractPresetSlug as a generalized function to extract slugs.#78328
Refactor: Add extractPresetSlug as a generalized function to extract slugs.#78328USERSATOSHI wants to merge 1 commit into
Conversation
keeping the extractColorSlug since we have exposed this function as a public function and removing it would be a breaking change?
|
I have added this, but kept cc: @ciampo |
|
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. |
ciampo
left a comment
ciampo
left a comment
There was a problem hiding this comment.
Tested in the editor, works well
Testing Instructions
Prerequisites
- A theme that ships at least one gradient preset (or a custom gradient added via Styles → Colors → Gradients). These instructions assume a gradient with slug
custom-marco-test-gradient; substitute your own slug as needed.
Steps
-
Insert a Paragraph block and type "Hello".
-
Open the post Options menu (three dots, top-right) → Code editor.
-
Replace the paragraph's markup with:
<!-- wp:paragraph {"style":{"color":{"gradient":"var(--wp--preset--gradient--custom-marco-test-gradient)"}}} --> <p style="background:var(--wp--preset--gradient--custom-marco-test-gradient)">Hello</p> <!-- /wp:paragraph -->
-
Switch back to Visual editor. The paragraph should render with the gradient background.
-
Select the paragraph → right sidebar Styles tab → under Color, click the Gradient swatch.
-
In the picker, pick a different gradient, then pick Custom Marco Test Gradient again (any action that fires
onChangeonce is enough). -
Switch back to Code editor.
Expected result (with this PR)
The markup collapses to:
<!-- wp:paragraph {"gradient":"custom-marco-test-gradient"} -->
<p class="has-custom-marco-test-gradient-gradient-background has-background">Hello</p>
<!-- /wp:paragraph -->style.color.gradientis gone.- The
gradientattribute holds the slug. - The rendered
<p>has thehas-<slug>-gradient-backgroundclass instead of an inlinestyle="background:var(...)".
Regression check (on trunk, without the PR)
After step 6, the markup still contains the original style.color.gradient: "var(--wp--preset--gradient--custom-marco-test-gradient)" and the gradient attribute is never set, because the old code only matched the var:preset|gradient|… form.
Notes
- Don't test this on Group / Cover. Those have
supports.background.gradientand route throughhooks/background.js, which doesn't go through the slug extraction this PR touches. - The Paragraph block is the cleanest test surface because it only has
supports.color.gradients, which is the exact path PR #78328 modifies. - Unit tests:
npx jest --config test/unit/jest.config.js packages/block-editor/src/utils/test/color-values.js— should report 14/14 passing.
We should also add a CHANGELOG entry, likely a short note under Bug Fixes in packages/block-editor/CHANGELOG.md to document the new behavior for gradients: theme CSS-var gradients now decode to a slug and end up persisted as a gradient attribute rather than as a raw style.color.gradient
| new RegExp( `^var\\(${ cssVarPrefix }([^)]+)\\)$` ) | ||
| ); | ||
| return themeFormatMatch?.[ 1 ]; | ||
| return themeFormatMatch?.[ 1 ] ?? undefined; |
There was a problem hiding this comment.
The ?? undefined here is redundant, because the optional chaining in themeFormatMatch?.[ 1 ] would already return undefined when there is no match
There was a problem hiding this comment.
I added ?? undefined since ?. would give null when there is no regex match
If we are fine with null then I can remove it
There was a problem hiding this comment.
I believe that (null)?.[1] returns undefined — see MDN about the ?. operator:
the expression short circuits and evaluates to
undefinedinstead of throwing an error.
| /** | ||
| * Extracts the palette slug from a color style value, supporting both the user | ||
| * preset format and the theme CSS-variable format: | ||
| * | ||
| * - User format: `var:preset|color|slug` | ||
| * - Theme format: `var(--wp--preset--color--slug)` | ||
| * | ||
| * Returns `undefined` for plain hex values, non-strings, or any other | ||
| * unrecognised format. | ||
| * | ||
| * @param {*} rawValue Raw style value stored in the style object. | ||
| * @return {string|undefined} The palette slug, or undefined. | ||
| */ | ||
| export function extractColorSlug( rawValue ) { | ||
| return extractPresetSlug( rawValue, 'color' ); | ||
| } |
There was a problem hiding this comment.
kept extractColorSlug since I don't know whether we should remove it or not. Since we made it a public function, removing it would cause a breaking change.
AFAIK, extractColorSlug is not a public API of the @wordpress/block-editor package, and it's only imported internally to the package; therefore, it should be fine to remove it and refactor internal usage to extractPresetSlug (including existing tests)
| const cssVarPrefix = `--wp--preset--${ type }--`; | ||
| const themeFormatMatch = rawValue.match( | ||
| /^var\(--wp--preset--color--([^)]+)\)$/ | ||
| new RegExp( `^var\\(${ cssVarPrefix }([^)]+)\\)$` ) |
There was a problem hiding this comment.
Very nitty, but we could cache the RegExp instead of creating a new one every time. eg
const themeRegexCache = new Map();
function themeRegex( type ) {
if ( ! themeRegexCache.has( type ) ) {
themeRegexCache.set(
type,
new RegExp( `^var\\(--wp--preset--${ type }--([^)]+)\\)$` )
);
}
return themeRegexCache.get( type );
}| * | ||
| * @param {*} rawValue Raw style value stored in the style object. | ||
| * @param {*} rawValue Raw style value stored in the style object. | ||
| * @param {string} type Preset type, e.g. `'color'` or `'gradient'`. |
There was a problem hiding this comment.
We can tighten the type here to @param {'color'|'gradient'}
2 participants
What?
A follow-up PR based on #78048 (comment) that generalizes preset slug extraction into a reusable utility function.
Adds
extractPresetSlug( rawValue, type )to handle slug extraction for any preset type ('color','gradient', etc.), supporting both the user format (var:preset|<type>|slug) and the theme CSS-variable format (var(--wp--preset--<type>--slug)).Why?
Previously, gradient slug extraction was done inline and only handled the
var:preset|gradient|format, missing the themevar(--wp--preset--gradient--…)form.How?
color-values.js
extractPresetSlug(rawValue, type)— generic function handling bothvar:preset|<type>|slugandvar(--wp--preset--<type>--slug)formatsextractColorSlugnow delegates toextractPresetSlug(rawValue, 'color')for backward compatibilitycolor-values.js
extractPresetSlugcovering both types, both formats, hyphenated slugs, type mismatches, non-strings, and malformed varscolor.js
extractPresetSlug( gradientValue, 'gradient' ), now handling both preset formatsTesting Instructions
Unit test should suffice
Testing Instructions for Keyboard
Use of AI Tools
None