From 5c7b6b1019ebec12cf17259d2d5fed82ae796c85 Mon Sep 17 00:00:00 2001 From: megane9988 Date: Thu, 7 Mar 2024 14:13:05 +0800 Subject: [PATCH 1/6] rebase from trunk --- .../components/src/palette-edit/index.tsx | 44 +++++++++---------- .../src/palette-edit/test/index.tsx | 12 ++--- 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/packages/components/src/palette-edit/index.tsx b/packages/components/src/palette-edit/index.tsx index 5dd8787eb6594..ee3460544aff7 100644 --- a/packages/components/src/palette-edit/index.tsx +++ b/packages/components/src/palette-edit/index.tsx @@ -74,17 +74,17 @@ function NameInput( { value, onChange, label }: NameInputProps ) { } /** - * Returns a name for a palette item in the format "Color + id". + * Returns a name and slug for a palette item. The name takes 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. * - * @param elements An array of color palette items. - * @param slugPrefix The slug prefix used to match the element slug. + * @param elements An array of color palette items. + * @param slugPrefix The slug prefix used to match the element slug. * - * @return A unique name for a palette item. + * @return {{name:string, slug:string}} A name and slug for the new palette item. */ -export function getNameForPosition( +export function getNameAndSlugForPosition( elements: PaletteElement[], slugPrefix: string ) { @@ -102,11 +102,14 @@ export function getNameForPosition( return previousValue; }, 1 ); - return sprintf( - /* translators: %s: is an id for a custom color */ - __( 'Color %s' ), - position - ); + return { + name: sprintf( + /* translators: %s: is an id for a custom color */ + __( 'Color %s' ), + position + ), + slug: slugPrefix + 'color-' + position, + }; } function ColorPickerPopover< T extends Color | Gradient >( { @@ -434,20 +437,19 @@ export function PaletteEdit( { : __( 'Add color' ) } onClick={ () => { - const optionName = getNameForPosition( - elements, - slugPrefix - ); + const { name, slug } = + getNameAndSlugForPosition( + elements, + slugPrefix + ); if ( !! gradients ) { onChange( [ ...gradients, { gradient: DEFAULT_GRADIENT, - name: optionName, - slug: - slugPrefix + - kebabCase( optionName ), + name, + slug, }, ] ); } else { @@ -455,10 +457,8 @@ export function PaletteEdit( { ...colors, { color: DEFAULT_COLOR, - name: optionName, - slug: - slugPrefix + - kebabCase( optionName ), + name, + slug, }, ] ); } diff --git a/packages/components/src/palette-edit/test/index.tsx b/packages/components/src/palette-edit/test/index.tsx index 36f2949b3fbd3..59571a7947e59 100644 --- a/packages/components/src/palette-edit/test/index.tsx +++ b/packages/components/src/palette-edit/test/index.tsx @@ -7,7 +7,7 @@ import { click, type, press } from '@ariakit/test'; /** * Internal dependencies */ -import PaletteEdit, { getNameForPosition } from '..'; +import PaletteEdit, { getNameAndSlugForPosition } from '..'; import type { PaletteElement } from '../types'; const noop = () => {}; @@ -21,12 +21,12 @@ async function clearInput( input: HTMLInputElement ) { } } -describe( 'getNameForPosition', () => { +describe( 'getNameAndSlugForPosition', () => { test( 'should return 1 by default', () => { const slugPrefix = 'test-'; const elements: PaletteElement[] = []; - expect( getNameForPosition( elements, slugPrefix ) ).toEqual( + expect( getNameAndSlugForPosition( elements, slugPrefix ) ).toEqual( 'Color 1' ); } ); @@ -41,7 +41,7 @@ describe( 'getNameForPosition', () => { }, ]; - expect( getNameForPosition( elements, slugPrefix ) ).toEqual( + expect( getNameAndSlugForPosition( elements, slugPrefix ) ).toEqual( 'Color 2' ); } ); @@ -56,7 +56,7 @@ describe( 'getNameForPosition', () => { }, ]; - expect( getNameForPosition( elements, slugPrefix ) ).toEqual( + expect( getNameAndSlugForPosition( elements, slugPrefix ) ).toEqual( 'Color 1' ); } ); @@ -86,7 +86,7 @@ describe( 'getNameForPosition', () => { }, ]; - expect( getNameForPosition( elements, slugPrefix ) ).toEqual( + expect( getNameAndSlugForPosition( elements, slugPrefix ) ).toEqual( 'Color 151' ); } ); From fcc6d9d1c0709b99adca4a605e4b92a401e7034f Mon Sep 17 00:00:00 2001 From: megane9988 Date: Thu, 7 Mar 2024 14:33:18 +0800 Subject: [PATCH 2/6] fix test --- packages/components/CHANGELOG.md | 5 ++++ .../src/palette-edit/test/index.tsx | 28 +++++++++++-------- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index 61fba6f264b52..bc974275a05ab 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -12,6 +12,11 @@ - `Button`: Keep deprecated props in type definitions ([#59913](https://github.com/WordPress/gutenberg/pull/59913)). +### Bug Fix +- `PaletteEdit`: Fix order numbers for color palette ([#52212](https://github.com/WordPress/gutenberg/pull/52212)). + + + ## 27.1.0 (2024-03-06) ### Bug Fix diff --git a/packages/components/src/palette-edit/test/index.tsx b/packages/components/src/palette-edit/test/index.tsx index 59571a7947e59..755b360f0770b 100644 --- a/packages/components/src/palette-edit/test/index.tsx +++ b/packages/components/src/palette-edit/test/index.tsx @@ -26,9 +26,10 @@ describe( 'getNameAndSlugForPosition', () => { const slugPrefix = 'test-'; const elements: PaletteElement[] = []; - expect( getNameAndSlugForPosition( elements, slugPrefix ) ).toEqual( - 'Color 1' - ); + expect( getNameAndSlugForPosition( elements, slugPrefix ) ).toEqual( { + name: 'Color 1', + slug: 'test-color-1', + } ); } ); test( 'should return a new color name with an incremented slug id', () => { @@ -41,9 +42,10 @@ describe( 'getNameAndSlugForPosition', () => { }, ]; - expect( getNameAndSlugForPosition( elements, slugPrefix ) ).toEqual( - 'Color 2' - ); + expect( getNameAndSlugForPosition( elements, slugPrefix ) ).toEqual( { + name: 'Color 2', + slug: 'test-color-2', + } ); } ); test( 'should ignore user-defined color names', () => { @@ -56,9 +58,10 @@ describe( 'getNameAndSlugForPosition', () => { }, ]; - expect( getNameAndSlugForPosition( elements, slugPrefix ) ).toEqual( - 'Color 1' - ); + expect( getNameAndSlugForPosition( elements, slugPrefix ) ).toEqual( { + name: 'Color 1', + slug: 'test-color-1', + } ); } ); test( 'should return a new color name with an incremented slug id one higher than the current highest', () => { @@ -86,9 +89,10 @@ describe( 'getNameAndSlugForPosition', () => { }, ]; - expect( getNameAndSlugForPosition( elements, slugPrefix ) ).toEqual( - 'Color 151' - ); + expect( getNameAndSlugForPosition( elements, slugPrefix ) ).toEqual( { + name: 'Color 151', + slug: 'test-color-151', + } ); } ); } ); From 66354288f68f01a8fe7c2f8e6f71aa6f245c56f9 Mon Sep 17 00:00:00 2001 From: megane9988 Date: Thu, 7 Mar 2024 14:46:37 +0800 Subject: [PATCH 3/6] Response to reviews https://github.com/WordPress/gutenberg/pull/52212#discussion_r1515621249 --- packages/components/src/palette-edit/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/src/palette-edit/index.tsx b/packages/components/src/palette-edit/index.tsx index ee3460544aff7..1f2b8159b06af 100644 --- a/packages/components/src/palette-edit/index.tsx +++ b/packages/components/src/palette-edit/index.tsx @@ -108,7 +108,7 @@ export function getNameAndSlugForPosition( __( 'Color %s' ), position ), - slug: slugPrefix + 'color-' + position, + slug: `${ slugPrefix }color-${ position }`, }; } From ad7517677c23cc9b4bc8dddfd2471740e8e64efc Mon Sep 17 00:00:00 2001 From: megane9988 Date: Tue, 19 Mar 2024 00:19:17 +0900 Subject: [PATCH 4/6] Apply suggestions from code review Co-authored-by: Aki Hamano <54422211+t-hamano@users.noreply.github.com> --- packages/components/src/palette-edit/index.tsx | 2 +- packages/components/src/palette-edit/test/index.tsx | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/components/src/palette-edit/index.tsx b/packages/components/src/palette-edit/index.tsx index 1f2b8159b06af..b61904d318446 100644 --- a/packages/components/src/palette-edit/index.tsx +++ b/packages/components/src/palette-edit/index.tsx @@ -82,7 +82,7 @@ function NameInput( { value, onChange, label }: NameInputProps ) { * @param elements An array of color palette items. * @param slugPrefix The slug prefix used to match the element slug. * - * @return {{name:string, slug:string}} A name and slug for the new palette item. + * @return A name and slug for the new palette item. */ export function getNameAndSlugForPosition( elements: PaletteElement[], diff --git a/packages/components/src/palette-edit/test/index.tsx b/packages/components/src/palette-edit/test/index.tsx index 755b360f0770b..7ebdaa77ef0b8 100644 --- a/packages/components/src/palette-edit/test/index.tsx +++ b/packages/components/src/palette-edit/test/index.tsx @@ -32,7 +32,7 @@ describe( 'getNameAndSlugForPosition', () => { } ); } ); - test( 'should return a new color name with an incremented slug id', () => { + test( 'should return a new color name and slug with an incremented slug id', () => { const slugPrefix = 'test-'; const elements = [ { @@ -48,7 +48,7 @@ describe( 'getNameAndSlugForPosition', () => { } ); } ); - test( 'should ignore user-defined color names', () => { + test( 'should ignore user-defined color name and slug', () => { const slugPrefix = 'test-'; const elements = [ { @@ -64,7 +64,7 @@ describe( 'getNameAndSlugForPosition', () => { } ); } ); - test( 'should return a new color name with an incremented slug id one higher than the current highest', () => { + test( 'should return a new color name and slug with an incremented slug id one higher than the current highest', () => { const slugPrefix = 'test-'; const elements = [ { From 0a8653f5c190d9c26a87799c596b074410241dab Mon Sep 17 00:00:00 2001 From: megane9988 Date: Tue, 19 Mar 2024 00:20:31 +0900 Subject: [PATCH 5/6] Update CHANGELOG.md --- 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 bc974275a05ab..afaf301d6bf96 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -13,7 +13,7 @@ - `Button`: Keep deprecated props in type definitions ([#59913](https://github.com/WordPress/gutenberg/pull/59913)). ### Bug Fix -- `PaletteEdit`: Fix order numbers for color palette ([#52212](https://github.com/WordPress/gutenberg/pull/52212)). +- `PaletteEdit`: Fix number incrementing of default names for new colors added in non-en-US locales ([#52212](https://github.com/WordPress/gutenberg/pull/52212)). From 99903e0d92206fbda185d44b1df8a76543bd8d9b Mon Sep 17 00:00:00 2001 From: megane9988 Date: Tue, 19 Mar 2024 00:30:56 +0900 Subject: [PATCH 6/6] fix doc format --- packages/components/src/palette-edit/index.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/components/src/palette-edit/index.tsx b/packages/components/src/palette-edit/index.tsx index b61904d318446..56234ca5c86bc 100644 --- a/packages/components/src/palette-edit/index.tsx +++ b/packages/components/src/palette-edit/index.tsx @@ -79,8 +79,8 @@ function NameInput( { value, onChange, label }: NameInputProps ) { * 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. * - * @param elements An array of color palette items. - * @param slugPrefix The slug prefix used to match the element slug. + * @param elements An array of color palette items. + * @param slugPrefix The slug prefix used to match the element slug. * * @return A name and slug for the new palette item. */