ColorPalette: show Clear button even when colors array is empty (…

…#46001) * Render `CircularOptionPicker` when `colors` is an empty array * Fix Storybook errors when `colors` array is empty * Move `onChange` mock inside each test * Rename `test` to `it` * Extract color constants outside `describe` scope, rename variables, use less US-centric colors * Add test to avoid regressions on `Clear` button not showing when `colors` is an empty array * Split clear button test into two separate tests * CHANGELOG * Remove initial color logic in Storybook example
WordPress · Nov 26, 2022 · 0b1998c · 0b1998c
1 parent 5292d80
commit 0b1998c
Show file tree

Hide file tree

Showing 5 changed files with 78 additions and 48 deletions.
diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md
@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+### Bug Fix
+
+-   `ColorPalette`: show "Clear" button even when colors array is empty ([#46001](https://github.com/WordPress/gutenberg/pull/46001)).
+
 ### Enhancements
 
 -   `TabPanel`: Add ability to set icon only tab buttons ([#45005](https://github.com/WordPress/gutenberg/pull/45005)).

diff --git a/packages/components/src/color-palette/index.tsx b/packages/components/src/color-palette/index.tsx
@@ -84,10 +84,6 @@ function SinglePalette( {
 		} );
 	}, [ colors, value, onChange, clearColor ] );
 
-	if ( colors.length === 0 ) {
-		return null;
-	}
-
 	return (
 		<CircularOptionPicker
 			className={ className }

diff --git a/packages/components/src/color-palette/stories/index.tsx b/packages/components/src/color-palette/stories/index.tsx
@@ -15,7 +15,6 @@ import { useState } from '@wordpress/element';
 import ColorPalette from '..';
 import Popover from '../../popover';
 import { Provider as SlotFillProvider } from '../../slot-fill';
-import type { ColorObject, PaletteObject } from '../types';
 
 const meta: ComponentMeta< typeof ColorPalette > = {
 	title: 'Components/ColorPalette',
@@ -43,10 +42,7 @@ const Template: ComponentStory< typeof ColorPalette > = ( {
 	onChange,
 	...args
 } ) => {
-	const firstColor =
-		( args.colors as ColorObject[] )[ 0 ].color ||
-		( args.colors as PaletteObject[] )[ 0 ].colors[ 0 ].color;
-	const [ color, setColor ] = useState< string | undefined >( firstColor );
+	const [ color, setColor ] = useState< string | undefined >();
 
 	return (
 		<SlotFillProvider>

diff --git a/packages/components/src/color-palette/test/__snapshots__/index.tsx.snap b/packages/components/src/color-palette/test/__snapshots__/index.tsx.snap
@@ -63,10 +63,10 @@ exports[`ColorPalette should allow disabling custom color picker 1`] = `
           class="components-circular-option-picker__option-wrapper"
         >
           <button
-            aria-label="Color: white"
+            aria-label="Color: green"
             aria-pressed="false"
             class="components-button components-circular-option-picker__option"
-            style="background-color: rgb(255, 255, 255); color: rgb(255, 255, 255);"
+            style="background-color: rgb(0, 255, 0); color: rgb(0, 255, 0);"
             type="button"
           />
         </div>
@@ -235,10 +235,10 @@ exports[`ColorPalette should render a dynamic toolbar of colors 1`] = `
           class="components-circular-option-picker__option-wrapper"
         >
           <button
-            aria-label="Color: white"
+            aria-label="Color: green"
             aria-pressed="false"
             class="components-button components-circular-option-picker__option"
-            style="background-color: rgb(255, 255, 255); color: rgb(255, 255, 255);"
+            style="background-color: rgb(0, 255, 0); color: rgb(0, 255, 0);"
             type="button"
           />
         </div>

diff --git a/packages/components/src/color-palette/test/index.tsx b/packages/components/src/color-palette/test/index.tsx
@@ -9,36 +9,35 @@ import userEvent from '@testing-library/user-event';
  */
 import ColorPalette from '..';
 
+const EXAMPLE_COLORS = [
+	{ name: 'red', color: '#f00' },
+	{ name: 'green', color: '#0f0' },
+	{ name: 'blue', color: '#00f' },
+];
+const INITIAL_COLOR = EXAMPLE_COLORS[ 0 ].color;
+
 describe( 'ColorPalette', () => {
-	const colors = [
-		{ name: 'red', color: '#f00' },
-		{ name: 'white', color: '#fff' },
-		{ name: 'blue', color: '#00f' },
-	];
-	const currentColor = '#f00';
-	const onChange = jest.fn();
-
-	beforeEach( () => {
-		onChange.mockClear();
-	} );
+	it( 'should render a dynamic toolbar of colors', () => {
+		const onChange = jest.fn();
 
-	test( 'should render a dynamic toolbar of colors', () => {
 		const { container } = render(
 			<ColorPalette
-				colors={ colors }
-				value={ currentColor }
+				colors={ EXAMPLE_COLORS }
+				value={ INITIAL_COLOR }
 				onChange={ onChange }
 			/>
 		);
 
 		expect( container ).toMatchSnapshot();
 	} );
 
-	test( 'should render three color button options', () => {
+	it( 'should render three color button options', () => {
+		const onChange = jest.fn();
+
 		render(
 			<ColorPalette
-				colors={ colors }
-				value={ currentColor }
+				colors={ EXAMPLE_COLORS }
+				value={ INITIAL_COLOR }
 				onChange={ onChange }
 			/>
 		);
@@ -48,15 +47,16 @@ describe( 'ColorPalette', () => {
 		).toHaveLength( 3 );
 	} );
 
-	test( 'should call onClick on an active button with undefined', async () => {
+	it( 'should call onClick on an active button with undefined', async () => {
 		const user = userEvent.setup( {
 			advanceTimers: jest.advanceTimersByTime,
 		} );
+		const onChange = jest.fn();
 
 		render(
 			<ColorPalette
-				colors={ colors }
-				value={ currentColor }
+				colors={ EXAMPLE_COLORS }
+				value={ INITIAL_COLOR }
 				onChange={ onChange }
 			/>
 		);
@@ -69,39 +69,44 @@ describe( 'ColorPalette', () => {
 		expect( onChange ).toHaveBeenCalledWith( undefined );
 	} );
 
-	test( 'should call onClick on an inactive button', async () => {
+	it( 'should call onClick on an inactive button', async () => {
 		const user = userEvent.setup( {
 			advanceTimers: jest.advanceTimersByTime,
 		} );
+		const onChange = jest.fn();
 
 		render(
 			<ColorPalette
-				colors={ colors }
-				value={ currentColor }
+				colors={ EXAMPLE_COLORS }
+				value={ INITIAL_COLOR }
 				onChange={ onChange }
 			/>
 		);
 
+		// Click the first unpressed button
+		// (i.e. a button representing a color that is not the current color)
 		await user.click(
 			screen.getAllByRole( 'button', {
 				name: /^Color:/,
 				pressed: false,
 			} )[ 0 ]
 		);
 
+		// Expect the green color to have been selected
 		expect( onChange ).toHaveBeenCalledTimes( 1 );
-		expect( onChange ).toHaveBeenCalledWith( '#fff', 1 );
+		expect( onChange ).toHaveBeenCalledWith( EXAMPLE_COLORS[ 1 ].color, 1 );
 	} );
 
-	test( 'should call onClick with undefined, when the clearButton onClick is triggered', async () => {
+	it( 'should call onClick with undefined, when the clearButton onClick is triggered', async () => {
 		const user = userEvent.setup( {
 			advanceTimers: jest.advanceTimersByTime,
 		} );
+		const onChange = jest.fn();
 
 		render(
 			<ColorPalette
-				colors={ colors }
-				value={ currentColor }
+				colors={ EXAMPLE_COLORS }
+				value={ INITIAL_COLOR }
 				onChange={ onChange }
 			/>
 		);
@@ -112,28 +117,31 @@ describe( 'ColorPalette', () => {
 		expect( onChange ).toHaveBeenCalledWith( undefined );
 	} );
 
-	test( 'should allow disabling custom color picker', () => {
+	it( 'should allow disabling custom color picker', () => {
+		const onChange = jest.fn();
+
 		const { container } = render(
 			<ColorPalette
-				colors={ colors }
+				colors={ EXAMPLE_COLORS }
 				disableCustomColors
-				value={ currentColor }
+				value={ INITIAL_COLOR }
 				onChange={ onChange }
 			/>
 		);
 
 		expect( container ).toMatchSnapshot();
 	} );
 
-	test( 'should render dropdown and its content', async () => {
+	it( 'should render dropdown and its content', async () => {
 		const user = userEvent.setup( {
 			advanceTimers: jest.advanceTimersByTime,
 		} );
+		const onChange = jest.fn();
 
 		render(
 			<ColorPalette
-				colors={ colors }
-				value={ currentColor }
+				colors={ EXAMPLE_COLORS }
+				value={ INITIAL_COLOR }
 				onChange={ onChange }
 			/>
 		);
@@ -153,12 +161,38 @@ describe( 'ColorPalette', () => {
 		expect( dropdownButton ).toBeVisible();
 
 		expect(
-			within( dropdownButton ).getByText( colors[ 0 ].name )
+			within( dropdownButton ).getByText( EXAMPLE_COLORS[ 0 ].name )
 		).toBeVisible();
 		expect(
 			within( dropdownButton ).getByText(
-				colors[ 0 ].color.replace( '#', '' )
+				EXAMPLE_COLORS[ 0 ].color.replace( '#', '' )
 			)
 		).toBeVisible();
 	} );
+
+	it( 'should show the clear button by default', () => {
+		const onChange = jest.fn();
+
+		render(
+			<ColorPalette
+				colors={ EXAMPLE_COLORS }
+				value={ INITIAL_COLOR }
+				onChange={ onChange }
+			/>
+		);
+
+		expect(
+			screen.getByRole( 'button', { name: 'Clear' } )
+		).toBeInTheDocument();
+	} );
+
+	it( 'should show the clear button even when `colors` is an empty array', () => {
+		const onChange = jest.fn();
+
+		render( <ColorPalette colors={ [] } onChange={ onChange } /> );
+
+		expect(
+			screen.getByRole( 'button', { name: 'Clear' } )
+		).toBeInTheDocument();
+	} );
 } );