Skip to content

Commit

Permalink
PaletteEdit: dedupe palette element slugs (#65772)
Browse files Browse the repository at this point in the history
* First pass at deduping palette element slugs.

* Addressing TypeScript errors where PaletteEditListViewProps is using a generic type parameter. The linter was complaining that I wasn't doing the same, even though I was using the same inherited types. Whatever. It could probably be optimized, but it's unrelated to the scope of this PR

* Add changelog

* Make use of PaletteElement type consistent

* Address feedback on deduplicate util

* Use count rather than index for new slugs

---------

Co-authored-by: ramonjd <ramonopoly@git.wordpress.org>
Co-authored-by: aaronrobertshaw <aaronrobertshaw@git.wordpress.org>
Co-authored-by: ciampo <mciampini@git.wordpress.org>
  • Loading branch information
4 people committed Oct 7, 2024
1 parent 21b3c2e commit 0b8e062
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 7 deletions.
8 changes: 7 additions & 1 deletion packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@

### Enhancements

- `Guide`: Update finish button to use the new default size ([#65680](https://github.com/WordPress/gutenberg/pull/65680)).
- `PaletteEdit`: dedupe palette element slugs ([#65772](https://github.com/WordPress/gutenberg/pull/65772)).

## 28.9.0 (2024-10-03)

### Bug Fixes

Expand All @@ -15,6 +17,10 @@
- `Composite`: make items tabbable if active element gets removed ([#65720](https://github.com/WordPress/gutenberg/pull/65720)).
- `DatePicker`: Use compact button size. ([#65653](https://github.com/WordPress/gutenberg/pull/65653)).

### Enhancements

- `Guide`: Update finish button to use the new default size ([#65680](https://github.com/WordPress/gutenberg/pull/65680)).

## 28.8.0 (2024-09-19)

### Bug Fixes
Expand Down
35 changes: 30 additions & 5 deletions packages/components/src/palette-edit/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ import { kebabCase } from '../utils/strings';
import type {
Color,
ColorPickerPopoverProps,
Gradient,
NameInputProps,
OptionProps,
PaletteEditListViewProps,
Expand All @@ -70,6 +69,28 @@ function NameInput( { value, onChange, label }: NameInputProps ) {
);
}

/*
* Deduplicates the slugs of the provided elements.
*/
export function deduplicateElementSlugs< T extends PaletteElement >(
elements: T[]
) {
const slugCounts: { [ slug: string ]: number } = {};

return elements.map( ( element ) => {
let newSlug: string | undefined;

const { slug } = element;
slugCounts[ slug ] = ( slugCounts[ slug ] || 0 ) + 1;

if ( slugCounts[ slug ] > 1 ) {
newSlug = `${ slug }-${ slugCounts[ slug ] - 1 }`;
}

return { ...element, slug: newSlug ?? slug };
} );
}

/**
* 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.
Expand Down Expand Up @@ -109,7 +130,7 @@ export function getNameAndSlugForPosition(
};
}

function ColorPickerPopover< T extends Color | Gradient >( {
function ColorPickerPopover< T extends PaletteElement >( {
isGradient,
element,
onChange,
Expand Down Expand Up @@ -167,7 +188,7 @@ function ColorPickerPopover< T extends Color | Gradient >( {
);
}

function Option< T extends Color | Gradient >( {
function Option< T extends PaletteElement >( {
canOnlyChangeValues,
element,
onChange,
Expand Down Expand Up @@ -265,7 +286,7 @@ function Option< T extends Color | Gradient >( {
);
}

function PaletteEditListView< T extends Color | Gradient >( {
function PaletteEditListView< T extends PaletteElement >( {
elements,
onChange,
canOnlyChangeValues,
Expand All @@ -280,7 +301,11 @@ function PaletteEditListView< T extends Color | Gradient >( {
elementsReferenceRef.current = elements;
}, [ elements ] );

const debounceOnChange = useDebounce( onChange, 100 );
const debounceOnChange = useDebounce(
( updatedElements: T[] ) =>
onChange( deduplicateElementSlugs( updatedElements ) ),
100
);

return (
<VStack spacing={ 3 }>
Expand Down
51 changes: 50 additions & 1 deletion packages/components/src/palette-edit/test/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ import { click, type, press } from '@ariakit/test';
/**
* Internal dependencies
*/
import PaletteEdit, { getNameAndSlugForPosition } from '..';
import PaletteEdit, {
getNameAndSlugForPosition,
deduplicateElementSlugs,
} from '..';
import type { PaletteElement } from '../types';

const noop = () => {};
Expand Down Expand Up @@ -97,6 +100,52 @@ describe( 'getNameAndSlugForPosition', () => {
} );
} );

describe( 'deduplicateElementSlugs', () => {
it( 'should not change the slugs if they are unique', () => {
const elements: PaletteElement[] = [
{
slug: 'test-color-1',
color: '#ffffff',
name: 'Test Color 1',
},
{
slug: 'test-color-2',
color: '#1a4548',
name: 'Test Color 2',
},
];

expect( deduplicateElementSlugs( elements ) ).toEqual( elements );
} );
it( 'should change the slugs if they are not unique', () => {
const elements: PaletteElement[] = [
{
slug: 'test-color-1',
color: '#ffffff',
name: 'Test Color 1',
},
{
slug: 'test-color-1',
color: '#1a4548',
name: 'Test Color 2',
},
];

expect( deduplicateElementSlugs( elements ) ).toEqual( [
{
slug: 'test-color-1',
color: '#ffffff',
name: 'Test Color 1',
},
{
slug: 'test-color-1-1',
color: '#1a4548',
name: 'Test Color 2',
},
] );
} );
} );

describe( 'PaletteEdit', () => {
const defaultProps = {
paletteLabel: 'Test label',
Expand Down

0 comments on commit 0b8e062

Please sign in to comment.