diff --git a/packages/components/src/custom-select-control-v2/legacy-component/test/index.tsx b/packages/components/src/custom-select-control-v2/legacy-component/test/index.tsx index aa933b0cf532c1..3b9b9c29180c21 100644 --- a/packages/components/src/custom-select-control-v2/legacy-component/test/index.tsx +++ b/packages/components/src/custom-select-control-v2/legacy-component/test/index.tsx @@ -14,8 +14,8 @@ import { useState } from '@wordpress/element'; */ import UncontrolledCustomSelectControl from '..'; -const className = 'amber-skies'; -const style = { +const customClassName = 'amber-skies'; +const customStyles = { backgroundColor: 'rgb(127, 255, 212)', rotate: '13deg', }; @@ -30,7 +30,7 @@ const legacyProps = { { key: 'flower2', name: 'crimson clover', - className, + className: customClassName, }, { key: 'flower3', @@ -39,18 +39,18 @@ const legacyProps = { { key: 'color1', name: 'amber', - className, + className: customClassName, }, { key: 'color2', name: 'aquamarine', - style, + style: customStyles, }, { - key: 'aquarela-key', - name: 'aquarela', - className, - style, + key: 'color3', + name: 'tomato (with custom props)', + className: customClassName, + style: customStyles, // try passing a valid HTML attribute 'aria-label': 'test label', // try adding a custom prop @@ -61,18 +61,21 @@ const legacyProps = { const ControlledCustomSelectControl = ( { options, - onChange, + onChange: onChangeProp, ...restProps }: React.ComponentProps< typeof UncontrolledCustomSelectControl > ) => { const [ value, setValue ] = useState( options[ 0 ] ); + + const onChange: typeof onChangeProp = ( changeObject ) => { + onChangeProp?.( changeObject ); + setValue( changeObject.selectedItem ); + }; + return ( { - onChange?.( args ); - setValue( args.selectedItem ); - } } + onChange={ onChange } value={ options.find( ( option: any ) => option.key === value.key ) } @@ -101,6 +104,8 @@ describe.each( [ } ) ); + // TODO: also check `value` instead of just content, + // here and everywhere else in the file expect( currentSelectedItem ).toHaveTextContent( 'crimson clover' ); await click( currentSelectedItem ); @@ -159,7 +164,7 @@ describe.each( [ // assert against filtered array itemsWithClass.map( ( { name } ) => expect( screen.getByRole( 'option', { name } ) ).toHaveClass( - className + customClassName ) ); @@ -171,15 +176,12 @@ describe.each( [ // assert against filtered array itemsWithoutClass.map( ( { name } ) => expect( screen.getByRole( 'option', { name } ) ).not.toHaveClass( - className + customClassName ) ); } ); it( 'Should apply styles only to options that have styles defined', async () => { - const customStyles = - 'background-color: rgb(127, 255, 212); rotate: 13deg;'; - render( ); await click( @@ -255,7 +257,7 @@ describe.each( [ screen.getByRole( 'combobox', { expanded: false, } ) - ).toHaveTextContent( /hint/i ) + ).toHaveTextContent( 'Hint' ) ); } ); @@ -292,6 +294,8 @@ describe.each( [ } ) ); + // NOTE: legacy CustomSelectControl doesn't fire onChange + // at this point in time. expect( mockOnChange ).toHaveBeenNthCalledWith( 1, expect.objectContaining( { @@ -322,9 +326,7 @@ describe.each( [ } ); it( 'Should return selectedItem object when specified onChange', async () => { - const mockOnChange = jest.fn( - ( { selectedItem } ) => selectedItem.key - ); + const mockOnChange = jest.fn(); render( ); @@ -336,13 +338,103 @@ describe.each( [ } ) ).toHaveFocus(); + // NOTE: legacy CustomSelectControl doesn't fire onChange + // at this point in time. + expect( mockOnChange ).toHaveBeenNthCalledWith( + 1, + expect.objectContaining( { + selectedItem: expect.objectContaining( { + key: 'flower1', + name: 'violets', + } ), + } ) + ); + await type( 'p' ); await press.Enter(); - expect( mockOnChange ).toHaveReturnedWith( 'flower1' ); + expect( mockOnChange ).toHaveBeenNthCalledWith( + 2, + expect.objectContaining( { + selectedItem: expect.objectContaining( { + key: 'flower3', + name: 'poppy', + } ), + } ) + ); + } ); + + it( 'Should accept and pass arbitrary properties to the selectedItem object in the onChange callback, but without applying them to the DOM elements apart from style and classname', async () => { + const onChangeMock = jest.fn(); + + render( ); + + const currentSelectedItem = screen.getByRole( 'combobox', { + expanded: false, + } ); + + await click( currentSelectedItem ); + + const optionWithCustomAttributes = screen.getByRole( 'option', { + name: 'tomato (with custom props)', + } ); + + // Assert that the option element does not have the custom attributes + expect( optionWithCustomAttributes ).not.toHaveAttribute( + 'customPropFoo' + ); + expect( optionWithCustomAttributes ).not.toHaveAttribute( + 'aria-label' + ); + + await click( optionWithCustomAttributes ); + + // NOTE: legacy CustomSelectControl doesn't fire onChange + // on first render, and so at this point in time `onChangeMock` + // would be called only once. + expect( onChangeMock ).toHaveBeenCalledTimes( 2 ); + expect( onChangeMock ).toHaveBeenCalledWith( + expect.objectContaining( { + selectedItem: expect.objectContaining( { + key: 'color3', + name: 'tomato (with custom props)', + className: customClassName, + style: customStyles, + 'aria-label': 'test label', + customPropFoo: 'foo', + } ), + } ) + ); } ); describe( 'Keyboard behavior and accessibility', () => { + // skip reason: legacy v2 doesn't currently implement this behavior + it.skip( 'Captures the keypress event and does not let it propagate', async () => { + const onKeyDown = jest.fn(); + + render( +
+ +
+ ); + const currentSelectedItem = screen.getByRole( 'combobox', { + expanded: false, + } ); + await click( currentSelectedItem ); + + const customSelect = screen.getByRole( 'listbox', { + name: 'label!', + } ); + expect( customSelect ).toHaveFocus(); + await press.Enter(); + + expect( onKeyDown ).toHaveBeenCalledTimes( 0 ); + } ); + it( 'Should be able to change selection using keyboard', async () => { render( ); @@ -467,43 +559,34 @@ describe.each( [ } ) ).toBeVisible(); } ); - } ); - - // V1 styles items via a `style` or `className` metadata property in the option item object. Some consumers still expect it, e.g: - // - https://github.com/WordPress/gutenberg/blob/trunk/packages/block-editor/src/components/font-appearance-control/index.js#L216 - // Besides that, the `option` prop is documented as havin the type: - // - `Array<{ key: String, name: String, style: ?{}, className: ?String, ...rest }>` - // Notice the `...test` there. We should keep supporting the arbitrary props like this. - it( 'Should return style and custom metadata as part of the selected option from onChange', async () => { - const mockOnChange = jest.fn(); - render( ); + it( 'Should call custom event handlers', async () => { + const onFocusMock = jest.fn(); + const onBlurMock = jest.fn(); + + render( + <> + + + + ); - await click( - screen.getByRole( 'combobox', { + const currentSelectedItem = screen.getByRole( 'combobox', { expanded: false, - } ) - ); - - const optionElement = screen.getByRole( 'option', { - name: 'aquarela', - } ); + } ); - // Assert that the option element does not have the custom attributes - expect( optionElement ).not.toHaveAttribute( 'customPropFoo' ); - expect( optionElement ).not.toHaveAttribute( 'aria-label' ); + await press.Tab(); - await click( optionElement ); + expect( currentSelectedItem ).toHaveFocus(); + expect( onFocusMock ).toHaveBeenCalledTimes( 1 ); - expect( mockOnChange ).toHaveBeenCalledWith( - expect.objectContaining( { - selectedItem: expect.objectContaining( { - className, - style, - customPropFoo: 'foo', - 'aria-label': 'test label', - } ), - } ) - ); + await press.Tab(); + expect( currentSelectedItem ).not.toHaveFocus(); + expect( onBlurMock ).toHaveBeenCalledTimes( 1 ); + } ); } ); } ); diff --git a/packages/components/src/custom-select-control/test/index.js b/packages/components/src/custom-select-control/test/index.js index ec30aa5bca42e9..550607a024906a 100644 --- a/packages/components/src/custom-select-control/test/index.js +++ b/packages/components/src/custom-select-control/test/index.js @@ -14,7 +14,11 @@ import { useState } from '@wordpress/element'; */ import UncontrolledCustomSelectControl from '..'; -const customClass = 'amber-skies'; +const customClassName = 'amber-skies'; +const customStyles = { + backgroundColor: 'rgb(127, 255, 212)', + rotate: '13deg', +}; const props = { label: 'label!', @@ -26,7 +30,7 @@ const props = { { key: 'flower2', name: 'crimson clover', - className: customClass, + className: customClassName, }, { key: 'flower3', @@ -35,15 +39,22 @@ const props = { { key: 'color1', name: 'amber', - className: customClass, + className: customClassName, }, { key: 'color2', name: 'aquamarine', - style: { - backgroundColor: 'rgb(127, 255, 212)', - rotate: '13deg', - }, + style: customStyles, + }, + { + key: 'color3', + name: 'tomato (with custom props)', + className: customClassName, + style: customStyles, + // try passing a valid HTML attribute + 'aria-label': 'test label', + // try adding a custom prop + customPropFoo: 'foo', }, ], }; @@ -54,10 +65,12 @@ const ControlledCustomSelectControl = ( { ...restProps } ) => { const [ value, setValue ] = useState( options[ 0 ] ); + const onChange = ( changeObject ) => { onChangeProp?.( changeObject ); setValue( changeObject.selectedItem ); }; + return ( expect( screen.getByRole( 'option', { name } ) ).toHaveClass( - customClass + customClassName ) ); @@ -164,15 +177,13 @@ describe.each( [ // assert against filtered array itemsWithoutClass.map( ( { name } ) => expect( screen.getByRole( 'option', { name } ) ).not.toHaveClass( - customClass + customClassName ) ); } ); it( 'Should apply styles only to options that have styles defined', async () => { const user = userEvent.setup(); - const customStyles = - 'background-color: rgb(127, 255, 212); rotate: 13deg;'; render( ); @@ -246,53 +257,141 @@ describe.each( [ ).toHaveTextContent( 'Hint' ); } ); - it( 'Should accept and pass arbitrary properties to the selectedItem object in the onChange callback', async () => { + it( 'shows selected hint in list of options when added, regardless of __experimentalShowSelectedHint prop', async () => { const user = userEvent.setup(); - const onChangeMock = jest.fn(); render( ); + await user.click( + screen.getByRole( 'button', { name: 'Custom select' } ) + ); + + expect( screen.getByRole( 'option', { name: /hint/i } ) ).toBeVisible(); + } ); + + it( 'Should return object onChange', async () => { + const user = userEvent.setup(); + const mockOnChange = jest.fn(); + + render( + + ); + + await user.click( + screen.getByRole( 'button', { + expanded: false, + } ) + ); + + // DIFFERENCE WITH V2: NOT CALLED + // expect( mockOnChange ).toHaveBeenNthCalledWith( + // 1, + // expect.objectContaining( { + // inputValue: '', + // isOpen: false, + // selectedItem: { key: 'flower1', name: 'violets' }, + // type: '', + // } ) + // ); + + await user.click( + screen.getByRole( 'option', { + name: 'aquamarine', + } ) + ); + + expect( mockOnChange ).toHaveBeenNthCalledWith( + 1, + expect.objectContaining( { + inputValue: '', + isOpen: false, + selectedItem: expect.objectContaining( { + name: 'aquamarine', + } ), + type: '__item_click__', + } ) + ); + } ); + + it( 'Should return selectedItem object when specified onChange', async () => { + const user = userEvent.setup(); + const mockOnChange = jest.fn(); + + render( ); + + await user.tab(); + expect( + screen.getByRole( 'button', { + expanded: false, + } ) + ).toHaveFocus(); + + await user.keyboard( 'p' ); + await user.keyboard( '{enter}' ); + + expect( mockOnChange ).toHaveBeenNthCalledWith( + 1, + expect.objectContaining( { + selectedItem: expect.objectContaining( { + key: 'flower3', + name: 'poppy', + } ), + } ) + ); + } ); + + it( 'Should accept and pass arbitrary properties to the selectedItem object in the onChange callback, but without applying them to the DOM elements apart from style and classname', async () => { + const user = userEvent.setup(); + const onChangeMock = jest.fn(); + + render( ); + const currentSelectedItem = screen.getByRole( 'button', { expanded: false, } ); await user.click( currentSelectedItem ); - await user.click( - screen.getByRole( 'option', { name: 'Custom Option' } ) + + const optionWithCustomAttributes = screen.getByRole( 'option', { + name: 'tomato (with custom props)', + } ); + + // Assert that the option element does not have the custom attributes + expect( optionWithCustomAttributes ).not.toHaveAttribute( + 'customPropFoo' + ); + expect( optionWithCustomAttributes ).not.toHaveAttribute( + 'aria-label' ); + await user.click( optionWithCustomAttributes ); + expect( onChangeMock ).toHaveBeenCalledTimes( 1 ); expect( onChangeMock ).toHaveBeenCalledWith( expect.objectContaining( { selectedItem: expect.objectContaining( { - key: 'custom', - name: 'Custom Option', - className: 'custom-class-name', - customProp1: 'value1', - customProp2: 42, - style: { - backgroundColor: 'rgb(127, 255, 212)', - rotate: '13deg', - }, + key: 'color3', + name: 'tomato (with custom props)', + className: customClassName, + style: customStyles, + 'aria-label': 'test label', + customPropFoo: 'foo', } ), } ) ); @@ -458,11 +557,14 @@ describe.each( [ const onBlurMock = jest.fn(); render( - + <> + + + ); const currentSelectedItem = screen.getByRole( 'button', {