From 84a26d8111dc4bb9d3cd1edbb99691e68899d261 Mon Sep 17 00:00:00 2001 From: Sarah Higley Date: Fri, 2 Dec 2022 18:14:18 -0800 Subject: [PATCH] feat: change Combobox/Dropdown Option to require `label` and use `value` for non-display text (#25379) --- ...-11a96c34-a691-46b7-800f-207349570f7b.json | 7 ++ .../react-combobox/etc/react-combobox.api.md | 8 +- .../src/components/Combobox/Combobox.test.tsx | 62 +++++++++++++++ .../src/components/Combobox/useCombobox.tsx | 6 +- .../src/components/Dropdown/Dropdown.test.tsx | 62 +++++++++++++++ .../src/components/Dropdown/useDropdown.tsx | 6 +- .../src/components/Listbox/Listbox.test.tsx | 12 ++- .../src/components/Option/Option.test.tsx | 56 +++++++++++-- .../src/components/Option/Option.types.ts | 28 ++++++- .../src/components/Option/useOption.tsx | 30 ++++--- .../ComboboxComplexOptions.stories.tsx | 79 +++++++++++++++++++ .../src/stories/Combobox/index.stories.tsx | 1 + .../DropdownComplexOptions.stories.tsx | 79 +++++++++++++++++++ .../DropdownCustomOptions.stories.tsx | 2 +- .../src/stories/Dropdown/index.stories.tsx | 1 + .../src/utils/ComboboxBase.types.ts | 2 +- .../src/utils/OptionCollection.types.ts | 7 +- .../src/utils/Selection.types.ts | 8 +- .../src/utils/useComboboxBaseState.ts | 6 +- .../src/utils/useOptionCollection.ts | 6 +- .../react-combobox/src/utils/useSelection.ts | 4 +- 21 files changed, 428 insertions(+), 44 deletions(-) create mode 100644 change/@fluentui-react-combobox-11a96c34-a691-46b7-800f-207349570f7b.json create mode 100644 packages/react-components/react-combobox/src/stories/Combobox/ComboboxComplexOptions.stories.tsx create mode 100644 packages/react-components/react-combobox/src/stories/Dropdown/DropdownComplexOptions.stories.tsx diff --git a/change/@fluentui-react-combobox-11a96c34-a691-46b7-800f-207349570f7b.json b/change/@fluentui-react-combobox-11a96c34-a691-46b7-800f-207349570f7b.json new file mode 100644 index 0000000000000..e123c349cf9d9 --- /dev/null +++ b/change/@fluentui-react-combobox-11a96c34-a691-46b7-800f-207349570f7b.json @@ -0,0 +1,7 @@ +{ + "type": "prerelease", + "comment": " feat: update Option value to be a non-display value, and add an Option text prop that is required if the child is not a string", + "packageName": "@fluentui/react-combobox", + "email": "sarah.higley@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/packages/react-components/react-combobox/etc/react-combobox.api.md b/packages/react-components/react-combobox/etc/react-combobox.api.md index 2b7bf39e9bb3d..4b410c323e64f 100644 --- a/packages/react-components/react-combobox/etc/react-combobox.api.md +++ b/packages/react-components/react-combobox/etc/react-combobox.api.md @@ -160,7 +160,13 @@ export type OptionGroupState = ComponentState; export type OptionProps = ComponentProps> & { disabled?: boolean; value?: string; -}; +} & ({ + text?: string; + children: string; +} | { + text: string; + children?: React_2.ReactNode; +}); // @public (undocumented) export type OptionSlots = { diff --git a/packages/react-components/react-combobox/src/components/Combobox/Combobox.test.tsx b/packages/react-components/react-combobox/src/components/Combobox/Combobox.test.tsx index 5d31d5dd1dc1d..4238c672acae6 100644 --- a/packages/react-components/react-combobox/src/components/Combobox/Combobox.test.tsx +++ b/packages/react-components/react-combobox/src/components/Combobox/Combobox.test.tsx @@ -278,6 +278,23 @@ describe('Combobox', () => { expect(getByTestId('blue').getAttribute('aria-selected')).toEqual('false'); }); + it('should set defaultSelectedOptions based on Option `value`', () => { + const { getByTestId } = render( + + + + + , + ); + + expect(getByTestId('green').getAttribute('aria-selected')).toEqual('true'); + expect(getByTestId('blue').getAttribute('aria-selected')).toEqual('true'); + }); + it('should set selectedOptions', () => { const { getByTestId } = render( @@ -290,6 +307,26 @@ describe('Combobox', () => { expect(getByTestId('green').getAttribute('aria-selected')).toEqual('true'); }); + it('should set selectedOptions based on Option `value`', () => { + const { getByTestId } = render( + + + + + , + ); + + expect(getByTestId('red').getAttribute('aria-selected')).toEqual('true'); + expect(getByTestId('blue').getAttribute('aria-selected')).toEqual('true'); + expect(getByTestId('green').getAttribute('aria-selected')).toEqual('false'); + }); + it('should change defaultSelectedOptions on click', () => { const { getByTestId } = render( @@ -458,10 +495,33 @@ describe('Combobox', () => { expect(onOptionSelect).toHaveBeenCalledTimes(1); expect(onOptionSelect).toHaveBeenCalledWith(expect.anything(), { optionValue: 'Green', + optionText: 'Green', selectedOptions: ['Green'], }); }); + it('calls onOptionSelect with Option value prop', () => { + const onOptionSelect = jest.fn(); + + const { getByRole, getByText } = render( + + + + + , + ); + + userEvent.click(getByRole('combobox')); + userEvent.click(getByText('Green')); + + expect(onOptionSelect).toHaveBeenCalledTimes(1); + expect(onOptionSelect).toHaveBeenCalledWith(expect.anything(), { + optionValue: 'test', + optionText: 'Green', + selectedOptions: ['test'], + }); + }); + it('calls onOptionSelect with correct data for multi-select', () => { const onOptionSelect = jest.fn(); @@ -480,10 +540,12 @@ describe('Combobox', () => { expect(onOptionSelect).toHaveBeenCalledTimes(2); expect(onOptionSelect).toHaveBeenNthCalledWith(1, expect.anything(), { optionValue: 'Green', + optionText: 'Green', selectedOptions: ['Green'], }); expect(onOptionSelect).toHaveBeenNthCalledWith(2, expect.anything(), { optionValue: 'Blue', + optionText: 'Blue', selectedOptions: ['Green', 'Blue'], }); }); diff --git a/packages/react-components/react-combobox/src/components/Combobox/useCombobox.tsx b/packages/react-components/react-combobox/src/components/Combobox/useCombobox.tsx index eeaedad25231e..a9c9496325386 100644 --- a/packages/react-components/react-combobox/src/components/Combobox/useCombobox.tsx +++ b/packages/react-components/react-combobox/src/components/Combobox/useCombobox.tsx @@ -32,7 +32,7 @@ export const useCombobox_unstable = (props: ComboboxProps, ref: React.Ref optionValue.toLowerCase().indexOf(searchString) === 0; - const matches = getOptionsMatchingValue(matcher); + const matcher = (optionText: string) => optionText.toLowerCase().indexOf(searchString) === 0; + const matches = getOptionsMatchingText(matcher); // return first matching option after the current active option, looping back to the top if (matches.length > 1 && activeOption) { diff --git a/packages/react-components/react-combobox/src/components/Dropdown/Dropdown.test.tsx b/packages/react-components/react-combobox/src/components/Dropdown/Dropdown.test.tsx index b5a838d4bbaa6..97206d2722dec 100644 --- a/packages/react-components/react-combobox/src/components/Dropdown/Dropdown.test.tsx +++ b/packages/react-components/react-combobox/src/components/Dropdown/Dropdown.test.tsx @@ -214,6 +214,23 @@ describe('Dropdown', () => { expect(getByTestId('blue').getAttribute('aria-selected')).toEqual('false'); }); + it('should set defaultSelectedOptions based on Option `value`', () => { + const { getByTestId } = render( + + + + + , + ); + + expect(getByTestId('green').getAttribute('aria-selected')).toEqual('true'); + expect(getByTestId('blue').getAttribute('aria-selected')).toEqual('true'); + }); + it('should set selectedOptions', () => { const { getByTestId } = render( @@ -226,6 +243,26 @@ describe('Dropdown', () => { expect(getByTestId('green').getAttribute('aria-selected')).toEqual('true'); }); + it('should set selectedOptions based on Option `value`', () => { + const { getByTestId } = render( + + + + + , + ); + + expect(getByTestId('red').getAttribute('aria-selected')).toEqual('true'); + expect(getByTestId('blue').getAttribute('aria-selected')).toEqual('true'); + expect(getByTestId('green').getAttribute('aria-selected')).toEqual('false'); + }); + it('should change defaultSelectedOptions on click', () => { const { getByTestId } = render( @@ -340,10 +377,33 @@ describe('Dropdown', () => { expect(onOptionSelect).toHaveBeenCalledTimes(1); expect(onOptionSelect).toHaveBeenCalledWith(expect.anything(), { optionValue: 'Green', + optionText: 'Green', selectedOptions: ['Green'], }); }); + it('calls onOptionSelect with Option value prop', () => { + const onOptionSelect = jest.fn(); + + const { getByRole, getByText } = render( + + + + + , + ); + + userEvent.click(getByRole('combobox')); + userEvent.click(getByText('Green')); + + expect(onOptionSelect).toHaveBeenCalledTimes(1); + expect(onOptionSelect).toHaveBeenCalledWith(expect.anything(), { + optionValue: 'test', + optionText: 'Green', + selectedOptions: ['test'], + }); + }); + it('calls onOptionSelect with correct data for multi-select', () => { const onOptionSelect = jest.fn(); @@ -362,10 +422,12 @@ describe('Dropdown', () => { expect(onOptionSelect).toHaveBeenCalledTimes(2); expect(onOptionSelect).toHaveBeenNthCalledWith(1, expect.anything(), { optionValue: 'Green', + optionText: 'Green', selectedOptions: ['Green'], }); expect(onOptionSelect).toHaveBeenNthCalledWith(2, expect.anything(), { optionValue: 'Blue', + optionText: 'Blue', selectedOptions: ['Green', 'Blue'], }); }); diff --git a/packages/react-components/react-combobox/src/components/Dropdown/useDropdown.tsx b/packages/react-components/react-combobox/src/components/Dropdown/useDropdown.tsx index 5024a7d1279ca..143d8a31565f0 100644 --- a/packages/react-components/react-combobox/src/components/Dropdown/useDropdown.tsx +++ b/packages/react-components/react-combobox/src/components/Dropdown/useDropdown.tsx @@ -25,7 +25,7 @@ export const useDropdown_unstable = (props: DropdownProps, ref: React.Ref { // first check for matches for the full searchString let matcher = (optionValue: string) => optionValue.toLowerCase().indexOf(searchString.current) === 0; - let matches = getOptionsMatchingValue(matcher); + let matches = getOptionsMatchingText(matcher); let startIndex = activeOption ? getIndexOfId(activeOption.id) : 0; // if the dropdown is already open and the searchstring is a single character, @@ -72,7 +72,7 @@ export const useDropdown_unstable = (props: DropdownProps, ref: React.Ref optionValue.toLowerCase().indexOf(letters[0]) === 0; - matches = getOptionsMatchingValue(matcher); + matches = getOptionsMatchingText(matcher); } } diff --git a/packages/react-components/react-combobox/src/components/Listbox/Listbox.test.tsx b/packages/react-components/react-combobox/src/components/Listbox/Listbox.test.tsx index d219e814102bd..8647523f17b72 100644 --- a/packages/react-components/react-combobox/src/components/Listbox/Listbox.test.tsx +++ b/packages/react-components/react-combobox/src/components/Listbox/Listbox.test.tsx @@ -219,7 +219,11 @@ describe('Listbox', () => { fireEvent.click(getByText('Red')); expect(onOptionSelect).toHaveBeenCalled(); - expect(onOptionSelect.mock.calls[0][1]).toEqual({ optionValue: 'Red', selectedOptions: ['Red'] }); + expect(onOptionSelect.mock.calls[0][1]).toEqual({ + optionValue: 'Red', + optionText: 'Red', + selectedOptions: ['Red'], + }); }); it('should not change selection with selectedOptions', () => { @@ -239,7 +243,11 @@ describe('Listbox', () => { expect(option.getAttribute('aria-selected')).toEqual('false'); expect(onOptionSelect).toHaveBeenCalled(); - expect(onOptionSelect.mock.calls[0][1]).toEqual({ optionValue: 'Red', selectedOptions: ['Red'] }); + expect(onOptionSelect.mock.calls[0][1]).toEqual({ + optionValue: 'Red', + optionText: 'Red', + selectedOptions: ['Red'], + }); }); it('should select option with the enter key', () => { diff --git a/packages/react-components/react-combobox/src/components/Option/Option.test.tsx b/packages/react-components/react-combobox/src/components/Option/Option.test.tsx index 7a05bf547b281..ced800c425e71 100644 --- a/packages/react-components/react-combobox/src/components/Option/Option.test.tsx +++ b/packages/react-components/react-combobox/src/components/Option/Option.test.tsx @@ -3,10 +3,11 @@ import { resetIdsForTests } from '@fluentui/react-utilities'; import { fireEvent, render } from '@testing-library/react'; import { ListboxContext } from '../../contexts/ListboxContext'; import { Option } from './Option'; +import type { OptionProps } from './Option.types'; import { isConformant } from '../../common/isConformant'; describe('Option', () => { - isConformant({ + isConformant({ Component: Option, displayName: 'Option', }); @@ -36,6 +37,16 @@ describe('Option', () => { expect(result.container).toMatchSnapshot(); }); + it('renders child content', () => { + const result = render( + , + ); + expect(result.getByText('Text content').tagName).toBe('B'); + expect(result.queryByText('Default')).toBeNull(); + }); + it('renders a default multi-select state', () => { const result = render( @@ -67,6 +78,18 @@ describe('Option', () => { }); it('sets aria-selected based on text', () => { + const { getByText } = render( + + + + , + ); + + expect(getByText('one').getAttribute('aria-selected')).toEqual('true'); + expect(getByText('two').getAttribute('aria-selected')).toEqual('false'); + }); + + it('sets aria-selected based on children', () => { const { getByText } = render( @@ -81,11 +104,26 @@ describe('Option', () => { it('ignores text if value is set', () => { const { getByText } = render( - + , ); - expect(getByText('Option 1').getAttribute('aria-selected')).toEqual('false'); + expect(getByText('one').getAttribute('aria-selected')).toEqual('false'); + }); + + it('calls console.warn if the text prop is absent and the children is not a string', () => { + const warnFn = jest.spyOn(console, 'warn').mockImplementation(() => undefined); + render( + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + , + ); + + expect(warnFn).toHaveBeenCalledTimes(1); }); it('calls onClick', () => { @@ -126,6 +164,7 @@ describe('Option', () => { expect(typeof registerProps?.id).toEqual('string'); expect(registerProps?.disabled).toBeFalsy(); expect(registerProps?.value).toEqual('Option 1'); + expect(registerProps?.text).toEqual('Option 1'); expect(registerRef).toEqual(getByRole('option')); }); @@ -133,8 +172,8 @@ describe('Option', () => { const registerOption = jest.fn(); render( - , ); @@ -144,18 +183,19 @@ describe('Option', () => { expect(registerProps?.id).toEqual('op1'); expect(registerProps?.disabled).toBeTruthy(); expect(registerProps?.value).toEqual('foo'); + expect(registerProps?.text).toEqual('Option 1'); }); it('re-registers when the value changes', () => { const registerOption = jest.fn(); const result = render( - + , ); result.rerender( - + , ); @@ -174,7 +214,7 @@ describe('Option', () => { ); fireEvent.click(getByRole('option')); - const optionData = { id: 'optionId', disabled: undefined, value: 'foo' }; + const optionData = { id: 'optionId', disabled: undefined, text: 'Option 1', value: 'foo' }; expect(selectOption).toHaveBeenCalledTimes(1); expect(selectOption).toHaveBeenCalledWith(expect.anything(), optionData); diff --git a/packages/react-components/react-combobox/src/components/Option/Option.types.ts b/packages/react-components/react-combobox/src/components/Option/Option.types.ts index 2d83a664a28f5..3db9cf287dbbf 100644 --- a/packages/react-components/react-combobox/src/components/Option/Option.types.ts +++ b/packages/react-components/react-combobox/src/components/Option/Option.types.ts @@ -1,4 +1,5 @@ import type { ComponentProps, ComponentState, Slot } from '@fluentui/react-utilities'; +import * as React from 'react'; export type OptionSlots = { /* The root option slot, with role="option" */ @@ -19,11 +20,32 @@ export type OptionProps = ComponentProps> & { disabled?: boolean; /* - * Defines a string value for the option, used for the parent Combobox's value. - * Use this if the children are not a string, or you wish the value to differ from the displayed text. + * Defines a unique identifier for the option. + * Use this to control selectedOptions, or to get the option value in the onOptionSelect callback. + * Defaults to `text` if not provided. */ value?: string; -}; +} & ( + | { + /** + * An optional override the string value of the Option's display text, + * defaulting to the Option's child content. + * This is used as the Dropdown button's or Combobox input's value when the option is selected, + * and as the comparison for type-to-find keyboard functionality. + */ + text?: string; + children: string; + } + | { + /** + * The string value of the Option's display text when the Option's children are not a string. + * This is used as the Dropdown button's or Combobox input's value when the option is selected, + * and as the comparison for type-to-find keyboard functionality. + */ + text: string; + children?: React.ReactNode; + } + ); /** * State used in rendering Option diff --git a/packages/react-components/react-combobox/src/components/Option/useOption.tsx b/packages/react-components/react-combobox/src/components/Option/useOption.tsx index 8bf86d69b00d3..e05a9e5a13e3a 100644 --- a/packages/react-components/react-combobox/src/components/Option/useOption.tsx +++ b/packages/react-components/react-combobox/src/components/Option/useOption.tsx @@ -7,18 +7,28 @@ import { ListboxContext } from '../../contexts/ListboxContext'; import type { OptionValue } from '../../utils/OptionCollection.types'; import type { OptionProps, OptionState } from './Option.types'; -function getValueString(value: string | undefined, children: React.ReactNode) { - if (value) { - return value; +function getTextString(text: string | undefined, children: React.ReactNode) { + if (text !== undefined) { + return text; } - let valueString = ''; + let textString = ''; + let hasNonStringChild = false; React.Children.forEach(children, child => { if (typeof child === 'string') { - valueString += child; + textString += child; + } else { + hasNonStringChild = true; } }); - return valueString; + + // warn if an Option has non-string children and no text prop + if (hasNonStringChild) { + // eslint-disable-next-line no-console + console.warn('Provide a `text` prop to Option components when they contain non-string children.'); + } + + return textString; } /** @@ -31,17 +41,19 @@ function getValueString(value: string | undefined, children: React.ReactNode) { * @param ref - reference to root HTMLElement of Option */ export const useOption_unstable = (props: OptionProps, ref: React.Ref): OptionState => { - const { disabled, value } = props; + const { children, disabled, text, value } = props; const optionRef = React.useRef(null); - const optionValue = getValueString(value, props.children); + const optionText = getTextString(text, children); + const optionValue = value ?? optionText; // use the id if provided, otherwise use a generated id const id = useId('fluent-option', props.id); // data used for context registration & events - const optionData = React.useMemo(() => ({ id, disabled, value: optionValue }), [ + const optionData = React.useMemo(() => ({ id, disabled, text: optionText, value: optionValue }), [ id, disabled, + optionText, optionValue, ]); diff --git a/packages/react-components/react-combobox/src/stories/Combobox/ComboboxComplexOptions.stories.tsx b/packages/react-components/react-combobox/src/stories/Combobox/ComboboxComplexOptions.stories.tsx new file mode 100644 index 0000000000000..dcc1426e0804d --- /dev/null +++ b/packages/react-components/react-combobox/src/stories/Combobox/ComboboxComplexOptions.stories.tsx @@ -0,0 +1,79 @@ +import * as React from 'react'; +import { Persona } from '@fluentui/react-persona'; +import { makeStyles, shorthands, useId } from '@fluentui/react-components'; +import { Combobox, Option } from '@fluentui/react-combobox'; +import type { ComboboxProps } from '@fluentui/react-combobox'; + +const useStyles = makeStyles({ + root: { + // Stack the label above the field with a gap + display: 'grid', + gridTemplateRows: 'repeat(1fr)', + justifyItems: 'start', + ...shorthands.gap('2px'), + maxWidth: '400px', + }, +}); + +export const ComplexOptions = (props: Partial) => { + const comboId = useId('combo-default'); + const styles = useStyles(); + return ( +
+ + + + + + + +
+ ); +}; + +ComplexOptions.parameters = { + docs: { + description: { + story: + 'Options can have structured JSX children. ' + + "When this is the case, the Option's `text` prop should be a the plain text version of the option, " + + 'and is used as the Combobox value when the option is selected.', + }, + }, +}; diff --git a/packages/react-components/react-combobox/src/stories/Combobox/index.stories.tsx b/packages/react-components/react-combobox/src/stories/Combobox/index.stories.tsx index 3bed9452a6952..b01ea1a26349d 100644 --- a/packages/react-components/react-combobox/src/stories/Combobox/index.stories.tsx +++ b/packages/react-components/react-combobox/src/stories/Combobox/index.stories.tsx @@ -5,6 +5,7 @@ import descriptionMd from './ComboboxDescription.md'; import bestPracticesMd from './ComboboxBestPractices.md'; export { Default } from './ComboboxDefault.stories'; +export { ComplexOptions } from './ComboboxComplexOptions.stories'; export { CustomOptions } from './ComboboxCustomOptions.stories'; export { Freeform } from './ComboboxFreeform.stories'; export { Multiselect } from './ComboboxMultiselect.stories'; diff --git a/packages/react-components/react-combobox/src/stories/Dropdown/DropdownComplexOptions.stories.tsx b/packages/react-components/react-combobox/src/stories/Dropdown/DropdownComplexOptions.stories.tsx new file mode 100644 index 0000000000000..f7f31f4385168 --- /dev/null +++ b/packages/react-components/react-combobox/src/stories/Dropdown/DropdownComplexOptions.stories.tsx @@ -0,0 +1,79 @@ +import * as React from 'react'; +import { Persona } from '@fluentui/react-persona'; +import { makeStyles, shorthands, useId } from '@fluentui/react-components'; +import { Dropdown, Option } from '@fluentui/react-combobox'; +import type { DropdownProps } from '@fluentui/react-combobox'; + +const useStyles = makeStyles({ + root: { + // Stack the label above the field with a gap + display: 'grid', + gridTemplateRows: 'repeat(1fr)', + justifyItems: 'start', + ...shorthands.gap('2px'), + maxWidth: '400px', + }, +}); + +export const ComplexOptions = (props: Partial) => { + const dropdownId = useId('dropdown'); + const styles = useStyles(); + return ( +
+ + + + + + + +
+ ); +}; + +ComplexOptions.parameters = { + docs: { + description: { + story: + 'Options can have structured JSX children. ' + + "When this is the case, the Option's `text` prop should be a plain text version of the option, " + + "and is used as the Dropdown button's content when the option is selected.", + }, + }, +}; diff --git a/packages/react-components/react-combobox/src/stories/Dropdown/DropdownCustomOptions.stories.tsx b/packages/react-components/react-combobox/src/stories/Dropdown/DropdownCustomOptions.stories.tsx index 41ff37cd9177f..9503f3131c0c6 100644 --- a/packages/react-components/react-combobox/src/stories/Dropdown/DropdownCustomOptions.stories.tsx +++ b/packages/react-components/react-combobox/src/stories/Dropdown/DropdownCustomOptions.stories.tsx @@ -40,7 +40,7 @@ const CustomOption = (props: CustomOptionProps) => { const Icon = animalIcons[animal]; const styles = useCustomOptionStyles(); return ( - diff --git a/packages/react-components/react-combobox/src/stories/Dropdown/index.stories.tsx b/packages/react-components/react-combobox/src/stories/Dropdown/index.stories.tsx index a584994bcfc3e..3fa62bc709358 100644 --- a/packages/react-components/react-combobox/src/stories/Dropdown/index.stories.tsx +++ b/packages/react-components/react-combobox/src/stories/Dropdown/index.stories.tsx @@ -8,6 +8,7 @@ import accessibilityMd from './DropdownAccessibility.md'; export { Default } from './DropdownDefault.stories'; export { Appearance } from './DropdownAppearance.stories'; export { Grouped } from './DropdownGrouped.stories'; +export { ComplexOptions } from './DropdownComplexOptions.stories'; export { CustomOptions } from './DropdownCustomOptions.stories'; export { Multiselect } from './DropdownMultiselect.stories'; export { Size } from './DropdownSize.stories'; diff --git a/packages/react-components/react-combobox/src/utils/ComboboxBase.types.ts b/packages/react-components/react-combobox/src/utils/ComboboxBase.types.ts index 24aeee49419d0..4822c7e5e0f5e 100644 --- a/packages/react-components/react-combobox/src/utils/ComboboxBase.types.ts +++ b/packages/react-components/react-combobox/src/utils/ComboboxBase.types.ts @@ -21,7 +21,7 @@ export type ComboboxBaseProps = SelectionProps & { defaultOpen?: boolean; /** - * The default value when the combobox's value is uncontrolled + * The default value displayed in the trigger input or button when the combobox's value is uncontrolled */ defaultValue?: string; diff --git a/packages/react-components/react-combobox/src/utils/OptionCollection.types.ts b/packages/react-components/react-combobox/src/utils/OptionCollection.types.ts index 53cf9c7d095e6..47e2197ef890f 100644 --- a/packages/react-components/react-combobox/src/utils/OptionCollection.types.ts +++ b/packages/react-components/react-combobox/src/utils/OptionCollection.types.ts @@ -5,6 +5,9 @@ export type OptionValue = { /** The `id` attribute of the option. */ id: string; + /** The `text` string for the option. */ + text: string; + /** The value string of the option. */ value: string; }; @@ -22,8 +25,8 @@ export type OptionCollectionState = { /** Returns the option data by key. */ getOptionById(id: string): OptionValue | undefined; - /** Returns an array of options filtered by a value matching function. */ - getOptionsMatchingValue(matcher: (value: string) => boolean): OptionValue[]; + /** Returns an array of options filtered by a value matching function against the option's text string. */ + getOptionsMatchingText(matcher: (value: string) => boolean): OptionValue[]; /** The unordered option data. */ options: OptionValue[]; diff --git a/packages/react-components/react-combobox/src/utils/Selection.types.ts b/packages/react-components/react-combobox/src/utils/Selection.types.ts index 83865bf0e3686..4bcc63cedb67c 100644 --- a/packages/react-components/react-combobox/src/utils/Selection.types.ts +++ b/packages/react-components/react-combobox/src/utils/Selection.types.ts @@ -34,9 +34,13 @@ export type SelectionValue = { /* * Data for the onOptionSelect callback. - * `optionValue` will be undefined if the multiple options are modified at once. + * `optionValue` and `optionText` will be undefined if multiple options are modified at once. */ -export type OptionOnSelectData = { optionValue: string | undefined; selectedOptions: string[] }; +export type OptionOnSelectData = { + optionValue: string | undefined; + optionText: string | undefined; + selectedOptions: string[]; +}; /* Possible event types for onOptionSelect */ export type SelectionEvents = diff --git a/packages/react-components/react-combobox/src/utils/useComboboxBaseState.ts b/packages/react-components/react-combobox/src/utils/useComboboxBaseState.ts index 2e50ea22f4805..85b8d241b8e08 100644 --- a/packages/react-components/react-combobox/src/utils/useComboboxBaseState.ts +++ b/packages/react-components/react-combobox/src/utils/useComboboxBaseState.ts @@ -12,7 +12,7 @@ export const useComboboxBaseState = (props: ComboboxBaseProps) => { const { appearance = 'outline', inlinePopup = false, multiselect, onOpenChange, size = 'medium' } = props; const optionCollection = useOptionCollection(); - const { getOptionAtIndex, getOptionsMatchingValue } = optionCollection; + const { getOptionAtIndex, getOptionsMatchingText } = optionCollection; const [activeOption, setActiveOption] = React.useState(); @@ -70,9 +70,7 @@ export const useComboboxBaseState = (props: ComboboxBaseProps) => { if (open && !activeOption) { // if there is a selection, start at the most recently selected item if (selectedOptions.length > 0) { - const lastSelectedOption = getOptionsMatchingValue( - v => v === selectedOptions[selectedOptions.length - 1], - ).pop(); + const lastSelectedOption = getOptionsMatchingText(v => v === selectedOptions[selectedOptions.length - 1]).pop(); lastSelectedOption && setActiveOption(lastSelectedOption); } // default to starting at the first option diff --git a/packages/react-components/react-combobox/src/utils/useOptionCollection.ts b/packages/react-components/react-combobox/src/utils/useOptionCollection.ts index 93dd27c439c8c..3df8d206827ef 100644 --- a/packages/react-components/react-combobox/src/utils/useOptionCollection.ts +++ b/packages/react-components/react-combobox/src/utils/useOptionCollection.ts @@ -15,8 +15,8 @@ export const useOptionCollection = (): OptionCollectionState => { const item = nodes.current.find(node => node.option.id === id); return item?.option; }; - const getOptionsMatchingValue = (matcher: (value: string) => boolean) => { - return nodes.current.filter(node => matcher(node.option.value)).map(node => node.option); + const getOptionsMatchingText = (matcher: (value: string) => boolean) => { + return nodes.current.filter(node => matcher(node.option.text)).map(node => node.option); }; return { @@ -24,7 +24,7 @@ export const useOptionCollection = (): OptionCollectionState => { getOptionAtIndex, getIndexOfId, getOptionById, - getOptionsMatchingValue, + getOptionsMatchingText, }; }, []); diff --git a/packages/react-components/react-combobox/src/utils/useSelection.ts b/packages/react-components/react-combobox/src/utils/useSelection.ts index 39b3383420031..160291fd1842e 100644 --- a/packages/react-components/react-combobox/src/utils/useSelection.ts +++ b/packages/react-components/react-combobox/src/utils/useSelection.ts @@ -33,12 +33,12 @@ export const useSelection = (props: SelectionProps): SelectionValue => { } setSelectedOptions(newSelection); - onOptionSelect?.(event, { optionValue: option.value, selectedOptions: newSelection }); + onOptionSelect?.(event, { optionValue: option.value, optionText: option.text, selectedOptions: newSelection }); }; const clearSelection = (event: SelectionEvents) => { setSelectedOptions([]); - onOptionSelect?.(event, { optionValue: undefined, selectedOptions: [] }); + onOptionSelect?.(event, { optionValue: undefined, optionText: undefined, selectedOptions: [] }); }; return { clearSelection, selectOption, selectedOptions };