From 729f70e2713c9bf83301adfc7437f07db96d3d2d Mon Sep 17 00:00:00 2001 From: Tetsuaki Hamano Date: Sun, 10 Sep 2023 17:38:05 +0900 Subject: [PATCH 1/5] Palette Edit: Don't discards colors with default name and slug --- .../components/src/palette-edit/index.tsx | 29 ------------------- 1 file changed, 29 deletions(-) diff --git a/packages/components/src/palette-edit/index.tsx b/packages/components/src/palette-edit/index.tsx index 9061222567103a..5e57300aa7f11a 100644 --- a/packages/components/src/palette-edit/index.tsx +++ b/packages/components/src/palette-edit/index.tsx @@ -257,18 +257,6 @@ function Option< T extends Color | Gradient >( { ); } -function isTemporaryElement( - slugPrefix: string, - { slug, color, gradient }: Color | Gradient -) { - const regex = new RegExp( `^${ slugPrefix }color-([\\d]+)$` ); - return ( - regex.test( slug ) && - ( ( !! color && color === DEFAULT_COLOR ) || - ( !! gradient && gradient === DEFAULT_GRADIENT ) ) - ); -} - function PaletteEditListView< T extends Color | Gradient >( { elements, onChange, @@ -284,23 +272,6 @@ function PaletteEditListView< T extends Color | Gradient >( { useEffect( () => { elementsReference.current = elements; }, [ elements ] ); - useEffect( () => { - return () => { - if ( - elementsReference.current?.some( ( element ) => - isTemporaryElement( slugPrefix, element ) - ) - ) { - const newElements = elementsReference.current.filter( - ( element ) => ! isTemporaryElement( slugPrefix, element ) - ); - onChange( newElements.length ? newElements : undefined ); - } - }; - // Disable reason: adding the missing dependency here would cause breaking changes that will require - // a heavier refactor to avoid. See https://github.com/WordPress/gutenberg/pull/43911 - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [] ); const debounceOnChange = useDebounce( onChange, 100 ); From eb125e72632fc65321b7ce921ce046ea2e962d0d Mon Sep 17 00:00:00 2001 From: Tetsuaki Hamano Date: Sun, 10 Sep 2023 17:54:13 +0900 Subject: [PATCH 2/5] Update comments and variables --- .../components/src/palette-edit/index.tsx | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/components/src/palette-edit/index.tsx b/packages/components/src/palette-edit/index.tsx index 5e57300aa7f11a..00c3c4ecf4e0ca 100644 --- a/packages/components/src/palette-edit/index.tsx +++ b/packages/components/src/palette-edit/index.tsx @@ -74,8 +74,8 @@ function NameInput( { value, onChange, label }: NameInputProps ) { } /** - * Returns a temporary name for a palette item in the format "Color + id". - * To ensure there are no duplicate ids, this function checks all slugs for temporary names. + * Returns a name for a palette item in the format "Color + id". + * To ensure there are no duplicate ids, this function checks all slugs. * It expects slugs to be in the format: slugPrefix + color- + number. * It then sets the id component of the new name based on the incremented id of the highest existing slug id. * @@ -88,10 +88,10 @@ export function getNameForPosition( elements: PaletteElement[], slugPrefix: string ) { - const temporaryNameRegex = new RegExp( `^${ slugPrefix }color-([\\d]+)$` ); + const nameRegex = new RegExp( `^${ slugPrefix }color-([\\d]+)$` ); const position = elements.reduce( ( previousValue, currentValue ) => { if ( typeof currentValue?.slug === 'string' ) { - const matches = currentValue?.slug.match( temporaryNameRegex ); + const matches = currentValue?.slug.match( nameRegex ); if ( matches ) { const id = parseInt( matches[ 1 ], 10 ); if ( id >= previousValue ) { @@ -103,7 +103,7 @@ export function getNameForPosition( }, 1 ); return sprintf( - /* translators: %s: is a temporary id for a custom color */ + /* translators: %s: is an id for a custom color */ __( 'Color %s' ), position ); @@ -427,7 +427,7 @@ export function PaletteEdit( { : __( 'Add color' ) } onClick={ () => { - const tempOptionName = getNameForPosition( + const optionName = getNameForPosition( elements, slugPrefix ); @@ -437,10 +437,10 @@ export function PaletteEdit( { ...gradients, { gradient: DEFAULT_GRADIENT, - name: tempOptionName, + name: optionName, slug: slugPrefix + - kebabCase( tempOptionName ), + kebabCase( optionName ), }, ] ); } else { @@ -448,10 +448,10 @@ export function PaletteEdit( { ...colors, { color: DEFAULT_COLOR, - name: tempOptionName, + name: optionName, slug: slugPrefix + - kebabCase( tempOptionName ), + kebabCase( optionName ), }, ] ); } From 1547507d0cf28d178ec870da9c8ebe430b9675ac Mon Sep 17 00:00:00 2001 From: Tetsuaki Hamano Date: Sun, 10 Sep 2023 17:55:10 +0900 Subject: [PATCH 3/5] Update changelog --- packages/components/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index 3dfc268ad21636..4527292a935dc7 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -9,6 +9,7 @@ ### Bug Fix - `Modal`: fix closing when contained iframe is focused ([#51602](https://github.com/WordPress/gutenberg/pull/51602)). +- `PaletteEdit`: Don't discards colors with default name and slug ([#54332](https://github.com/WordPress/gutenberg/pull/54332)). ### Internal From 7f3524ef9eaf8a56be7cad1ff016d79f494b286d Mon Sep 17 00:00:00 2001 From: Tetsuaki Hamano Date: Wed, 20 Dec 2023 14:58:58 +0900 Subject: [PATCH 4/5] Remove isTemporaryElement function and tests --- .../components/src/palette-edit/index.tsx | 28 +------ .../src/palette-edit/test/index.tsx | 76 +------------------ 2 files changed, 2 insertions(+), 102 deletions(-) diff --git a/packages/components/src/palette-edit/index.tsx b/packages/components/src/palette-edit/index.tsx index 4a3936716c3ff3..87df1894dc36fc 100644 --- a/packages/components/src/palette-edit/index.tsx +++ b/packages/components/src/palette-edit/index.tsx @@ -60,7 +60,7 @@ import type { PaletteElement, } from './types'; -export const DEFAULT_COLOR = '#000'; +const DEFAULT_COLOR = '#000'; function NameInput( { value, onChange, label }: NameInputProps ) { return ( @@ -261,32 +261,6 @@ function Option< T extends Color | Gradient >( { ); } -/** - * Checks if a color or gradient is a temporary element by testing against default values. - */ -export function isTemporaryElement( - slugPrefix: string, - { slug, color, gradient }: Color | Gradient -): Boolean { - const regex = new RegExp( `^${ slugPrefix }color-([\\d]+)$` ); - - // If the slug matches the temporary name regex, - // check if the color or gradient matches the default value. - if ( regex.test( slug ) ) { - // The order is important as gradient elements - // contain a color property. - if ( !! gradient ) { - return gradient === DEFAULT_GRADIENT; - } - - if ( !! color ) { - return color === DEFAULT_COLOR; - } - } - - return false; -} - function PaletteEditListView< T extends Color | Gradient >( { elements, onChange, diff --git a/packages/components/src/palette-edit/test/index.tsx b/packages/components/src/palette-edit/test/index.tsx index 1a0b2fdaaab3fb..1bf2802709de7f 100644 --- a/packages/components/src/palette-edit/test/index.tsx +++ b/packages/components/src/palette-edit/test/index.tsx @@ -6,13 +6,8 @@ import { render, fireEvent, screen } from '@testing-library/react'; /** * Internal dependencies */ -import PaletteEdit, { - getNameForPosition, - isTemporaryElement, - DEFAULT_COLOR, -} from '..'; +import PaletteEdit, { getNameForPosition } from '..'; import type { PaletteElement } from '../types'; -import { DEFAULT_GRADIENT } from '../../custom-gradient-picker/constants'; describe( 'getNameForPosition', () => { test( 'should return 1 by default', () => { @@ -85,75 +80,6 @@ describe( 'getNameForPosition', () => { } ); } ); -describe( 'isTemporaryElement', () => { - [ - { - message: 'identifies temporary color', - slug: 'test-', - obj: { - name: '', - slug: 'test-color-1', - color: DEFAULT_COLOR, - }, - expected: true, - }, - { - message: 'identifies temporary gradient', - slug: 'test-', - obj: { - name: '', - slug: 'test-color-1', - gradient: DEFAULT_GRADIENT, - }, - expected: true, - }, - { - message: 'identifies custom color slug', - slug: 'test-', - obj: { - name: '', - slug: 'test-color-happy', - color: DEFAULT_COLOR, - }, - expected: false, - }, - { - message: 'identifies custom color value', - slug: 'test-', - obj: { - name: '', - slug: 'test-color-1', - color: '#ccc', - }, - expected: false, - }, - { - message: 'identifies custom gradient slug', - slug: 'test-', - obj: { - name: '', - slug: 'test-gradient-joy', - color: DEFAULT_GRADIENT, - }, - expected: false, - }, - { - message: 'identifies custom gradient value', - slug: 'test-', - obj: { - name: '', - slug: 'test-color-3', - color: 'linear-gradient(90deg, rgba(22, 22, 22, 1) 0%, rgb(155, 81, 224) 100%)', - }, - expected: false, - }, - ].forEach( ( { message, slug, obj, expected } ) => { - it( `should ${ message }`, () => { - expect( isTemporaryElement( slug, obj ) ).toBe( expected ); - } ); - } ); -} ); - describe( 'PaletteEdit', () => { const defaultProps = { colors: [ { color: '#ffffff', name: 'Base', slug: 'base' } ], From cb9e94a16e7d8818144df0b384566c3292bec3e4 Mon Sep 17 00:00:00 2001 From: Aki Hamano <54422211+t-hamano@users.noreply.github.com> Date: Thu, 21 Dec 2023 12:20:50 +0900 Subject: [PATCH 5/5] Fix changelog typo Co-authored-by: Ramon --- packages/components/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index 2c7f67986e70af..ebeef34af8b6d9 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -10,7 +10,7 @@ - `Button`: Fix logic of `has-text` class addition ([#56949](https://github.com/WordPress/gutenberg/pull/56949)). - `FormTokenField`: Fix a regression where the suggestion list would re-open after clicking away from the input ([#57002](https://github.com/WordPress/gutenberg/pull/57002)). - `Snackbar`: Remove erroneous `__unstableHTML` prop from TypeScript definitions ([#57218](https://github.com/WordPress/gutenberg/pull/57218)). -- `PaletteEdit`: Don't discards colors with default name and slug ([#54332](https://github.com/WordPress/gutenberg/pull/54332)). +- `PaletteEdit`: Don't discard colors with default name and slug ([#54332](https://github.com/WordPress/gutenberg/pull/54332)). ### Enhancements