Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ColorPalette: show Clear button even when colors array is empty #46001

Merged
merged 10 commits into from
Nov 26, 2022
4 changes: 4 additions & 0 deletions packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)).
Expand Down
4 changes: 0 additions & 4 deletions packages/components/src/color-palette/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,6 @@ function SinglePalette( {
} );
}, [ colors, value, onChange, clearColor ] );

if ( colors.length === 0 ) {
return null;
}

return (
<CircularOptionPicker
className={ className }
Expand Down
6 changes: 1 addition & 5 deletions packages/components/src/color-palette/stories/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Expand Down
104 changes: 69 additions & 35 deletions packages/components/src/color-palette/test/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
/>
);
Expand All @@ -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 }
/>
);
Expand All @@ -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 }
/>
);
Expand All @@ -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 }
/>
);
Expand All @@ -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 } /> );
ciampo marked this conversation as resolved.
Show resolved Hide resolved

expect(
screen.getByRole( 'button', { name: 'Clear' } )
).toBeInTheDocument();
} );
} );