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

CustomSelectControl (v1 & v2): Fix errors in unit test setup #59038

Merged
merged 5 commits into from
Feb 15, 2024
Merged

CustomSelectControl (v1 & v2): Fix errors in unit test setup #59038

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f7cec48
CustomSelectControl: Fix test matrix
mirka Feb 14, 2024
85def83
Rename to prevent mistakes
mirka Feb 14, 2024
eb52aba
Fix prop passing errors
mirka Feb 14, 2024
c6e546c
CustomSelectControlV2: Fix legacy test matrix
mirka Feb 14, 2024
596452f
Merge branch 'trunk' into fix/custom-select-tests
mirka Feb 15, 2024
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
80 changes: 45 additions & 35 deletions packages/components/src/custom-select-control-v2/test/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* External dependencies
*/
import { render, screen } from '@testing-library/react';
import { click, press, sleep, type } from '@ariakit/test';
import { click, press, sleep, type, waitFor } from '@ariakit/test';

/**
* WordPress dependencies
Expand All @@ -12,7 +12,7 @@ import { useState } from '@wordpress/element';
/**
* Internal dependencies
*/
import { CustomSelect, CustomSelectItem } from '..';
import { CustomSelect as UncontrolledCustomSelect, CustomSelectItem } from '..';
Copy link
Member Author

Choose a reason for hiding this comment

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

Renamed to prevent accidental usage in a controlled/uncontrolled test matrix.

import type { CustomSelectProps, LegacyCustomSelectProps } from '../types';

const customClass = 'amber-skies';
Expand Down Expand Up @@ -51,12 +51,18 @@ const legacyProps = {

const LegacyControlledCustomSelect = ( {
options,
onChange,
...restProps
}: LegacyCustomSelectProps ) => {
const [ value, setValue ] = useState( options[ 0 ] );
return (
<CustomSelect
{ ...legacyProps }
onChange={ ( { selectedItem }: any ) => setValue( selectedItem ) }
<UncontrolledCustomSelect
{ ...restProps }
Copy link
Member Author

Choose a reason for hiding this comment

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

Ensure that the props from each test instance are actually passed through, instead of just spreading the scaffolded legacyProps.

options={ options }
onChange={ ( args: any ) => {
onChange?.( args );
Copy link
Member Author

Choose a reason for hiding this comment

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

Also pass through the onChange.

setValue( args.selectedItem );
} }
value={ options.find(
( option: any ) => option.key === value.key
) }
Expand All @@ -66,7 +72,7 @@ const LegacyControlledCustomSelect = ( {

describe( 'With Legacy Props', () => {
describe.each( [
[ 'Uncontrolled', CustomSelect ],
[ 'Uncontrolled', UncontrolledCustomSelect ],
[ 'Controlled', LegacyControlledCustomSelect ],
] )( '%s', ( ...modeAndComponent ) => {
const [ , Component ] = modeAndComponent;
Expand Down Expand Up @@ -100,7 +106,7 @@ describe( 'With Legacy Props', () => {
} );

it( 'Should keep current selection if dropdown is closed without changing selection', async () => {
render( <CustomSelect { ...legacyProps } /> );
Copy link
Member Author

Choose a reason for hiding this comment

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

These were all pointing to the uncontrolled version of the component, even though it was part of a uncontrolled/controlled matrix.

Copy link
Member

Choose a reason for hiding this comment

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

What a good find!

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

const currentSelectedItem = screen.getByRole( 'combobox', {
expanded: false,
Expand Down Expand Up @@ -128,7 +134,7 @@ describe( 'With Legacy Props', () => {
} );

it( 'Should apply class only to options that have a className defined', async () => {
render( <CustomSelect { ...legacyProps } /> );
render( <Component { ...legacyProps } /> );

await click(
screen.getByRole( 'combobox', {
Expand Down Expand Up @@ -165,7 +171,7 @@ describe( 'With Legacy Props', () => {
const customStyles =
'background-color: rgb(127, 255, 212); rotate: 13deg;';

render( <CustomSelect { ...legacyProps } /> );
render( <Component { ...legacyProps } /> );

await click(
screen.getByRole( 'combobox', {
Expand Down Expand Up @@ -198,9 +204,9 @@ describe( 'With Legacy Props', () => {
);
} );

it( 'does not show selected hint by default', () => {
it( 'does not show selected hint by default', async () => {
render(
<CustomSelect
<Component
{ ...legacyProps }
label="Custom select"
options={ [
Expand All @@ -212,14 +218,16 @@ describe( 'With Legacy Props', () => {
] }
/>
);
expect(
screen.getByRole( 'combobox', { name: 'Custom select' } )
).not.toHaveTextContent( 'Hint' );
await waitFor( () =>
Copy link
Member Author

Choose a reason for hiding this comment

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

Some async wait is necessary for the controlled version.

expect(
screen.getByRole( 'combobox', { name: 'Custom select' } )
).not.toHaveTextContent( 'Hint' )
);
} );

it( 'shows selected hint when __experimentalShowSelectedHint is set', async () => {
render(
<CustomSelect
<Component
{ ...legacyProps }
label="Custom select"
options={ [
Expand All @@ -233,16 +241,18 @@ describe( 'With Legacy Props', () => {
/>
);

expect(
screen.getByRole( 'combobox', {
expanded: false,
} )
).toHaveTextContent( /hint/i );
await waitFor( () =>
expect(
screen.getByRole( 'combobox', {
expanded: false,
} )
).toHaveTextContent( /hint/i )
);
} );

it( 'shows selected hint in list of options when added', async () => {
render(
<CustomSelect
<Component
{ ...legacyProps }
label="Custom select"
options={ [
Expand All @@ -269,7 +279,7 @@ describe( 'With Legacy Props', () => {
const mockOnChange = jest.fn();

render(
<CustomSelect { ...legacyProps } onChange={ mockOnChange } />
<Component { ...legacyProps } onChange={ mockOnChange } />
);

await click(
Expand Down Expand Up @@ -313,7 +323,7 @@ describe( 'With Legacy Props', () => {
);

render(
<CustomSelect { ...legacyProps } onChange={ mockOnChange } />
<Component { ...legacyProps } onChange={ mockOnChange } />
);

await sleep();
Expand All @@ -332,7 +342,7 @@ describe( 'With Legacy Props', () => {

describe( 'Keyboard behavior and accessibility', () => {
it( 'Should be able to change selection using keyboard', async () => {
render( <CustomSelect { ...legacyProps } /> );
render( <Component { ...legacyProps } /> );

const currentSelectedItem = screen.getByRole( 'combobox', {
expanded: false,
Expand All @@ -358,7 +368,7 @@ describe( 'With Legacy Props', () => {
} );

it( 'Should be able to type characters to select matching options', async () => {
render( <CustomSelect { ...legacyProps } /> );
render( <Component { ...legacyProps } /> );

const currentSelectedItem = screen.getByRole( 'combobox', {
expanded: false,
Expand All @@ -379,7 +389,7 @@ describe( 'With Legacy Props', () => {
} );

it( 'Can change selection with a focused input and closed dropdown if typed characters match an option', async () => {
render( <CustomSelect { ...legacyProps } /> );
render( <Component { ...legacyProps } /> );

const currentSelectedItem = screen.getByRole( 'combobox', {
expanded: false,
Expand All @@ -403,7 +413,7 @@ describe( 'With Legacy Props', () => {
} );

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

const currentSelectedItem = screen.getByRole( 'combobox', {
expanded: false,
Expand Down Expand Up @@ -460,29 +470,29 @@ describe( 'With Legacy Props', () => {
describe( 'static typing', () => {
<>
{ /* @ts-expect-error - when `options` prop is passed, `onChange` should have legacy signature */ }
<CustomSelect
<UncontrolledCustomSelect
label="foo"
options={ [] }
onChange={ ( _: string | string[] ) => undefined }
/>
<CustomSelect
<UncontrolledCustomSelect
label="foo"
options={ [] }
onChange={ ( _: { selectedItem: unknown } ) => undefined }
/>
<CustomSelect
<UncontrolledCustomSelect
label="foo"
onChange={ ( _: string | string[] ) => undefined }
>
foobar
</CustomSelect>
</UncontrolledCustomSelect>
{ /* @ts-expect-error - when `children` are passed, `onChange` should have new default signature */ }
<CustomSelect
<UncontrolledCustomSelect
label="foo"
onChange={ ( _: { selectedItem: unknown } ) => undefined }
>
foobar
</CustomSelect>
</UncontrolledCustomSelect>
</>;
} );

Expand All @@ -496,7 +506,7 @@ const defaultProps = {
const ControlledCustomSelect = ( props: CustomSelectProps ) => {
const [ value, setValue ] = useState< string | string[] >();
return (
<CustomSelect
<UncontrolledCustomSelect
{ ...props }
onChange={ ( nextValue: string | string[] ) => {
setValue( nextValue );
Expand All @@ -509,7 +519,7 @@ const ControlledCustomSelect = ( props: CustomSelectProps ) => {

describe( 'With Default Props', () => {
describe.each( [
[ 'Uncontrolled', CustomSelect ],
[ 'Uncontrolled', UncontrolledCustomSelect ],
[ 'Controlled', ControlledCustomSelect ],
] )( '%s', ( ...modeAndComponent ) => {
const [ , Component ] = modeAndComponent;
Expand Down
33 changes: 17 additions & 16 deletions packages/components/src/custom-select-control/test/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { useState } from '@wordpress/element';
/**
* Internal dependencies
*/
import CustomSelectControl from '..';
import UncontrolledCustomSelectControl from '..';

const customClass = 'amber-skies';

Expand Down Expand Up @@ -48,19 +48,20 @@ const props = {
],
};

const ControlledCustomSelectControl = ( { options } ) => {
const ControlledCustomSelectControl = ( { options, ...restProps } ) => {
const [ value, setValue ] = useState( options[ 0 ] );
return (
<CustomSelectControl
{ ...props }
<UncontrolledCustomSelectControl
{ ...restProps }
options={ options }
onChange={ ( { selectedItem } ) => setValue( selectedItem ) }
value={ options.find( ( option ) => option.key === value.key ) }
/>
);
};

describe.each( [
[ 'uncontrolled', CustomSelectControl ],
[ 'uncontrolled', UncontrolledCustomSelectControl ],
[ 'controlled', ControlledCustomSelectControl ],
] )( 'CustomSelectControl %s', ( ...modeAndComponent ) => {
const [ , Component ] = modeAndComponent;
Expand Down Expand Up @@ -98,7 +99,7 @@ describe.each( [
it( 'Should keep current selection if dropdown is closed without changing selection', async () => {
const user = userEvent.setup();

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

const currentSelectedItem = screen.getByRole( 'button', {
expanded: false,
Expand Down Expand Up @@ -127,7 +128,7 @@ describe.each( [
it( 'Should apply class only to options that have a className defined', async () => {
const user = userEvent.setup();

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

await user.click(
screen.getByRole( 'button', {
Expand Down Expand Up @@ -165,7 +166,7 @@ describe.each( [
const customStyles =
'background-color: rgb(127, 255, 212); rotate: 13deg;';

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

await user.click(
screen.getByRole( 'button', {
Expand Down Expand Up @@ -200,7 +201,7 @@ describe.each( [

it( 'does not show selected hint by default', () => {
render(
<CustomSelectControl
<Component
{ ...props }
label="Custom select"
options={ [
Expand All @@ -219,7 +220,7 @@ describe.each( [

it( 'shows selected hint when __experimentalShowSelectedHint is set', () => {
render(
<CustomSelectControl
<Component
{ ...props }
label="Custom select"
options={ [
Expand Down Expand Up @@ -248,7 +249,7 @@ describe.each( [
role="none"
onKeyDown={ onKeyDown }
>
<CustomSelectControl { ...props } />
<Component { ...props } />
</div>
);
const currentSelectedItem = screen.getByRole( 'button', {
Expand All @@ -267,7 +268,7 @@ describe.each( [
it( 'Should be able to change selection using keyboard', async () => {
const user = userEvent.setup();

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

const currentSelectedItem = screen.getByRole( 'button', {
expanded: false,
Expand All @@ -292,7 +293,7 @@ describe.each( [
it( 'Should be able to type characters to select matching options', async () => {
const user = userEvent.setup();

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

const currentSelectedItem = screen.getByRole( 'button', {
expanded: false,
Expand All @@ -314,7 +315,7 @@ describe.each( [
it( 'Can change selection with a focused input and closed dropdown if typed characters match an option', async () => {
const user = userEvent.setup();

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

const currentSelectedItem = screen.getByRole( 'button', {
expanded: false,
Expand All @@ -340,7 +341,7 @@ describe.each( [
it( 'Should have correct aria-selected value for selections', async () => {
const user = userEvent.setup();

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

const currentSelectedItem = screen.getByRole( 'button', {
expanded: false,
Expand Down Expand Up @@ -397,7 +398,7 @@ describe.each( [
const onBlurMock = jest.fn();

render(
<CustomSelectControl
<Component
{ ...props }
onFocus={ onFocusMock }
onBlur={ onBlurMock }
Expand Down
Loading