Site editor: preset custom color duplicates (#40837)

* Initial commit
Check the slugs for the highest id number and add one.

* Ensure that color option keys are unique, even where colors are the same

* Updating test snapshots
Adding tests for getNameForPosition()

* Roll back unique keys test

* Update packages/components/src/palette-edit/index.js

Update doc comments

Co-authored-by: Glen Davies <[email protected]>

* Extra tests

Co-authored-by: Glen Davies <[email protected]>

packages/components/src/palette-edit/index.js

@@ -56,11 +56,36 @@ function NameInput( { value, onChange, label } ) {
  );
 }
 
-function getNameForPosition( position ) {
+/**
+ * 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.
+ * 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 {string} elements An array of color palette items.
+ * @param {string} slugPrefix The slug prefix used to match the element slug.
+ *
+ * @return {string} A unique name for a palette item.
+ */
+export function getNameForPosition( elements, slugPrefix ) {
+ const temporaryNameRegex = new RegExp( `^${ slugPrefix }color-([\\d]+)$` );
+ const position = elements.reduce( ( previousValue, currentValue ) => {
+ if ( typeof currentValue?.slug === 'string' ) {
+ const matches = currentValue?.slug.match( temporaryNameRegex );
+ if ( matches ) {
+ const id = parseInt( matches[ 1 ], 10 );
+ if ( id >= previousValue ) {
+ return id + 1;
+ }
+ }
+ }
+ return previousValue;
+ }, 1 );
+
  return sprintf(
  /* translators: %s: is a temporary id for a custom color */
- __( 'Color %s ' ),
- position + 1
+ __( 'Color %s' ),
+ position
  );
 }
 
@@ -163,9 +188,10 @@ function Option( {
  );
 }
 
-function isTemporaryElement( slugPrefix, { slug, color, gradient }, index ) {
+function isTemporaryElement( slugPrefix, { slug, color, gradient } ) {
+ const regex = new RegExp( `^${ slugPrefix }color-([\\d]+)$` );
  return (
- slug === slugPrefix + kebabCase( getNameForPosition( index ) ) &&
+ regex.test( slug ) &&
  ( ( !! color && color === DEFAULT_COLOR ) ||
  ( !! gradient && gradient === DEFAULT_GRADIENT ) )
  );
@@ -193,8 +219,7 @@ function PaletteEditListView( {
  )
  ) {
  const newElements = elementsReference.current.filter(
- ( element, index ) =>
- ! isTemporaryElement( slugPrefix, element, index )
+ ( element ) => ! isTemporaryElement( slugPrefix, element )
  );
  onChange( newElements.length ? newElements : undefined );
  }
@@ -309,8 +334,10 @@ export default function PaletteEdit( {
  }
  onClick={ () => {
  const tempOptionName = getNameForPosition(
- elementsLength
+ elements,
+ slugPrefix
  );
+
  onChange( [
  ...elements,
  {

packages/components/src/palette-edit/test/index.js

@@ -0,0 +1,63 @@
+/**
+ * Internal dependencies
+ */
+import { getNameForPosition } from '../';
+
+describe( 'getNameForPosition', () => {
+ test( 'should return 1 by default', () => {
+ const slugPrefix = 'test-';
+ const elements = [];
+
+ expect( getNameForPosition( elements, slugPrefix ) ).toEqual(
+ 'Color 1'
+ );
+ } );
+
+ test( 'should return a new color name with an incremented slug id', () => {
+ const slugPrefix = 'test-';
+ const elements = [
+ {
+ slug: 'test-color-1',
+ },
+ ];
+
+ expect( getNameForPosition( elements, slugPrefix ) ).toEqual(
+ 'Color 2'
+ );
+ } );
+
+ test( 'should ignore user-defined color names', () => {
+ const slugPrefix = 'test-';
+ const elements = [
+ {
+ slug: 'a-sweet-color-2',
+ },
+ ];
+
+ expect( getNameForPosition( elements, slugPrefix ) ).toEqual(
+ 'Color 1'
+ );
+ } );
+
+ test( 'should return a new color name with an incremented slug id one higher than the current highest', () => {
+ const slugPrefix = 'test-';
+ const elements = [
+ {
+ slug: 'test-color-1',
+ },
+ {
+ slug: 'test-color-2',
+ },
+ {
+ slug: 'test-color-150',
+ },
+ {
+ slug: 'a-sweet-color-100',
+ },
+ ];
+
+ expect( getNameForPosition( elements, slugPrefix ) ).toEqual(
+ 'Color 151'
+ );
+ } );
+} );