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

Jump to bottom

CustomSelectControlV2: Rename for consistency #60178

Merged
merged 6 commits into from Mar 27, 2024
Merged

CustomSelectControlV2: Rename for consistency #60178

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
cac25d8
CustomSelectControlV2: Rename for consistency
mirka Mar 25, 2024
a64759c
Add changelog
mirka Mar 25, 2024
bf056b6
Improve naming consistency in tests
mirka Mar 26, 2024
7b678d4
Simply subcomponent composition and exports
mirka Mar 26, 2024
f1b79a8
Merge branch 'trunk' into rename-custom-select-v2
mirka Mar 26, 2024
75cbc0d
Apply suggestions to readme
mirka Mar 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
54 changes: 27 additions & 27 deletions packages/components/src/custom-select-control-v2/README.md
View file
Open in desktop
@@ -1,4 +1,4 @@
# CustomSelect
# CustomSelectControlV2

<div class="callout callout-alert">
This feature is still experimental. “Experimental” means this is an early implementation subject to drastic and breaking changes.
Expand All @@ -12,31 +12,31 @@ Used to render a customizable select control component.

#### Uncontrolled Mode

CustomSelect can be used in an uncontrolled mode, where the component manages its own state. In this mode, the `defaultValue` prop can be used to set the initial selected value. If this prop is not set, the first value from the children will be selected by default.
CustomSelectControlV2 can be used in an uncontrolled mode, where the component manages its own state. In this mode, the `defaultValue` prop can be used to set the initial selected value. If this prop is not set, the first value from the children will be selected by default.

mirka marked this conversation as resolved.
Show resolved Hide resolved
```jsx
const UncontrolledCustomSelect = () => (
<CustomSelect label="Colors">
<CustomSelectItem value="Blue">
const UncontrolledCustomSelectControlV2 = () => (
<CustomSelectControlV2 label="Colors">
<CustomSelectControlV2.Item value="Blue">
{ /* The `defaultValue` since it wasn't defined */ }
<span style={ { color: 'blue' } }>Blue</span>
</CustomSelectItem>
<CustomSelectItem value="Purple">
</CustomSelectControlV2.Item>
<CustomSelectControlV2.Item value="Purple">
<span style={ { color: 'purple' } }>Purple</span>
</CustomSelectItem>
<CustomSelectItem value="Pink">
</CustomSelectControlV2.Item>
<CustomSelectControlV2.Item value="Pink">
<span style={ { color: 'deeppink' } }>Pink</span>
</CustomSelectItem>
</CustomSelect>
</CustomSelectControlV2.Item>
</CustomSelectControlV2>
);
```

#### Controlled Mode

CustomSelect can also be used in a controlled mode, where the parent component specifies the `value` and the `onChange` props to control selection.
CustomSelectControlV2 can also be used in a controlled mode, where the parent component specifies the `value` and the `onChange` props to control selection.

mirka marked this conversation as resolved.
Show resolved Hide resolved
```jsx
const ControlledCustomSelect = () => {
const ControlledCustomSelectControlV2 = () => {
const [ value, setValue ] = useState< string | string[] >();

const renderControlledValue = ( renderValue: string | string[] ) => (
Expand All @@ -46,7 +46,7 @@ const ControlledCustomSelect = () => {
);

return (
<CustomSelect
<CustomSelectControlV2
{ ...props }
onChange={ ( nextValue ) => {
setValue( nextValue );
Expand All @@ -55,11 +55,11 @@ const ControlledCustomSelect = () => {
value={ value }
>
{ [ 'blue', 'purple', 'pink' ].map( ( option ) => (
<CustomSelectItem key={ option } value={ option }>
<CustomSelectControlV2.Item key={ option } value={ option }>
{ renderControlledValue( option ) }
</CustomSelectItem>
</CustomSelectControlV2.Item>
) ) }
</CustomSelect>
</CustomSelectControlV2>
);
};
```
Expand All @@ -70,31 +70,31 @@ Multiple selection can be enabled by using an array for the `value` and
`defaultValue` props. The argument of the `onChange` function will also change accordingly.

```jsx
const MultiSelectCustomSelect = () => (
<CustomSelect defaultValue={ [ 'blue', 'pink' ] } label="Colors">
const MultiSelectCustomSelectControlV2 = () => (
<CustomSelectControlV2 defaultValue={ [ 'blue', 'pink' ] } label="Colors">
{ [ 'blue', 'purple', 'pink' ].map( ( item ) => (
<CustomSelectItem key={ item } value={ item }>
<CustomSelectControlV2.Item key={ item } value={ item }>
{ item }
</CustomSelectItem>
</CustomSelectControlV2.Item>
) ) }
</CustomSelect>
</CustomSelectControlV2>
);
```

### Components and Sub-components

CustomSelect is comprised of two individual components:
CustomSelectControlV2 is comprised of two individual components:

mirka marked this conversation as resolved.
Show resolved Hide resolved
- `CustomSelect`: a wrapper component and context provider. It is responsible for managing the state of the `CustomSelectItem` children.
- `CustomSelectItem`: renders a single select item. The first `CustomSelectItem` child will be used as the `defaultValue` when `defaultValue` is undefined.
- `CustomSelectControlV2`: a wrapper component and context provider. It is responsible for managing the state of the `CustomSelectControlV2.Item` children.
- `CustomSelectControlV2.Item`: renders a single select item. The first `CustomSelectControlV2.Item` child will be used as the `defaultValue` when `defaultValue` is undefined.

#### Props

The component accepts the following props:

##### `children`: `React.ReactNode`

The child elements. This should be composed of CustomSelect.Item components.
The child elements. This should be composed of CustomSelectControlV2.Item components.

mirka marked this conversation as resolved.
Show resolved Hide resolved
- Required: yes

Expand Down Expand Up @@ -142,7 +142,7 @@ Can be used to externally control the value of the control.

- Required: no

### `CustomSelectItem`
### `CustomSelectControlV2.Item`

Used to render a select item.

Expand Down
4 changes: 2 additions & 2 deletions packages/components/src/custom-select-control-v2/default-component/index.tsx
View file
Open in desktop
Expand Up @@ -10,7 +10,7 @@ import _CustomSelect from '../custom-select';
import type { CustomSelectProps } from '../types';
import type { WordPressComponentProps } from '../../context';

function CustomSelect(
function CustomSelectControlV2(
props: WordPressComponentProps< CustomSelectProps, 'button', false >
) {
const { defaultValue, onChange, value, ...restProps } = props;
Expand All @@ -24,4 +24,4 @@ function CustomSelect(
return <_CustomSelect { ...restProps } store={ store } />;
}

export default CustomSelect;
export default CustomSelectControlV2;
8 changes: 6 additions & 2 deletions packages/components/src/custom-select-control-v2/index.tsx
View file
Open in desktop
@@ -1,5 +1,9 @@
/**
* Internal dependencies
*/
export { default as CustomSelect } from './default-component';
export { default as CustomSelectItem } from './custom-select-item';
import CustomSelect from './default-component';
Copy link
Member

Choose a reason for hiding this comment

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

Should we also use CustomSelectControlV2 here for consistency?

Copy link
Member Author

Choose a reason for hiding this comment

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

Addressed in 7b678d4

import Item from './item';

const CustomSelectControlV2 = Object.assign( CustomSelect, { Item } );
Copy link
Member

Choose a reason for hiding this comment

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

Do we really need to use Object.assign() rather than just do a direct assignment?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is necessary for TypeScript reasons because here the component is already exported once and "sealed" to new keys.

But I moved the assignment to the default-component file so it's a bit simpler and doesn't require the Object.assign (7b678d4).


export default CustomSelectControlV2;
2 changes: 2 additions & 0 deletions ...-select-control-v2/custom-select-item.tsx → ...nts/src/custom-select-control-v2/item.tsx
View file
Open in desktop
Expand Up @@ -26,4 +26,6 @@ export function CustomSelectItem( {
);
}

CustomSelectItem.displayName = 'CustomSelectControlV2.Item';
Copy link
Member Author

Choose a reason for hiding this comment

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

This was necessary to ensure that React devtools and the Storybook code snippet generator displays the intended name.

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps this shouldn't be necessary if we exported them like this (untested):

const CustomSelectControlV2 = CustomSelect;
CustomSelectControlV2.Item = Item;
export default CustomSelectControlV2;

which would be something similar to what we're doing with Tabs right now.

Copy link
Member Author

Choose a reason for hiding this comment

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

That doesn't seem to work, it looks like it just uses the name of the initial function no matter what.

And your comment made me realize that the Tabs subcomponents aren't showing the correct display names either 😅 I'll fix that.


export default CustomSelectItem;
6 changes: 3 additions & 3 deletions packages/components/src/custom-select-control-v2/legacy-component/index.tsx
View file
Open in desktop
Expand Up @@ -11,12 +11,12 @@ import { useMemo } from '@wordpress/element';
* Internal dependencies
*/
import _CustomSelect from '../custom-select';
import CustomSelectItem from '../item';
import type { LegacyCustomSelectProps } from '../types';
import { CustomSelectItem } from '..';
import * as Styled from '../styles';
import { ContextSystemProvider } from '../../context';

function CustomSelect( props: LegacyCustomSelectProps ) {
function CustomSelectControl( props: LegacyCustomSelectProps ) {
const {
__experimentalShowSelectedHint,
__next40pxDefaultSize = false,
Expand Down Expand Up @@ -130,4 +130,4 @@ function CustomSelect( props: LegacyCustomSelectProps ) {
);
}

export default CustomSelect;
export default CustomSelectControl;
33 changes: 16 additions & 17 deletions packages/components/src/custom-select-control-v2/stories/default.story.tsx
View file
Open in desktop
Expand Up @@ -11,15 +11,14 @@ import { useState } from '@wordpress/element';
/**
* Internal dependencies
*/
import CustomSelect from '../default-component';
import { CustomSelectItem } from '..';
import CustomSelectControlV2 from '..';

const meta: Meta< typeof CustomSelect > = {
const meta: Meta< typeof CustomSelectControlV2 > = {
title: 'Components (Experimental)/CustomSelectControl v2/Default',
component: CustomSelect,
component: CustomSelectControlV2,
subcomponents: {
// @ts-expect-error - See https://github.com/storybookjs/storybook/issues/23170
CustomSelectItem,
'CustomSelectControlV2.Item': CustomSelectControlV2.Item,
},
argTypes: {
children: { control: { type: null } },
Expand Down Expand Up @@ -48,10 +47,10 @@ const meta: Meta< typeof CustomSelect > = {
};
export default meta;

const Template: StoryFn< typeof CustomSelect > = ( props ) => {
const Template: StoryFn< typeof CustomSelectControlV2 > = ( props ) => {
const [ value, setValue ] = useState< string | string[] >();
return (
<CustomSelect
<CustomSelectControlV2
{ ...props }
onChange={ ( nextValue: string | string[] ) => {
setValue( nextValue );
Expand All @@ -68,15 +67,15 @@ Default.args = {
defaultValue: 'Select a color...',
children: (
<>
<CustomSelectItem value="Blue">
<CustomSelectControlV2.Item value="Blue">
<span style={ { color: 'blue' } }>Blue</span>
</CustomSelectItem>
<CustomSelectItem value="Purple">
</CustomSelectControlV2.Item>
<CustomSelectControlV2.Item value="Purple">
<span style={ { color: 'purple' } }>Purple</span>
</CustomSelectItem>
<CustomSelectItem value="Pink">
</CustomSelectControlV2.Item>
<CustomSelectControlV2.Item value="Pink">
<span style={ { color: 'deeppink' } }>Pink</span>
</CustomSelectItem>
</CustomSelectControlV2.Item>
</>
),
};
Expand All @@ -100,9 +99,9 @@ MultipleSelection.args = {
'maroon',
'tangerine',
].map( ( item ) => (
<CustomSelectItem key={ item } value={ item }>
<CustomSelectControlV2.Item key={ item } value={ item }>
{ item }
</CustomSelectItem>
</CustomSelectControlV2.Item>
) ) }
</>
),
Expand Down Expand Up @@ -134,9 +133,9 @@ CustomSelectedValue.args = {
<>
{ [ 'mystery-person', 'identicon', 'wavatar', 'retro' ].map(
( option ) => (
<CustomSelectItem key={ option } value={ option }>
<CustomSelectControlV2.Item key={ option } value={ option }>
{ renderItem( option ) }
</CustomSelectItem>
</CustomSelectControlV2.Item>
)
) }
</>
Expand Down
16 changes: 10 additions & 6 deletions packages/components/src/custom-select-control-v2/stories/legacy.story.tsx
View file
Open in desktop
Expand Up @@ -11,11 +11,11 @@ import { useState } from '@wordpress/element';
/**
* Internal dependencies
*/
import CustomSelect from '../legacy-component';
import CustomSelectControl from '../legacy-component';

const meta: Meta< typeof CustomSelect > = {
const meta: Meta< typeof CustomSelectControl > = {
title: 'Components (Experimental)/CustomSelectControl v2/Legacy',
component: CustomSelect,
component: CustomSelectControl,
argTypes: {
onChange: { control: { type: null } },
value: { control: { type: null } },
Expand All @@ -42,18 +42,22 @@ const meta: Meta< typeof CustomSelect > = {
};
export default meta;

const Template: StoryFn< typeof CustomSelect > = ( props ) => {
const Template: StoryFn< typeof CustomSelectControl > = ( props ) => {
const [ fontSize, setFontSize ] = useState( props.options[ 0 ] );

const onChange: React.ComponentProps<
typeof CustomSelect
typeof CustomSelectControl
>[ 'onChange' ] = ( changeObject ) => {
setFontSize( changeObject.selectedItem );
props.onChange?.( changeObject );
};

return (
<CustomSelect { ...props } onChange={ onChange } value={ fontSize } />
<CustomSelectControl
{ ...props }
onChange={ onChange }
value={ fontSize }
/>
);
};

Expand Down
34 changes: 20 additions & 14 deletions packages/components/src/custom-select-control-v2/test/index.tsx
View file
Open in desktop
Expand Up @@ -12,7 +12,7 @@ import { useState } from '@wordpress/element';
/**
* Internal dependencies
*/
import { CustomSelect as UncontrolledCustomSelect, CustomSelectItem } from '..';
import UncontrolledCustomSelectControl from '..';
Copy link
Member

Choose a reason for hiding this comment

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

Should we use UncontrolledCustomSelectControlV2 instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

Addressed in bf056b6 👍

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

const items = [
Expand Down Expand Up @@ -41,14 +41,14 @@ const items = [
const defaultProps = {
label: 'label!',
children: items.map( ( { value, key } ) => (
<CustomSelectItem value={ value } key={ key } />
<UncontrolledCustomSelectControl.Item value={ value } key={ key } />
) ),
};

const ControlledCustomSelect = ( props: CustomSelectProps ) => {
const ControlledCustomSelectControl = ( props: CustomSelectProps ) => {
const [ value, setValue ] = useState< string | string[] >();
return (
<UncontrolledCustomSelect
<UncontrolledCustomSelectControl
{ ...props }
onChange={ ( nextValue: string | string[] ) => {
setValue( nextValue );
Expand All @@ -60,8 +60,8 @@ const ControlledCustomSelect = ( props: CustomSelectProps ) => {
};

describe.each( [
[ 'Uncontrolled', UncontrolledCustomSelect ],
[ 'Controlled', ControlledCustomSelect ],
[ 'Uncontrolled', UncontrolledCustomSelectControl ],
[ 'Controlled', ControlledCustomSelectControl ],
] )( 'CustomSelectControlV2 (%s)', ( ...modeAndComponent ) => {
const [ , Component ] = modeAndComponent;

Expand Down Expand Up @@ -257,9 +257,12 @@ describe.each( [
'rose blush',
'ultraviolet morning light',
].map( ( item ) => (
<CustomSelectItem key={ item } value={ item }>
<UncontrolledCustomSelectControl.Item
key={ item }
value={ item }
>
{ item }
</CustomSelectItem>
</UncontrolledCustomSelectControl.Item>
) ) }
</Component>
);
Expand Down Expand Up @@ -326,9 +329,12 @@ describe.each( [
render(
<Component defaultValue={ defaultValues } label="Multi-select">
{ defaultValues.map( ( item ) => (
<CustomSelectItem key={ item } value={ item }>
<UncontrolledCustomSelectControl.Item
key={ item }
value={ item }
>
{ item }
</CustomSelectItem>
</UncontrolledCustomSelectControl.Item>
) ) }
</Component>
);
Expand Down Expand Up @@ -378,12 +384,12 @@ describe.each( [

render(
<Component label="Rendered" renderSelectedValue={ renderValue }>
<CustomSelectItem value="april-29">
<UncontrolledCustomSelectControl.Item value="april-29">
{ renderValue( 'april-29' ) }
</CustomSelectItem>
<CustomSelectItem value="july-9">
</UncontrolledCustomSelectControl.Item>
<UncontrolledCustomSelectControl.Item value="july-9">
{ renderValue( 'july-9' ) }
</CustomSelectItem>
</UncontrolledCustomSelectControl.Item>
</Component>
);

Expand Down