diff --git a/packages/components/src/custom-select-control-v2/custom-select.tsx b/packages/components/src/custom-select-control-v2/custom-select.tsx index 9ae402d37f40d9..05d548d4958cfc 100644 --- a/packages/components/src/custom-select-control-v2/custom-select.tsx +++ b/packages/components/src/custom-select-control-v2/custom-select.tsx @@ -92,7 +92,7 @@ function _CustomSelect( } = props; return ( - <> + { hideLabelFromVision ? ( // TODO: Replace with BaseControl { label } ) : ( @@ -116,7 +116,7 @@ function _CustomSelect( - + ); } diff --git a/packages/components/src/custom-select-control-v2/legacy-component/index.tsx b/packages/components/src/custom-select-control-v2/legacy-component/index.tsx index 43f102e6ee0493..d0e7eddcfb4460 100644 --- a/packages/components/src/custom-select-control-v2/legacy-component/index.tsx +++ b/packages/components/src/custom-select-control-v2/legacy-component/index.tsx @@ -3,7 +3,10 @@ */ // eslint-disable-next-line no-restricted-imports import * as Ariakit from '@ariakit/react'; - +/** + * WordPress dependencies + */ +import { useEffect } from '@wordpress/element'; /** * Internal dependencies */ @@ -34,24 +37,33 @@ function CustomSelectControl( props: LegacyCustomSelectProps ) { // Executes the logic in a microtask after the popup is closed. // This is simply to ensure the isOpen state matches that in Downshift. await Promise.resolve(); + const state = store.getState(); + const option = options.find( ( item ) => item.name === nextValue ); + const changeObject = { - highlightedIndex: state.renderedItems.findIndex( + highlightedIndex: state.items.findIndex( ( item ) => item.value === nextValue ), inputValue: '', isOpen: state.open, - selectedItem: { - name: nextValue as string, - key: nextValue as string, - }, + selectedItem: option!, type: '', }; + onChange( changeObject ); }, } ); + useEffect( () => { + // This is a workaround for selecting the right item upon mount + if ( value ) { + store.setValue( value.name ); + } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [] ); + const children = options.map( ( { name, key, __experimentalHint, ...rest } ) => { const withHint = ( @@ -121,4 +133,15 @@ function CustomSelectControl( props: LegacyCustomSelectProps ) { ); } -export default CustomSelectControl; +export function ClassicCustomSelectControlV2Adapter( + props: LegacyCustomSelectProps +) { + return ( + + ); +} + +export default ClassicCustomSelectControlV2Adapter; 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 825a9b63464ca4..6089dbfee1eebc 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,7 +14,11 @@ import { useState } from '@wordpress/element'; */ import UncontrolledCustomSelectControl from '..'; -const customClass = 'amber-skies'; +const className = 'amber-skies'; +const style = { + backgroundColor: 'rgb(127, 255, 212)', + rotate: '13deg', +}; const legacyProps = { label: 'label!', @@ -26,7 +30,7 @@ const legacyProps = { { key: 'flower2', name: 'crimson clover', - className: customClass, + className, }, { key: 'flower3', @@ -35,15 +39,18 @@ const legacyProps = { { key: 'color1', name: 'amber', - className: customClass, + className, }, { key: 'color2', name: 'aquamarine', - style: { - backgroundColor: 'rgb(127, 255, 212)', - rotate: '13deg', - }, + style, + }, + { + key: 'aquarela-key', + name: 'aquarela', + className, + style, }, ], }; @@ -53,7 +60,9 @@ const ControlledCustomSelectControl = ( { onChange, ...restProps }: React.ComponentProps< typeof UncontrolledCustomSelectControl > ) => { + const { value: overrideValue } = restProps; const [ value, setValue ] = useState( options[ 0 ] ); + const initialValue = overrideValue ?? value; return ( option.key === value.key + ( option: any ) => option.key === initialValue.key ) } /> ); @@ -148,7 +157,7 @@ describe.each( [ // assert against filtered array itemsWithClass.map( ( { name } ) => expect( screen.getByRole( 'option', { name } ) ).toHaveClass( - customClass + className ) ); @@ -160,7 +169,7 @@ describe.each( [ // assert against filtered array itemsWithoutClass.map( ( { name } ) => expect( screen.getByRole( 'option', { name } ) ).not.toHaveClass( - customClass + className ) ); } ); @@ -286,7 +295,7 @@ describe.each( [ expect.objectContaining( { inputValue: '', isOpen: false, - selectedItem: { key: 'violets', name: 'violets' }, + selectedItem: { key: 'flower1', name: 'violets' }, type: '', } ) ); @@ -302,6 +311,7 @@ describe.each( [ expect.objectContaining( { inputValue: '', isOpen: false, + selectedItem: expect.objectContaining( { name: 'aquamarine', } ), @@ -328,7 +338,7 @@ describe.each( [ await type( 'p' ); await press.Enter(); - expect( mockOnChange ).toHaveReturnedWith( 'poppy' ); + expect( mockOnChange ).toHaveReturnedWith( 'flower1' ); } ); describe( 'Keyboard behavior and accessibility', () => { @@ -457,4 +467,57 @@ 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 + // + // Returning these properties as part of the item object was not tested as part of the V1 test. Possibly this was an accidental API? + // or was it intentional? If intentional, we might need to implement something similar in V2, too? The alternative is to rely on the + // `key` attriute for the item and get the actual data from some dictionary in a store somewhere, which would require refactoring + // consumers that rely on the self-contained `style` and `className` attributes. + it( 'Should return style metadata as part of the selected option from onChange', async () => { + const mockOnChange = jest.fn(); + + render( ); + + await click( + screen.getByRole( 'combobox', { + expanded: false, + } ) + ); + + await click( + screen.getByRole( 'option', { + name: 'aquarela', + } ) + ); + + expect( mockOnChange ).toHaveBeenCalledWith( + expect.objectContaining( { + selectedItem: expect.objectContaining( { + className, + style, + } ), + } ) + ); + } ); + + it( 'Should display the initial value passed as the selected value', async () => { + const initialSelectedItem = legacyProps.options[ 5 ]; + + const testProps = { + ...legacyProps, + value: initialSelectedItem, + }; + + render( ); + + const currentSelectedItem = await screen.findByRole( 'combobox', { + expanded: false, + } ); + + // Verify the initial selected value + expect( currentSelectedItem ).toHaveTextContent( 'aquarela' ); + } ); } ); diff --git a/packages/components/src/custom-select-control-v2/styles.ts b/packages/components/src/custom-select-control-v2/styles.ts index 676a9c1a1ec590..5950639e8abf34 100644 --- a/packages/components/src/custom-select-control-v2/styles.ts +++ b/packages/components/src/custom-select-control-v2/styles.ts @@ -14,10 +14,15 @@ import type { CustomSelectButtonSize } from './types'; const ITEM_PADDING = space( 2 ); +export const SelectWrapper = styled.div` + display: block; +`; + export const WithHintWrapper = styled.div` display: flex; justify-content: space-between; flex: 1; + flex-wrap: wrap; `; export const SelectedExperimentalHintItem = styled.span` @@ -37,6 +42,7 @@ export const SelectLabel = styled( Ariakit.SelectLabel )` line-height: 1.4; text-transform: uppercase; margin-bottom: ${ space( 2 ) }; + display: block; `; export const Select = styled( Ariakit.Select, { @@ -96,6 +102,11 @@ export const SelectPopover = styled( Ariakit.SelectPopover )` border-radius: 2px; background: ${ COLORS.theme.background }; border: 1px solid ${ COLORS.theme.foreground }; + z-index: 9999; // Ensure the popover is on top + position: absolute !important; + max-height: 400px; + overflow: auto; + min-width: 100%; &[data-focus-visible] { outline: none; // outline will be on the trigger, rather than the popover @@ -109,6 +120,7 @@ export const SelectItem = styled( Ariakit.SelectItem )` padding: ${ ITEM_PADDING }; font-size: ${ CONFIG.fontSize }; line-height: 2.15rem; // TODO: Remove this in default but keep for back-compat in legacy + user-select: none; &[data-active-item] { background-color: ${ COLORS.theme.gray[ 300 ] }; } diff --git a/packages/components/src/index.ts b/packages/components/src/index.ts index a824162cb24129..bf42504dde1e34 100644 --- a/packages/components/src/index.ts +++ b/packages/components/src/index.ts @@ -63,7 +63,8 @@ export { useCompositeState as __unstableUseCompositeState, } from './composite'; export { ConfirmDialog as __experimentalConfirmDialog } from './confirm-dialog'; -export { StableCustomSelectControl as CustomSelectControl } from './custom-select-control'; +export { ClassicCustomSelectControlV2Adapter as CustomSelectControl } from './custom-select-control-v2/legacy-component'; +export { default as CustomSelectControlV2 } from './custom-select-control-v2'; export { default as Dashicon } from './dashicon'; export { default as DateTimePicker, DatePicker, TimePicker } from './date-time'; export { default as __experimentalDimensionControl } from './dimension-control';