Skip to content

Commit 307a0f7

Browse files
ciampociampo
authored
CustomSelectControl V2: keep legacy arrow down behavior only for legacy 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 />`
1 parent 78fc70e commit 307a0f7

File tree

5 files changed

+103
-26
lines changed

5 files changed

+103
-26
lines changed

‎packages/components/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
- `CustomSelectControlV2`: fix trigger text alignment in RTL languages ([#62869](https://github.com/WordPress/gutenberg/pull/62869)).
2525
- `CustomSelectControlV2`: allow wrapping item hint to new line ([#62848](https://github.com/WordPress/gutenberg/pull/62848)).
2626
- `CustomSelectControlV2`: fix select popover content overflow. ([#62844](https://github.com/WordPress/gutenberg/pull/62844))
27+
- `CustomSelectControlV2`: keep legacy arrow down behavior only for legacy wrapper. ([#62919](https://github.com/WordPress/gutenberg/pull/62919))
2728
- Extract `TimeInput` component from `TimePicker` ([#60613](https://github.com/WordPress/gutenberg/pull/60613)).
2829
- `TimeInput`: Add `label` prop ([#63106](https://github.com/WordPress/gutenberg/pull/63106)).
2930

‎packages/components/src/custom-select-control-v2/custom-select.tsx‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,9 @@
1+
/**
2+
* External dependencies
3+
*/
4+
// eslint-disable-next-line no-restricted-imports
5+
import type * as Ariakit from '@ariakit/react';
6+
17
/**
28
* WordPress dependencies
39
*/
@@ -17,7 +23,6 @@ import type {
1723
_CustomSelectInternalProps,
1824
_CustomSelectProps,
1925
} from './types';
20-
import type { WordPressComponentProps } from '../context';
2126
import InputBase from '../input-control/input-base';
2227
import SelectControlChevronDown from '../select-control/chevron-down';
2328

@@ -51,11 +56,10 @@ const CustomSelectButton = ( {
5156
store,
5257
...restProps
5358
}: Omit<
54-
WordPressComponentProps<
55-
CustomSelectButtonProps & CustomSelectButtonSize & CustomSelectStore,
56-
'button',
57-
false
58-
>,
59+
React.ComponentProps< typeof Ariakit.Select > &
60+
CustomSelectButtonProps &
61+
CustomSelectButtonSize &
62+
CustomSelectStore,
5963
'onChange'
6064
> ) => {
6165
const { value: currentValue } = store.useState();
@@ -71,9 +75,6 @@ const CustomSelectButton = ( {
7175
size={ size }
7276
hasCustomRenderProp={ !! renderSelectedValue }
7377
store={ store }
74-
// to match legacy behavior where using arrow keys
75-
// move selection rather than open the popover
76-
showOnKeyDown={ false }
7778
>
7879
{ computedRenderSelectedValue( currentValue ) }
7980
</Styled.Select>
@@ -128,6 +129,8 @@ function _CustomSelect(
128129
{ ...restProps }
129130
size={ size }
130131
store={ store }
132+
// Match legacy behavior (move selection rather than open the popover)
133+
showOnKeyDown={ ! isLegacy }
131134
/>
132135
<Styled.SelectPopover
133136
gutter={ 12 }

‎packages/components/src/custom-select-control-v2/legacy-component/test/index.tsx‎

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -204,14 +204,14 @@ describe.each( [
204204
await press.Enter();
205205
expect(
206206
screen.getByRole( 'listbox', {
207-
name: 'label!',
207+
name: legacyProps.label,
208208
} )
209209
).toBeVisible();
210210

211211
await press.Escape();
212212
expect(
213213
screen.queryByRole( 'listbox', {
214-
name: 'label!',
214+
name: legacyProps.label,
215215
} )
216216
).not.toBeInTheDocument();
217217

@@ -472,7 +472,7 @@ describe.each( [
472472
await click( currentSelectedItem );
473473

474474
const customSelect = screen.getByRole( 'listbox', {
475-
name: 'label!',
475+
name: legacyProps.label,
476476
} );
477477
expect( customSelect ).toHaveFocus();
478478
await press.Enter();
@@ -494,7 +494,7 @@ describe.each( [
494494
await press.Enter();
495495
expect(
496496
screen.getByRole( 'listbox', {
497-
name: 'label!',
497+
name: legacyProps.label,
498498
} )
499499
).toHaveFocus();
500500

@@ -518,7 +518,7 @@ describe.each( [
518518
await press.Enter();
519519
expect(
520520
screen.getByRole( 'listbox', {
521-
name: 'label!',
521+
name: legacyProps.label,
522522
} )
523523
).toHaveFocus();
524524

@@ -546,7 +546,7 @@ describe.each( [
546546

547547
expect(
548548
screen.queryByRole( 'listbox', {
549-
name: 'label!',
549+
name: legacyProps.label,
550550
hidden: true,
551551
} )
552552
).not.toBeInTheDocument();
@@ -557,6 +557,33 @@ describe.each( [
557557
expect( currentSelectedItem ).toHaveTextContent( 'amber' );
558558
} );
559559

560+
it( 'Can change selection with a focused input and closed dropdown while pressing arrow keys', async () => {
561+
render( <Component { ...legacyProps } /> );
562+
563+
const currentSelectedItem = screen.getByRole( 'combobox', {
564+
expanded: false,
565+
} );
566+
567+
await sleep();
568+
await press.Tab();
569+
expect( currentSelectedItem ).toHaveFocus();
570+
expect( currentSelectedItem ).toHaveTextContent(
571+
legacyProps.options[ 0 ].name
572+
);
573+
574+
await press.ArrowDown();
575+
await press.ArrowDown();
576+
expect(
577+
screen.queryByRole( 'listbox', {
578+
name: legacyProps.label,
579+
} )
580+
).not.toBeInTheDocument();
581+
582+
expect( currentSelectedItem ).toHaveTextContent(
583+
legacyProps.options[ 2 ].name
584+
);
585+
} );
586+
560587
it( 'Should have correct aria-selected value for selections', async () => {
561588
render( <Component { ...legacyProps } /> );
562589

‎packages/components/src/custom-select-control-v2/test/index.tsx‎

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -105,14 +105,14 @@ describe.each( [
105105
await press.Enter();
106106
expect(
107107
screen.getByRole( 'listbox', {
108-
name: 'label!',
108+
name: defaultProps.label,
109109
} )
110110
).toBeVisible();
111111

112112
await press.Escape();
113113
expect(
114114
screen.queryByRole( 'listbox', {
115-
name: 'label!',
115+
name: defaultProps.label,
116116
} )
117117
).not.toBeInTheDocument();
118118

@@ -134,7 +134,7 @@ describe.each( [
134134
await press.Enter();
135135
expect(
136136
screen.getByRole( 'listbox', {
137-
name: 'label!',
137+
name: defaultProps.label,
138138
} )
139139
).toHaveFocus();
140140

@@ -156,7 +156,7 @@ describe.each( [
156156
await press.Enter();
157157
expect(
158158
screen.getByRole( 'listbox', {
159-
name: 'label!',
159+
name: defaultProps.label,
160160
} )
161161
).toHaveFocus();
162162

@@ -182,7 +182,7 @@ describe.each( [
182182

183183
expect(
184184
screen.queryByRole( 'listbox', {
185-
name: 'label!',
185+
name: defaultProps.label,
186186
hidden: true,
187187
} )
188188
).not.toBeInTheDocument();
@@ -416,4 +416,24 @@ describe.each( [
416416
screen.getByRole( 'option', { name: 'july-9' } )
417417
).toBeVisible();
418418
} );
419+
420+
it( 'Should open the select popover when focussing the trigger button and pressing arrow down', async () => {
421+
render( <Component { ...defaultProps } /> );
422+
423+
const currentSelectedItem = screen.getByRole( 'combobox', {
424+
expanded: false,
425+
} );
426+
427+
await sleep();
428+
await press.Tab();
429+
expect( currentSelectedItem ).toHaveFocus();
430+
expect( currentSelectedItem ).toHaveTextContent( items[ 0 ].value );
431+
432+
await press.ArrowDown();
433+
expect(
434+
screen.getByRole( 'listbox', {
435+
name: defaultProps.label,
436+
} )
437+
).toBeVisible();
438+
} );
419439
} );

‎packages/components/src/custom-select-control/test/index.js‎

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -190,14 +190,14 @@ describe.each( [
190190
await user.keyboard( '{enter}' );
191191
expect(
192192
screen.getByRole( 'listbox', {
193-
name: 'label!',
193+
name: props.label,
194194
} )
195195
).toBeVisible();
196196

197197
await user.keyboard( '{escape}' );
198198
expect(
199199
screen.queryByRole( 'listbox', {
200-
name: 'label!',
200+
name: props.label,
201201
} )
202202
).not.toBeInTheDocument();
203203

@@ -460,7 +460,7 @@ describe.each( [
460460
await user.click( currentSelectedItem );
461461

462462
const customSelect = screen.getByRole( 'listbox', {
463-
name: 'label!',
463+
name: props.label,
464464
} );
465465
await user.type( customSelect, '{enter}' );
466466

@@ -482,7 +482,7 @@ describe.each( [
482482
await user.keyboard( '{enter}' );
483483
expect(
484484
screen.getByRole( 'listbox', {
485-
name: 'label!',
485+
name: props.label,
486486
} )
487487
).toHaveFocus();
488488

@@ -507,7 +507,7 @@ describe.each( [
507507
await user.keyboard( '{enter}' );
508508
expect(
509509
screen.getByRole( 'listbox', {
510-
name: 'label!',
510+
name: props.label,
511511
} )
512512
).toHaveFocus();
513513

@@ -533,7 +533,7 @@ describe.each( [
533533

534534
expect(
535535
screen.queryByRole( 'listbox', {
536-
name: 'label!',
536+
name: props.label,
537537
hidden: true,
538538
} )
539539
).not.toBeInTheDocument();
@@ -542,6 +542,32 @@ describe.each( [
542542
expect( currentSelectedItem ).toHaveTextContent( 'aquamarine' );
543543
} );
544544

545+
it( 'Can change selection with a focused input and closed dropdown while pressing arrow keys', async () => {
546+
const user = userEvent.setup();
547+
548+
render( <Component { ...props } /> );
549+
550+
const currentSelectedItem = screen.getByRole( 'button', {
551+
expanded: false,
552+
} );
553+
554+
await user.tab();
555+
expect( currentSelectedItem ).toHaveFocus();
556+
expect( currentSelectedItem ).toHaveTextContent(
557+
props.options[ 0 ].name
558+
);
559+
560+
await user.keyboard( '{arrowdown}' );
561+
await user.keyboard( '{arrowdown}' );
562+
expect(
563+
screen.queryByRole( 'listbox', { name: props.label } )
564+
).not.toBeInTheDocument();
565+
566+
expect( currentSelectedItem ).toHaveTextContent(
567+
props.options[ 2 ].name
568+
);
569+
} );
570+
545571
it( 'Should have correct aria-selected value for selections', async () => {
546572
const user = userEvent.setup();
547573

0 commit comments

Comments
 (0)