Skip to content

Commit

Permalink
Site editor: preset custom color duplicates (#40837)
Browse files Browse the repository at this point in the history
* 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 <glendaviesnz@users.noreply.github.com>

* Extra tests

Co-authored-by: Glen Davies <glendaviesnz@users.noreply.github.com>
  • Loading branch information
ramonjd and glendaviesnz authored May 6, 2022
1 parent 49c80fb commit bcc497b
Show file tree
Hide file tree
Showing 2 changed files with 98 additions and 8 deletions.
43 changes: 35 additions & 8 deletions packages/components/src/palette-edit/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
}

Expand Down Expand Up @@ -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 ) )
);
Expand Down Expand Up @@ -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 );
}
Expand Down Expand Up @@ -309,8 +334,10 @@ export default function PaletteEdit( {
}
onClick={ () => {
const tempOptionName = getNameForPosition(
elementsLength
elements,
slugPrefix
);

onChange( [
...elements,
{
Expand Down
63 changes: 63 additions & 0 deletions packages/components/src/palette-edit/test/index.js
Original file line number Diff line number Diff line change
@@ -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'
);
} );
} );

0 comments on commit bcc497b

Please sign in to comment.