-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Refactor: Add extractPresetSlug as a generalized function to extract slugs. #78328
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
USERSATOSHI
wants to merge
4
commits into
WordPress:trunk
Choose a base branch
from
USERSATOSHI:feat/refactor-gradient-slug-fn-in-colors
base: trunk
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
0d54bb2
Refactor: add generalized extractPresetSlug.
USERSATOSHI 2ff653d
Refactor: replace extractColorSlug with extractPresetSlug in color pa…
USERSATOSHI 8d0c934
Merge branch 'trunk' into feat/refactor-gradient-slug-fn-in-colors
USERSATOSHI f55bfae
chore: add bug fix for ColorPanel gradient slug persistence in changelog
USERSATOSHI File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,25 +1,28 @@ | ||
| /** | ||
| * Extracts the palette slug from a style value, supporting both the user | ||
| * preset format and the theme CSS-variable format: | ||
| * Extracts the palette slug from a style value for any preset type, 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)` | ||
| * - User format: `var:preset|<type>|slug` | ||
| * - Theme format: `var(--wp--preset--<type>--slug)` | ||
| * | ||
| * Returns `undefined` for plain hex values, non-strings, or any other | ||
| * Returns `undefined` for plain values, non-strings, or any other | ||
| * unrecognised format. | ||
| * | ||
| * @param {*} rawValue Raw style value stored in the style object. | ||
| * @param {*} rawValue Raw style value stored in the style object. | ||
| * @param {'color'|'gradient'} type Preset type, e.g. `'color'` or `'gradient'`. | ||
| * @return {string|undefined} The palette slug, or undefined. | ||
| */ | ||
| export function extractColorSlug( rawValue ) { | ||
| export function extractPresetSlug( rawValue, type ) { | ||
| if ( typeof rawValue !== 'string' ) { | ||
| return undefined; | ||
| } | ||
| if ( rawValue.startsWith( 'var:preset|color|' ) ) { | ||
| return rawValue.slice( 'var:preset|color|'.length ); | ||
| const userPrefix = `var:preset|${ type }|`; | ||
| if ( rawValue.startsWith( userPrefix ) ) { | ||
| return rawValue.slice( userPrefix.length ); | ||
| } | ||
| const cssVarPrefix = `--wp--preset--${ type }--`; | ||
| const themeFormatMatch = rawValue.match( | ||
| /^var\(--wp--preset--color--([^)]+)\)$/ | ||
| new RegExp( `^var\\(${ cssVarPrefix }([^)]+)\\)$` ) | ||
| ); | ||
| return themeFormatMatch?.[ 1 ]; | ||
| } | ||
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,43 +1,78 @@ | ||
| /** | ||
| * Internal dependencies | ||
| */ | ||
| import { extractColorSlug } from '../color-values'; | ||
| import { extractPresetSlug } from '../color-values'; | ||
|
|
||
| describe( 'extractColorSlug', () => { | ||
| it( 'extracts the slug from the user preset format (var:preset|color|slug)', () => { | ||
| expect( extractColorSlug( 'var:preset|color|dark-text' ) ).toBe( | ||
| 'dark-text' | ||
| ); | ||
| describe( 'extractPresetSlug', () => { | ||
| it( 'extracts the slug for a color from the user preset format', () => { | ||
| expect( | ||
| extractPresetSlug( 'var:preset|color|dark-text', 'color' ) | ||
| ).toBe( 'dark-text' ); | ||
| } ); | ||
|
|
||
| it( 'extracts the slug from the theme CSS-var format (var(--wp--preset--color--slug))', () => { | ||
| it( 'extracts the slug for a color from the theme CSS-var format', () => { | ||
| expect( | ||
| extractColorSlug( 'var(--wp--preset--color--vivid-purple)' ) | ||
| extractPresetSlug( | ||
| 'var(--wp--preset--color--vivid-purple)', | ||
| 'color' | ||
| ) | ||
| ).toBe( 'vivid-purple' ); | ||
| } ); | ||
|
|
||
| it( 'handles slugs that contain hyphens', () => { | ||
| it( 'extracts the slug for a gradient from the user preset format', () => { | ||
| expect( | ||
| extractColorSlug( 'var:preset|color|my-custom-blue-100' ) | ||
| ).toBe( 'my-custom-blue-100' ); | ||
| extractPresetSlug( | ||
| 'var:preset|gradient|blush-bordeaux', | ||
| 'gradient' | ||
| ) | ||
| ).toBe( 'blush-bordeaux' ); | ||
| } ); | ||
|
|
||
| it( 'extracts the slug for a gradient from the theme CSS-var format', () => { | ||
| expect( | ||
| extractColorSlug( 'var(--wp--preset--color--my-custom-blue-100)' ) | ||
| ).toBe( 'my-custom-blue-100' ); | ||
| extractPresetSlug( | ||
| 'var(--wp--preset--gradient--blush-bordeaux)', | ||
| 'gradient' | ||
| ) | ||
| ).toBe( 'blush-bordeaux' ); | ||
| } ); | ||
|
|
||
| it( 'returns undefined for a plain hex value', () => { | ||
| expect( extractColorSlug( '#000000' ) ).toBeUndefined(); | ||
| it( 'handles slugs that contain hyphens for any type', () => { | ||
| expect( | ||
| extractPresetSlug( | ||
| 'var:preset|gradient|my-custom-gradient-100', | ||
| 'gradient' | ||
| ) | ||
| ).toBe( 'my-custom-gradient-100' ); | ||
| expect( | ||
| extractPresetSlug( | ||
| 'var(--wp--preset--gradient--my-custom-gradient-100)', | ||
| 'gradient' | ||
| ) | ||
| ).toBe( 'my-custom-gradient-100' ); | ||
| } ); | ||
|
|
||
| it( 'returns undefined for a non-matching type', () => { | ||
| expect( | ||
| extractPresetSlug( 'var:preset|color|dark-text', 'gradient' ) | ||
| ).toBeUndefined(); | ||
| expect( | ||
| extractPresetSlug( | ||
| 'var(--wp--preset--color--vivid-purple)', | ||
| 'gradient' | ||
| ) | ||
| ).toBeUndefined(); | ||
| } ); | ||
|
|
||
| it( 'returns undefined for non-string values', () => { | ||
| expect( extractColorSlug( undefined ) ).toBeUndefined(); | ||
| expect( extractColorSlug( null ) ).toBeUndefined(); | ||
| expect( extractColorSlug( 42 ) ).toBeUndefined(); | ||
| expect( extractPresetSlug( undefined, 'gradient' ) ).toBeUndefined(); | ||
| expect( extractPresetSlug( null, 'gradient' ) ).toBeUndefined(); | ||
| expect( extractPresetSlug( 42, 'gradient' ) ).toBeUndefined(); | ||
| } ); | ||
|
|
||
| it( 'returns undefined for a theme var missing its closing parenthesis', () => { | ||
| expect( | ||
| extractColorSlug( 'var(--wp--preset--color--oops' ) | ||
| extractPresetSlug( 'var(--wp--preset--gradient--oops', 'gradient' ) | ||
| ).toBeUndefined(); | ||
| } ); | ||
| } ); |
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nitty, but we could cache the RegExp instead of creating a new one every time. eg
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should I bound the map or use a caching strategy like LRU so that we don't have an increasing memory from caching the regexes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc: @ciampo