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.

Sign up for GitHub

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

Making Circular Option Picker a listbox #52255

Merged
merged 29 commits into from
Sep 4, 2023
Merged

Making Circular Option Picker a listbox #52255

Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
9232f14
Making Circular Option Picker a `listbox`
Jul 3, 2023
1f87fc5
Merge branch 'WordPress:trunk' into 35292/improve_color_palette_keybo…
andrewhayward Aug 1, 2023
309e523
Various updates
Aug 15, 2023
84ac262
Merge branch 'trunk' into 35292/improve_color_palette_keyboard_access…
Aug 15, 2023
37016b3
Merge branch 'trunk' into 35290/improve_color_palette_keyboard_access…
Aug 21, 2023
72e8889
Fixing tests for `core/cover`
Aug 21, 2023
d932389
Fixing tests for `BorderControl`
Aug 21, 2023
b331d7c
Fixing tests for `BorderControl`
Aug 23, 2023
a90e82f
Adding default values for ARIA labels
Aug 29, 2023
142b7c2
Fixing merge conflict
Aug 29, 2023
7336b69
Removing redundant `ColorPalette` test
Aug 29, 2023
3692ccd
Updating custom color tests
Aug 29, 2023
fc000ee
Removing redundant snapshots
Aug 29, 2023
17f8837
Updating broken snapshot
Aug 29, 2023
a61af94
Merge branch 'trunk' into 35292/improve_color_palette_keyboard_access…
Aug 29, 2023
5385c6e
Updating CHANGELOG.md
Aug 29, 2023
eb6658a
Fixing various E2E tests
Aug 29, 2023
718f81f
Updating test for new keyboard navigation
Aug 29, 2023
d20d985
Updating E2E tests
Aug 29, 2023
432af69
Fixing navigation-colors E2E test
Aug 30, 2023
65a74fb
Fixing E2E tests for cover and heading blocks
Aug 30, 2023
01693bb
Removing `option` constant
Aug 30, 2023
aaa8c45
Removing redundant type coercion
Sep 1, 2023
149c4da
Enforcing mutual exclusivity of `-label`/`-labelledby`
Sep 1, 2023
8c22df2
Making label exclusivity clearer
Sep 1, 2023
78d88d0
Merge branch 'trunk' into 35292/improve_color_palette_keyboard_access…
Sep 1, 2023
e808582
Updating CHANGELOG.md
Sep 1, 2023
b15497f
Fixing broken test
Sep 1, 2023
717469b
Merge branch 'trunk' into 35292/improve_color_palette_keyboard_access…
ciampo Sep 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 29 additions & 19 deletions packages/block-editor/src/components/color-palette/test/__snapshots__/control.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,34 +199,44 @@ exports[`ColorPaletteControl matches the snapshot 1`] = `
</div>
</div>
<div
aria-label="Custom color picker."
class="components-circular-option-picker"
id="option-picker-0"
role="listbox"
>
<div
class="components-circular-option-picker__swatches"
>
<div
class="components-circular-option-picker__option-wrapper"
class="components-circular-option-picker__option-group components-circular-option-picker__swatches"
>
<button
aria-label="Color: red"
aria-pressed="true"
class="components-button components-circular-option-picker__option is-pressed"
style="background-color: rgb(255, 0, 0); color: rgb(255, 0, 0);"
type="button"
/>
<svg
aria-hidden="true"
fill="#000"
focusable="false"
height="24"
viewBox="0 0 24 24"
width="24"
xmlns="http://www.w3.org/2000/svg"
<div
class="components-circular-option-picker__option-wrapper"
>
<path
d="M16.7 7.1l-6.3 8.5-3.3-2.5-.9 1.2 4.5 3.4L17.9 8z"
<button
aria-label="Color: red"
aria-selected="true"
class="components-button components-circular-option-picker__option is-pressed"
id="option-picker-0-0"
role="option"
style="background-color: rgb(255, 0, 0); color: rgb(255, 0, 0);"
tabindex="0"
type="button"
/>
</svg>
<svg
aria-hidden="true"
fill="#000"
focusable="false"
height="24"
viewBox="0 0 24 24"
width="24"
xmlns="http://www.w3.org/2000/svg"
>
<path
d="M16.7 7.1l-6.3 8.5-3.3-2.5-.9 1.2 4.5 3.4L17.9 8z"
/>
</svg>
</div>
</div>
</div>
<div
Expand Down
14 changes: 7 additions & 7 deletions packages/block-library/src/cover/test/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ async function setup( attributes, useCoreBlocks, customSettings ) {

async function createAndSelectBlock() {
await userEvent.click(
screen.getByRole( 'button', {
screen.getByRole( 'option', {
name: 'Color: Black',
} )
);
Expand All @@ -72,7 +72,7 @@ describe( 'Cover block', () => {

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

await userEvent.click(
screen.getByRole( 'button', {
screen.getByRole( 'option', {
name: 'Color: Black',
} )
);
Expand Down Expand Up @@ -389,7 +389,7 @@ describe( 'Cover block', () => {
describe( 'isDark settings', () => {
test( 'should toggle is-light class if background changed from light to dark', async () => {
await setup();
const colorPicker = screen.getByRole( 'button', {
const colorPicker = screen.getByRole( 'option', {
name: 'Color: White',
} );
await userEvent.click( colorPicker );
Expand All @@ -405,15 +405,15 @@ describe( 'Cover block', () => {
} )
);
await userEvent.click( screen.getByText( 'Overlay' ) );
const popupColorPicker = screen.getByRole( 'button', {
const popupColorPicker = screen.getByRole( 'option', {
name: 'Color: Black',
} );
await userEvent.click( popupColorPicker );
expect( coverBlock ).not.toHaveClass( 'is-light' );
} );
test( 'should remove is-light class if overlay color is removed', async () => {
await setup();
const colorPicker = screen.getByRole( 'button', {
const colorPicker = screen.getByRole( 'option', {
name: 'Color: White',
} );
await userEvent.click( colorPicker );
Expand All @@ -428,7 +428,7 @@ describe( 'Cover block', () => {
await userEvent.click( screen.getByText( 'Overlay' ) );
// The default color is black, so clicking the black color option will remove the background color,
// which should remove the isDark setting and assign the is-light class.
const popupColorPicker = screen.getByRole( 'button', {
const popupColorPicker = screen.getByRole( 'option', {
name: 'Color: White',
} );
await userEvent.click( popupColorPicker );
Expand Down
4 changes: 4 additions & 0 deletions packages/components/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@

- 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)).

### Enhancements

- 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))

## 25.7.0 (2023-08-31)

### Enhancements
Expand Down
26 changes: 17 additions & 9 deletions packages/components/src/border-control/test/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,10 @@ const getButton = ( name ) => {
return screen.getByRole( 'button', { name } );
};

const getColorOption = ( name ) => {
return screen.getByRole( 'option', { name } );
};

const queryButton = ( name ) => {
return screen.queryByRole( 'button', { name } );
};
Expand All @@ -75,6 +79,10 @@ const clickButton = ( name ) => {
fireEvent.click( getButton( name ) );
};

const selectColorOption = ( name ) => {
fireEvent.click( getColorOption( name ) );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another idea for a follow-up PR: refactor BorderControl's unit tests from fireEvent to testing-library/user-event

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #54155.

};

const getSliderInput = () => {
return screen.getByRole( 'slider', { name: 'Border width' } );
};
Expand Down Expand Up @@ -151,7 +159,7 @@ describe( 'BorderControl', () => {
await openPopover();

const customColorPicker = getButton( /Custom color picker/ );
const colorSwatchButtons = screen.getAllByRole( 'button', {
const colorSwatchButtons = screen.getAllByRole( 'option', {
name: /^Color:/,
} );
const styleLabel = screen.getByText( 'Style' );
Expand Down Expand Up @@ -324,7 +332,7 @@ describe( 'BorderControl', () => {
const props = createProps();
render( <BorderControl { ...props } /> );
await openPopover();
clickButton( 'Color: Green' );
selectColorOption( 'Color: Green' );

expect( props.onChange ).toHaveBeenNthCalledWith( 1, {
...defaultBorder,
Expand All @@ -336,7 +344,7 @@ describe( 'BorderControl', () => {
const props = createProps();
render( <BorderControl { ...props } /> );
await openPopover();
clickButton( 'Color: Blue' );
selectColorOption( 'Color: Blue' );

expect( props.onChange ).toHaveBeenNthCalledWith( 1, {
...defaultBorder,
Expand Down Expand Up @@ -384,7 +392,7 @@ describe( 'BorderControl', () => {
clearWidthInput();
rerender( <BorderControl { ...props } /> );
await openPopover();
clickButton( 'Color: Blue' );
selectColorOption( 'Color: Blue' );

expect( props.onChange ).toHaveBeenCalledWith( undefined );
} );
Expand All @@ -397,7 +405,7 @@ describe( 'BorderControl', () => {
clearWidthInput();
rerender( <BorderControl { ...props } /> );
await openPopover();
clickButton( 'Color: Blue' );
selectColorOption( 'Color: Blue' );

expect( props.onChange ).toHaveBeenNthCalledWith( 2, {
color: undefined,
Expand All @@ -410,7 +418,7 @@ describe( 'BorderControl', () => {
const props = createProps();
render( <BorderControl { ...props } /> );
await openPopover();
clickButton( 'Color: Green' );
selectColorOption( 'Color: Green' );
clickButton( 'Dotted' );
setWidthInput( '0' );

Expand All @@ -425,7 +433,7 @@ describe( 'BorderControl', () => {
const props = createProps();
const { rerender } = render( <BorderControl { ...props } /> );
await openPopover();
clickButton( 'Color: Green' );
selectColorOption( 'Color: Green' );
rerender( <BorderControl { ...props } /> );
clickButton( 'Dotted' );
rerender( <BorderControl { ...props } /> );
Expand All @@ -443,7 +451,7 @@ describe( 'BorderControl', () => {
const props = createProps( { value: undefined } );
const { rerender } = render( <BorderControl { ...props } /> );
await openPopover();
clickButton( 'Color: Yellow' );
selectColorOption( 'Color: Yellow' );

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

setWidthInput( '0' );
rerender( <BorderControl { ...props } /> );
clickButton( 'Color: Green' );
selectColorOption( 'Color: Green' );

expect( props.onChange ).toHaveBeenCalledWith( {
color: '#00a32a',
Expand Down
Loading