Skip to content

Commit

Permalink
CustomSelectControl V2: keep legacy arrow down behavior only for lega…
Browse files Browse the repository at this point in the history 
…cy wrapper (#62919)

* Use isLelacy flag to enable/disable showOnKeyDown behavior

* Add unit tests

* CHANGELOG

* Unit tests: Add `sleep()` before pressing `Tab` key

* Use label when checking that the listbox hasn't opened

* Use label variable instead of hardcoded string

* Use `Ariakit.Select` prop types instead of `CustomSelectButtonInternalProps` + `<button />`
  • Loading branch information
@ciampo
ciampo authored Jul 4, 2024
1 parent 78fc70e commit 307a0f7
Show file tree
Hide file tree
Showing 5 changed files with 103 additions and 26 deletions.
1 change: 1 addition & 0 deletions packages/components/CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
- `CustomSelectControlV2`: fix trigger text alignment in RTL languages ([#62869](https://github.com/WordPress/gutenberg/pull/62869)).
- `CustomSelectControlV2`: allow wrapping item hint to new line ([#62848](https://github.com/WordPress/gutenberg/pull/62848)).
- `CustomSelectControlV2`: fix select popover content overflow. ([#62844](https://github.com/WordPress/gutenberg/pull/62844))
- `CustomSelectControlV2`: keep legacy arrow down behavior only for legacy wrapper. ([#62919](https://github.com/WordPress/gutenberg/pull/62919))
- Extract `TimeInput` component from `TimePicker` ([#60613](https://github.com/WordPress/gutenberg/pull/60613)).
- `TimeInput`: Add `label` prop ([#63106](https://github.com/WordPress/gutenberg/pull/63106)).

Expand Down
21 changes: 12 additions & 9 deletions packages/components/src/custom-select-control-v2/custom-select.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
/**
* External dependencies
*/
// eslint-disable-next-line no-restricted-imports
import type * as Ariakit from '@ariakit/react';

/**
* WordPress dependencies
*/
Expand All @@ -17,7 +23,6 @@ import type {
_CustomSelectInternalProps,
_CustomSelectProps,
} from './types';
import type { WordPressComponentProps } from '../context';
import InputBase from '../input-control/input-base';
import SelectControlChevronDown from '../select-control/chevron-down';

Expand Down Expand Up @@ -51,11 +56,10 @@ const CustomSelectButton = ( {
store,
...restProps
}: Omit<
WordPressComponentProps<
CustomSelectButtonProps & CustomSelectButtonSize & CustomSelectStore,
'button',
false
>,
React.ComponentProps< typeof Ariakit.Select > &
CustomSelectButtonProps &
CustomSelectButtonSize &
CustomSelectStore,
'onChange'
> ) => {
const { value: currentValue } = store.useState();
Expand All @@ -71,9 +75,6 @@ const CustomSelectButton = ( {
size={ size }
hasCustomRenderProp={ !! renderSelectedValue }
store={ store }
// to match legacy behavior where using arrow keys
// move selection rather than open the popover
showOnKeyDown={ false }
>
{ computedRenderSelectedValue( currentValue ) }
</Styled.Select>
Expand Down Expand Up @@ -128,6 +129,8 @@ function _CustomSelect(
{ ...restProps }
size={ size }
store={ store }
// Match legacy behavior (move selection rather than open the popover)
showOnKeyDown={ ! isLegacy }
/>
<Styled.SelectPopover
gutter={ 12 }
Expand Down
39 changes: 33 additions & 6 deletions packages/components/src/custom-select-control-v2/legacy-component/test/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -204,14 +204,14 @@ describe.each( [
await press.Enter();
expect(
screen.getByRole( 'listbox', {
name: 'label!',
name: legacyProps.label,
} )
).toBeVisible();

await press.Escape();
expect(
screen.queryByRole( 'listbox', {
name: 'label!',
name: legacyProps.label,
} )
).not.toBeInTheDocument();

Expand Down Expand Up @@ -472,7 +472,7 @@ describe.each( [
await click( currentSelectedItem );

const customSelect = screen.getByRole( 'listbox', {
name: 'label!',
name: legacyProps.label,
} );
expect( customSelect ).toHaveFocus();
await press.Enter();
Expand All @@ -494,7 +494,7 @@ describe.each( [
await press.Enter();
expect(
screen.getByRole( 'listbox', {
name: 'label!',
name: legacyProps.label,
} )
).toHaveFocus();

Expand All @@ -518,7 +518,7 @@ describe.each( [
await press.Enter();
expect(
screen.getByRole( 'listbox', {
name: 'label!',
name: legacyProps.label,
} )
).toHaveFocus();

Expand Down Expand Up @@ -546,7 +546,7 @@ describe.each( [

expect(
screen.queryByRole( 'listbox', {
name: 'label!',
name: legacyProps.label,
hidden: true,
} )
).not.toBeInTheDocument();
Expand All @@ -557,6 +557,33 @@ describe.each( [
expect( currentSelectedItem ).toHaveTextContent( 'amber' );
} );

it( 'Can change selection with a focused input and closed dropdown while pressing arrow keys', async () => {
render( <Component { ...legacyProps } /> );

const currentSelectedItem = screen.getByRole( 'combobox', {
expanded: false,
} );

await sleep();
await press.Tab();
expect( currentSelectedItem ).toHaveFocus();
expect( currentSelectedItem ).toHaveTextContent(
legacyProps.options[ 0 ].name
);

await press.ArrowDown();
await press.ArrowDown();
expect(
screen.queryByRole( 'listbox', {
name: legacyProps.label,
} )
).not.toBeInTheDocument();

expect( currentSelectedItem ).toHaveTextContent(
legacyProps.options[ 2 ].name
);
} );

it( 'Should have correct aria-selected value for selections', async () => {
render( <Component { ...legacyProps } /> );

Expand Down
30 changes: 25 additions & 5 deletions packages/components/src/custom-select-control-v2/test/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,14 +105,14 @@ describe.each( [
await press.Enter();
expect(
screen.getByRole( 'listbox', {
name: 'label!',
name: defaultProps.label,
} )
).toBeVisible();

await press.Escape();
expect(
screen.queryByRole( 'listbox', {
name: 'label!',
name: defaultProps.label,
} )
).not.toBeInTheDocument();

Expand All @@ -134,7 +134,7 @@ describe.each( [
await press.Enter();
expect(
screen.getByRole( 'listbox', {
name: 'label!',
name: defaultProps.label,
} )
).toHaveFocus();

Expand All @@ -156,7 +156,7 @@ describe.each( [
await press.Enter();
expect(
screen.getByRole( 'listbox', {
name: 'label!',
name: defaultProps.label,
} )
).toHaveFocus();

Expand All @@ -182,7 +182,7 @@ describe.each( [

expect(
screen.queryByRole( 'listbox', {
name: 'label!',
name: defaultProps.label,
hidden: true,
} )
).not.toBeInTheDocument();
Expand Down Expand Up @@ -416,4 +416,24 @@ describe.each( [
screen.getByRole( 'option', { name: 'july-9' } )
).toBeVisible();
} );

it( 'Should open the select popover when focussing the trigger button and pressing arrow down', async () => {
render( <Component { ...defaultProps } /> );

const currentSelectedItem = screen.getByRole( 'combobox', {
expanded: false,
} );

await sleep();
await press.Tab();
expect( currentSelectedItem ).toHaveFocus();
expect( currentSelectedItem ).toHaveTextContent( items[ 0 ].value );

await press.ArrowDown();
expect(
screen.getByRole( 'listbox', {
name: defaultProps.label,
} )
).toBeVisible();
} );
} );
38 changes: 32 additions & 6 deletions packages/components/src/custom-select-control/test/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,14 +190,14 @@ describe.each( [
await user.keyboard( '{enter}' );
expect(
screen.getByRole( 'listbox', {
name: 'label!',
name: props.label,
} )
).toBeVisible();

await user.keyboard( '{escape}' );
expect(
screen.queryByRole( 'listbox', {
name: 'label!',
name: props.label,
} )
).not.toBeInTheDocument();

Expand Down Expand Up @@ -460,7 +460,7 @@ describe.each( [
await user.click( currentSelectedItem );

const customSelect = screen.getByRole( 'listbox', {
name: 'label!',
name: props.label,
} );
await user.type( customSelect, '{enter}' );

Expand All @@ -482,7 +482,7 @@ describe.each( [
await user.keyboard( '{enter}' );
expect(
screen.getByRole( 'listbox', {
name: 'label!',
name: props.label,
} )
).toHaveFocus();

Expand All @@ -507,7 +507,7 @@ describe.each( [
await user.keyboard( '{enter}' );
expect(
screen.getByRole( 'listbox', {
name: 'label!',
name: props.label,
} )
).toHaveFocus();

Expand All @@ -533,7 +533,7 @@ describe.each( [

expect(
screen.queryByRole( 'listbox', {
name: 'label!',
name: props.label,
hidden: true,
} )
).not.toBeInTheDocument();
Expand All @@ -542,6 +542,32 @@ describe.each( [
expect( currentSelectedItem ).toHaveTextContent( 'aquamarine' );
} );

it( 'Can change selection with a focused input and closed dropdown while pressing arrow keys', async () => {
const user = userEvent.setup();

render( <Component { ...props } /> );

const currentSelectedItem = screen.getByRole( 'button', {
expanded: false,
} );

await user.tab();
expect( currentSelectedItem ).toHaveFocus();
expect( currentSelectedItem ).toHaveTextContent(
props.options[ 0 ].name
);

await user.keyboard( '{arrowdown}' );
await user.keyboard( '{arrowdown}' );
expect(
screen.queryByRole( 'listbox', { name: props.label } )
).not.toBeInTheDocument();

expect( currentSelectedItem ).toHaveTextContent(
props.options[ 2 ].name
);
} );

it( 'Should have correct aria-selected value for selections', async () => {
const user = userEvent.setup();

Expand Down

1 comment on commit 307a0f7

@github-actions
Copy link

Choose a reason for hiding this comment

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

Flaky tests detected in 307a0f7.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/9792817601
📝 Reported issues:

Please sign in to comment.