diff --git a/.changeset/old-guests-cover.md b/.changeset/old-guests-cover.md new file mode 100644 index 00000000000..b29ed87597d --- /dev/null +++ b/.changeset/old-guests-cover.md @@ -0,0 +1,5 @@ +--- +'@shopify/polaris': patch +--- + +Improve the type safety and accuracy of ChoiceList diff --git a/polaris-react/src/components/ChoiceList/ChoiceList.stories.tsx b/polaris-react/src/components/ChoiceList/ChoiceList.stories.tsx index e3c6424478b..ff9fe92c861 100644 --- a/polaris-react/src/components/ChoiceList/ChoiceList.stories.tsx +++ b/polaris-react/src/components/ChoiceList/ChoiceList.stories.tsx @@ -1,15 +1,18 @@ import React, {useCallback, useState} from 'react'; import type {ComponentMeta} from '@storybook/react'; +import type {ChoiceListProps} from '@shopify/polaris'; import {ChoiceList, TextField} from '@shopify/polaris'; export default { component: ChoiceList, } as ComponentMeta; -export function Default() { - const [selected, setSelected] = useState(['hidden']); +type HiddenOptionalRequired = 'hidden' | 'optional' | 'required'; - const handleChange = useCallback((value) => setSelected(value), []); +export function Default() { + const [selected, setSelected] = useState([ + 'hidden', + ]); return ( ); } export function WithError() { - const [selected, setSelected] = useState(['hidden']); - - const handleChange = useCallback((value) => setSelected(value), []); + const [selected, setSelected] = useState([ + 'hidden', + ]); return ( ); } export function Magic() { - const [selected, setSelected] = useState(['hidden']); + const [selected, setSelected] = useState([ + 'hidden', + ]); - const handleChange = useCallback((value) => setSelected(value), []); + const choices: ChoiceListProps['choices'] = [ + {label: 'Hidden', value: 'hidden'}, + {label: 'Optional', value: 'optional'}, + {label: 'Required', value: 'required'}, + ]; return ( ); } export function WithMultiChoice() { - const [selected, setSelected] = useState(['hidden']); - - const handleChange = useCallback((value) => setSelected(value), []); + const [selected, setSelected] = useState([]); return ( ); } export function MagicWithMultiChoice() { - const [selected, setSelected] = useState(['hidden']); - - const handleChange = useCallback((value) => setSelected(value), []); + const [selected, setSelected] = useState([]); return ( ); @@ -128,24 +129,17 @@ export function WithChildrenContent() { const [selected, setSelected] = useState(['none']); const [textFieldValue, setTextFieldValue] = useState(''); - const handleChoiceListChange = useCallback((value) => setSelected(value), []); - - const handleTextFieldChange = useCallback( - (value) => setTextFieldValue(value), - [], - ); - const renderChildren = useCallback( () => ( ), - [handleTextFieldChange, textFieldValue], + [textFieldValue], ); return ( @@ -164,7 +158,7 @@ export function WithChildrenContent() { }, ]} selected={selected} - onChange={handleChoiceListChange} + onChange={setSelected} /> ); } @@ -173,25 +167,18 @@ export function WithDynamicChildrenContent() { const [selected, setSelected] = useState(['none']); const [textFieldValue, setTextFieldValue] = useState(''); - const handleChoiceListChange = useCallback((value) => setSelected(value), []); - - const handleTextFieldChange = useCallback( - (value) => setTextFieldValue(value), - [], - ); - const renderChildren = useCallback( (isSelected) => isSelected && ( ), - [handleTextFieldChange, textFieldValue], + [textFieldValue], ); return ( @@ -212,7 +199,7 @@ export function WithDynamicChildrenContent() { }, ]} selected={selected} - onChange={handleChoiceListChange} + onChange={setSelected} /> ); diff --git a/polaris-react/src/components/ChoiceList/ChoiceList.tsx b/polaris-react/src/components/ChoiceList/ChoiceList.tsx index 6226bdbdec0..184b3891a44 100644 --- a/polaris-react/src/components/ChoiceList/ChoiceList.tsx +++ b/polaris-react/src/components/ChoiceList/ChoiceList.tsx @@ -10,9 +10,9 @@ import {Bleed} from '../Bleed'; import styles from './ChoiceList.scss'; -interface Choice { +interface Choice { /** Value of the choice */ - value: string; + value: TValue; /** Label for the choice */ label: React.ReactNode; /** A unique identifier for the choice */ @@ -27,13 +27,13 @@ interface Choice { renderChildren?(isSelected: boolean): React.ReactNode | false; } -export interface ChoiceListProps { +export interface ChoiceListProps { /** Label for list of choices */ title: React.ReactNode; /** Collection of choices */ - choices: Choice[]; + choices: Choice[]; /** Collection of selected choices */ - selected: string[]; + selected: TValue[]; /** Name for form input */ name?: string; /** Allow merchants to select multiple options at once */ @@ -45,12 +45,12 @@ export interface ChoiceListProps { /** Disable all choices **/ disabled?: boolean; /** Callback when the selected choices change */ - onChange?(selected: string[], name: string): void; + onChange?(selected: TValue[], name: string): void; /** Indicates the tone of the choice list */ tone?: 'magic'; } -export function ChoiceList({ +export function ChoiceList({ title, titleHidden, allowMultiple, @@ -61,7 +61,7 @@ export function ChoiceList({ disabled = false, name: nameProp, tone, -}: ChoiceListProps) { +}: ChoiceListProps) { // Type asserting to any is required for TS3.2 but can be removed when we update to 3.3 // see https://github.com/Microsoft/TypeScript/issues/28768 const ControlComponent: any = allowMultiple ? Checkbox : RadioButton; @@ -97,7 +97,7 @@ export function ChoiceList({ ); } - const isSelected = choiceIsSelected(choice, selected); + const isSelected = selected.includes(choice.value); const renderedChildren = choice.renderChildren ? choice.renderChildren(isSelected) : null; @@ -116,7 +116,7 @@ export function ChoiceList({ label={label} disabled={choiceDisabled || disabled} fill={{xs: true, sm: false}} - checked={choiceIsSelected(choice, selected)} + checked={isSelected} helpText={helpText} onChange={handleChange} ariaDescribedBy={ @@ -154,14 +154,10 @@ export function ChoiceList({ function noop() {} -function choiceIsSelected({value}: Choice, selected: string[]) { - return selected.includes(value); -} - -function updateSelectedChoices( - {value}: Choice, +function updateSelectedChoices( + {value}: Choice, checked: boolean, - selected: string[], + selected: TValue[], allowMultiple = false, ) { if (checked) {