Skip to content

Commit 3434e37

Browse files
Andrew Haywardciampo
Andrew Hayward
and
ciampo
authored
Making Circular Option Picker a listbox (#52255)
* Making Circular Option Picker a `listbox` Using the `Composite` component to make the Circular Option Picker present as a listbox. * Various updates – Adding support for RTL - Adding support for preselected values - Adjusting how groups behave - Addressing some feedback * Fixing tests for `core/cover` * Fixing tests for `BorderControl` * Fixing tests for `BorderControl` * Adding default values for ARIA labels Using `useEffect` to fix updates during render cycle * Removing redundant `ColorPalette` test * Updating custom color tests * Removing redundant snapshots * Updating broken snapshot * Updating CHANGELOG.md * Fixing various E2E tests * Updating test for new keyboard navigation * Updating E2E tests * Fixing navigation-colors E2E test * Fixing E2E tests for cover and heading blocks * Removing `option` constant * Removing redundant type coercion * Enforcing mutual exclusivity of `-label`/`-labelledby` * Making label exclusivity clearer * Updating CHANGELOG.md * Fixing broken test --------- Co-authored-by: Marco Ciampini <[email protected]>
1 parent f71f153 commit 3434e37

File tree

24 files changed

+405
-430
lines changed

24 files changed

+405
-430
lines changed

‎packages/block-editor/src/components/color-palette/test/__snapshots__/control.js.snap

+29-19
Original file line numberDiff line numberDiff line change
@@ -199,34 +199,44 @@ exports[`ColorPaletteControl matches the snapshot 1`] = `
199199
</div>
200200
</div>
201201
<div
202+
aria-label="Custom color picker."
202203
class="components-circular-option-picker"
204+
id="option-picker-0"
205+
role="listbox"
203206
>
204207
<div
205208
class="components-circular-option-picker__swatches"
206209
>
207210
<div
208-
class="components-circular-option-picker__option-wrapper"
211+
class="components-circular-option-picker__option-group components-circular-option-picker__swatches"
209212
>
210-
<button
211-
aria-label="Color: red"
212-
aria-pressed="true"
213-
class="components-button components-circular-option-picker__option is-pressed"
214-
style="background-color: rgb(255, 0, 0); color: rgb(255, 0, 0);"
215-
type="button"
216-
/>
217-
<svg
218-
aria-hidden="true"
219-
fill="#000"
220-
focusable="false"
221-
height="24"
222-
viewBox="0 0 24 24"
223-
width="24"
224-
xmlns="http://www.w3.org/2000/svg"
213+
<div
214+
class="components-circular-option-picker__option-wrapper"
225215
>
226-
<path
227-
d="M16.7 7.1l-6.3 8.5-3.3-2.5-.9 1.2 4.5 3.4L17.9 8z"
216+
<button
217+
aria-label="Color: red"
218+
aria-selected="true"
219+
class="components-button components-circular-option-picker__option is-pressed"
220+
id="option-picker-0-0"
221+
role="option"
222+
style="background-color: rgb(255, 0, 0); color: rgb(255, 0, 0);"
223+
tabindex="0"
224+
type="button"
228225
/>
229-
</svg>
226+
<svg
227+
aria-hidden="true"
228+
fill="#000"
229+
focusable="false"
230+
height="24"
231+
viewBox="0 0 24 24"
232+
width="24"
233+
xmlns="http://www.w3.org/2000/svg"
234+
>
235+
<path
236+
d="M16.7 7.1l-6.3 8.5-3.3-2.5-.9 1.2 4.5 3.4L17.9 8z"
237+
/>
238+
</svg>
239+
</div>
230240
</div>
231241
</div>
232242
<div

‎packages/block-library/src/cover/test/edit.js

+7-7
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ async function setup( attributes, useCoreBlocks, customSettings ) {
4747

4848
async function createAndSelectBlock() {
4949
await userEvent.click(
50-
screen.getByRole( 'button', {
50+
screen.getByRole( 'option', {
5151
name: 'Color: Black',
5252
} )
5353
);
@@ -72,7 +72,7 @@ describe( 'Cover block', () => {
7272

7373
test( 'can set overlay color using color picker on block placeholder', async () => {
7474
const { container } = await setup();
75-
const colorPicker = screen.getByRole( 'button', {
75+
const colorPicker = screen.getByRole( 'option', {
7676
name: 'Color: Black',
7777
} );
7878
await userEvent.click( colorPicker );
@@ -96,7 +96,7 @@ describe( 'Cover block', () => {
9696
await setup();
9797

9898
await userEvent.click(
99-
screen.getByRole( 'button', {
99+
screen.getByRole( 'option', {
100100
name: 'Color: Black',
101101
} )
102102
);
@@ -389,7 +389,7 @@ describe( 'Cover block', () => {
389389
describe( 'isDark settings', () => {
390390
test( 'should toggle is-light class if background changed from light to dark', async () => {
391391
await setup();
392-
const colorPicker = screen.getByRole( 'button', {
392+
const colorPicker = screen.getByRole( 'option', {
393393
name: 'Color: White',
394394
} );
395395
await userEvent.click( colorPicker );
@@ -405,15 +405,15 @@ describe( 'Cover block', () => {
405405
} )
406406
);
407407
await userEvent.click( screen.getByText( 'Overlay' ) );
408-
const popupColorPicker = screen.getByRole( 'button', {
408+
const popupColorPicker = screen.getByRole( 'option', {
409409
name: 'Color: Black',
410410
} );
411411
await userEvent.click( popupColorPicker );
412412
expect( coverBlock ).not.toHaveClass( 'is-light' );
413413
} );
414414
test( 'should remove is-light class if overlay color is removed', async () => {
415415
await setup();
416-
const colorPicker = screen.getByRole( 'button', {
416+
const colorPicker = screen.getByRole( 'option', {
417417
name: 'Color: White',
418418
} );
419419
await userEvent.click( colorPicker );
@@ -428,7 +428,7 @@ describe( 'Cover block', () => {
428428
await userEvent.click( screen.getByText( 'Overlay' ) );
429429
// The default color is black, so clicking the black color option will remove the background color,
430430
// which should remove the isDark setting and assign the is-light class.
431-
const popupColorPicker = screen.getByRole( 'button', {
431+
const popupColorPicker = screen.getByRole( 'option', {
432432
name: 'Color: White',
433433
} );
434434
await userEvent.click( popupColorPicker );

‎packages/components/CHANGELOG.md

+4
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@
66

77
- Make the `Popover.Slot` optional and render popovers at the bottom of the document's body by default. ([#53889](https://github.com/WordPress/gutenberg/pull/53889), [#53982](https://github.com/WordPress/gutenberg/pull/53982)).
88

9+
### Enhancements
10+
11+
- Making Circular Option Picker a `listbox`. Note that while this changes some public API, new props are optional, and currently have default values; this will change in another patch. ([#52255](https://github.com/WordPress/gutenberg/pull/52255))
12+
913
### Bug Fix
1014

1115
- `PaletteEdit`: Fix padding in RTL languages ([#54034](https://github.com/WordPress/gutenberg/pull/54034)).

‎packages/components/src/border-control/test/index.js

+17-9
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,10 @@ const getButton = ( name ) => {
6767
return screen.getByRole( 'button', { name } );
6868
};
6969

70+
const getColorOption = ( name ) => {
71+
return screen.getByRole( 'option', { name } );
72+
};
73+
7074
const queryButton = ( name ) => {
7175
return screen.queryByRole( 'button', { name } );
7276
};
@@ -75,6 +79,10 @@ const clickButton = ( name ) => {
7579
fireEvent.click( getButton( name ) );
7680
};
7781

82+
const selectColorOption = ( name ) => {
83+
fireEvent.click( getColorOption( name ) );
84+
};
85+
7886
const getSliderInput = () => {
7987
return screen.getByRole( 'slider', { name: 'Border width' } );
8088
};
@@ -151,7 +159,7 @@ describe( 'BorderControl', () => {
151159
await openPopover();
152160

153161
const customColorPicker = getButton( /Custom color picker/ );
154-
const colorSwatchButtons = screen.getAllByRole( 'button', {
162+
const colorSwatchButtons = screen.getAllByRole( 'option', {
155163
name: /^Color:/,
156164
} );
157165
const styleLabel = screen.getByText( 'Style' );
@@ -324,7 +332,7 @@ describe( 'BorderControl', () => {
324332
const props = createProps();
325333
render( <BorderControl { ...props } /> );
326334
await openPopover();
327-
clickButton( 'Color: Green' );
335+
selectColorOption( 'Color: Green' );
328336

329337
expect( props.onChange ).toHaveBeenNthCalledWith( 1, {
330338
...defaultBorder,
@@ -336,7 +344,7 @@ describe( 'BorderControl', () => {
336344
const props = createProps();
337345
render( <BorderControl { ...props } /> );
338346
await openPopover();
339-
clickButton( 'Color: Blue' );
347+
selectColorOption( 'Color: Blue' );
340348

341349
expect( props.onChange ).toHaveBeenNthCalledWith( 1, {
342350
...defaultBorder,
@@ -384,7 +392,7 @@ describe( 'BorderControl', () => {
384392
clearWidthInput();
385393
rerender( <BorderControl { ...props } /> );
386394
await openPopover();
387-
clickButton( 'Color: Blue' );
395+
selectColorOption( 'Color: Blue' );
388396

389397
expect( props.onChange ).toHaveBeenCalledWith( undefined );
390398
} );
@@ -397,7 +405,7 @@ describe( 'BorderControl', () => {
397405
clearWidthInput();
398406
rerender( <BorderControl { ...props } /> );
399407
await openPopover();
400-
clickButton( 'Color: Blue' );
408+
selectColorOption( 'Color: Blue' );
401409

402410
expect( props.onChange ).toHaveBeenNthCalledWith( 2, {
403411
color: undefined,
@@ -410,7 +418,7 @@ describe( 'BorderControl', () => {
410418
const props = createProps();
411419
render( <BorderControl { ...props } /> );
412420
await openPopover();
413-
clickButton( 'Color: Green' );
421+
selectColorOption( 'Color: Green' );
414422
clickButton( 'Dotted' );
415423
setWidthInput( '0' );
416424

@@ -425,7 +433,7 @@ describe( 'BorderControl', () => {
425433
const props = createProps();
426434
const { rerender } = render( <BorderControl { ...props } /> );
427435
await openPopover();
428-
clickButton( 'Color: Green' );
436+
selectColorOption( 'Color: Green' );
429437
rerender( <BorderControl { ...props } /> );
430438
clickButton( 'Dotted' );
431439
rerender( <BorderControl { ...props } /> );
@@ -443,7 +451,7 @@ describe( 'BorderControl', () => {
443451
const props = createProps( { value: undefined } );
444452
const { rerender } = render( <BorderControl { ...props } /> );
445453
await openPopover();
446-
clickButton( 'Color: Yellow' );
454+
selectColorOption( 'Color: Yellow' );
447455

448456
expect( props.onChange ).toHaveBeenCalledWith( {
449457
color: '#bd8600',
@@ -453,7 +461,7 @@ describe( 'BorderControl', () => {
453461

454462
setWidthInput( '0' );
455463
rerender( <BorderControl { ...props } /> );
456-
clickButton( 'Color: Green' );
464+
selectColorOption( 'Color: Green' );
457465

458466
expect( props.onChange ).toHaveBeenCalledWith( {
459467
color: '#00a32a',

0 commit comments

Comments
 (0)