From b6e513a08683095e1ce9e5e1b65ff525d40cacd3 Mon Sep 17 00:00:00 2001 From: Powerplex Date: Wed, 13 Mar 2024 12:08:49 +0100 Subject: [PATCH] feat(combobox): combobox controlled value behaviour --- .../components/combobox/src/Combobox.doc.mdx | 10 ++ .../combobox/src/Combobox.stories.tsx | 80 +++++++++++++ .../combobox/src/ComboboxContext.tsx | 105 ++++++++++++++---- .../combobox/src/ComboboxTrigger.styles.tsx | 4 +- .../src/tests/multipleSelection.test.tsx | 46 ++++++++ .../src/tests/singleSelection.test.tsx | 76 +++++++------ .../useCombobox/multipleSelectionReducer.ts | 2 +- .../src/useCombobox/singleSelectionReducer.ts | 18 ++- 8 files changed, 276 insertions(+), 65 deletions(-) diff --git a/packages/components/combobox/src/Combobox.doc.mdx b/packages/components/combobox/src/Combobox.doc.mdx index 2af42e9c9..2fd3f70b7 100644 --- a/packages/components/combobox/src/Combobox.doc.mdx +++ b/packages/components/combobox/src/Combobox.doc.mdx @@ -91,6 +91,12 @@ AutoSuggest is used as the default behaviour. The user can type anything in the +### Controlled + +Use `value` and `onValueChange` props to control the value of the combobox. + + + ### Custom filtering Disable `autoFilter` to implement your own logic to filter out items depending on the inputValue or some external logic. @@ -170,6 +176,10 @@ This is up to the developer to make it clear to the user which items are selecte +### Controlled + + + ### Read only Use `readOnly` prop to indicate the combobox is only readable. diff --git a/packages/components/combobox/src/Combobox.stories.tsx b/packages/components/combobox/src/Combobox.stories.tsx index 297301875..f060d3008 100644 --- a/packages/components/combobox/src/Combobox.stories.tsx +++ b/packages/components/combobox/src/Combobox.stories.tsx @@ -1,7 +1,9 @@ /* eslint-disable max-lines */ // import { Button } from '@spark-ui/button' +import { Checkbox, CheckboxGroup } from '@spark-ui/checkbox' import { FormField } from '@spark-ui/form-field' import { PenOutline } from '@spark-ui/icons/dist/icons/PenOutline' +import { RadioGroup } from '@spark-ui/radio-group' import { Tag } from '@spark-ui/tag' import { VisuallyHidden } from '@spark-ui/visually-hidden' import { Meta, StoryFn } from '@storybook/react' @@ -45,6 +47,46 @@ export const Default: StoryFn = _args => { ) } +export const Controlled: StoryFn = _args => { + const [value, setValue] = useState('book-2') + + return ( +
+ + + + + + + + + + + + + No results found + To Kill a Mockingbird + War and Peace + The Idiot + A Picture of Dorian Gray + 1984 + Pride and Prejudice + + + + + + To Kill a Mockingbird + War and Peace + The Idiot + A Picture of Dorian Gray + 1984 + Pride and Prejudice + +
+ ) +} + export const CustomItem: StoryFn = _args => { return (
@@ -395,6 +437,44 @@ export const MultipleSelection: StoryFn = _args => { ) } +export const MultipleSelectionControlled: StoryFn = _args => { + const [value, setValue] = useState(['book-2']) + + return ( +
+ + To Kill a Mockingbird + War and Peace + The Idiot + A Picture of Dorian Gray + 1984 + Pride and Prejudice + + + + + + + + + + + + + No results found + To Kill a Mockingbird + War and Peace + The Idiot + A Picture of Dorian Gray + 1984 + Pride and Prejudice + + + +
+ ) +} + export const MultipleSelectionDisabled: StoryFn = _args => { return (
diff --git a/packages/components/combobox/src/ComboboxContext.tsx b/packages/components/combobox/src/ComboboxContext.tsx index ce4c925fc..3c98883d9 100644 --- a/packages/components/combobox/src/ComboboxContext.tsx +++ b/packages/components/combobox/src/ComboboxContext.tsx @@ -1,6 +1,7 @@ import { useId } from '@radix-ui/react-id' import { useFormFieldControl } from '@spark-ui/form-field' import { Popover } from '@spark-ui/popover' +import { useCombinedState } from '@spark-ui/use-combined-state' import { useCombobox, useMultipleSelection } from 'downshift' import { createContext, @@ -83,11 +84,11 @@ interface ComboboxPropsSingle { /** * The controlled value of the select. Should be used in conjunction with `onValueChange`. */ - value?: string + value?: string | null /** * Event handler called when the value changes. */ - onValueChange?: (value: string) => void + onValueChange?: (value: string | undefined) => void } interface ComboboxPropsMultiple { @@ -130,26 +131,75 @@ export const ComboboxProvider = ({ defaultValue, disabled: disabledProp = false, multiple = false, - // onValueChange, readOnly: readOnlyProp = false, state: stateProp, + // controlled behaviour, + value: controlledValue, + onValueChange, }: ComboboxContextProps) => { + const isMounted = useRef(false) + // Input state const [inputValue, setInputValue] = useState('') const triggerAreaRef = useRef(null) const innerInputRef = useRef(null) + const [comboboxValue] = useCombinedState(controlledValue, defaultValue) + // Items state const [itemsMap, setItemsMap] = useState(getItemsFromChildren(children)) const [filteredItemsMap, setFilteredItems] = useState( autoFilter ? getFilteredItemsMap(itemsMap, inputValue) : itemsMap ) + + const [selectedItem, setSelectedItem] = useState( + itemsMap.get(comboboxValue as string) || null + ) + const [selectedItems, setSelectedItems] = useState( - defaultValue - ? [...itemsMap.values()].filter(item => (defaultValue as string[]).includes(item.value)) + comboboxValue + ? [...itemsMap.values()].filter(item => (comboboxValue as string[]).includes(item.value)) : [] ) + const onInternalSelectedItemChange = (item: ComboboxItem | null) => { + setSelectedItem(item) + setTimeout(() => { + onValueChange?.(item?.value as string & string[]) + }, 0) + } + + const onInternalSelectedItemsChange = (items: ComboboxItem[]) => { + setSelectedItems(items) + setTimeout(() => { + onValueChange?.(items.map(i => i.value) as string & string[]) + }, 0) + } + + // Sync internal state with controlled value + useEffect(() => { + if (!isMounted.current) { + isMounted.current = true + + return + } + + if (multiple) { + const newSelectedItems = (comboboxValue as string[]).reduce( + (accum: ComboboxItem[], value) => { + const match = itemsMap.get(value) + + return match ? [...accum, match] : accum + }, + [] + ) + + setSelectedItems(comboboxValue ? newSelectedItems : []) + } else { + setSelectedItem(itemsMap.get(comboboxValue as string) || null) + } + }, [multiple ? JSON.stringify(comboboxValue) : comboboxValue]) + // Form field state const field = useFormFieldControl() const id = useId(field.id) @@ -170,25 +220,29 @@ export const ComboboxProvider = ({ const multiselect = useMultipleSelection({ selectedItems, stateReducer: (state, { type, changes }) => { - switch (type) { - case useMultipleSelection.stateChangeTypes.SelectedItemKeyDownDelete: - setSelectedItems(changes.selectedItems || []) + const types = useMultipleSelection.stateChangeTypes - return { - ...changes, - activeIndex: - state?.activeIndex === changes.selectedItems?.length ? -1 : state.activeIndex, + switch (type) { + case types.SelectedItemKeyDownBackspace: + case types.SelectedItemKeyDownDelete: { + onInternalSelectedItemsChange(changes.selectedItems || []) + + let activeIndex + + if (type === types.SelectedItemKeyDownDelete) { + const isLastItem = state?.activeIndex === changes.selectedItems?.length + activeIndex = isLastItem ? -1 : state.activeIndex + } else { + const hasItemBefore = (changes?.activeIndex || 0) - 1 >= 0 + activeIndex = hasItemBefore ? state.activeIndex - 1 : changes?.activeIndex } - case useMultipleSelection.stateChangeTypes.SelectedItemKeyDownBackspace: - setSelectedItems(changes.selectedItems || []) return { ...changes, - ...((changes?.activeIndex || 0) - 1 >= 0 && { - activeIndex: state.activeIndex - 1, - }), + activeIndex, } - case useMultipleSelection.stateChangeTypes.SelectedItemClick: + } + case types.SelectedItemClick: if (innerInputRef.current) { innerInputRef.current.focus() } @@ -197,12 +251,12 @@ export const ComboboxProvider = ({ ...changes, activeIndex: -1, // the focus will remain on the input } - case useMultipleSelection.stateChangeTypes.FunctionRemoveSelectedItem: + case types.FunctionRemoveSelectedItem: return { ...changes, activeIndex: -1, // the focus will remain on the input } - case useMultipleSelection.stateChangeTypes.DropdownKeyDownNavigationPrevious: + case types.DropdownKeyDownNavigationPrevious: downshift.closeMenu() return changes @@ -221,11 +275,12 @@ export const ComboboxProvider = ({ */ const downshift = useCombobox({ items: filteredItems, + selectedItem, + // initialSelectedItem: comboboxValue ? itemsMap.get(comboboxValue as string) : undefined, id, labelId, inputValue, initialIsOpen: defaultOpen, - initialSelectedItem: defaultValue ? itemsMap.get(defaultValue as string) : undefined, ...(multiple && { selectedItem: undefined }), itemToString: item => { return (item as ComboboxItem)?.text @@ -252,11 +307,12 @@ export const ComboboxProvider = ({ multiselect, selectedItems, allowCustomValue, - setSelectedItems, + setSelectedItems: onInternalSelectedItemsChange, triggerAreaRef, }) : singleSelectionReducer({ allowCustomValue, + setSelectedItem: onInternalSelectedItemChange, filteredItems: [...filteredItemsMap.values()], }), }) @@ -321,8 +377,9 @@ export const ComboboxProvider = ({ // Downshift state ...downshift, ...multiselect, - setInputValue, // todo -override downshift logic (merge) - setSelectedItems, // todo -override downshift logic (merge) + setInputValue, + selectItem: onInternalSelectedItemChange, + setSelectedItems: onInternalSelectedItemsChange, }} > {children} diff --git a/packages/components/combobox/src/ComboboxTrigger.styles.tsx b/packages/components/combobox/src/ComboboxTrigger.styles.tsx index c7f78e045..27091f421 100644 --- a/packages/components/combobox/src/ComboboxTrigger.styles.tsx +++ b/packages/components/combobox/src/ComboboxTrigger.styles.tsx @@ -2,8 +2,8 @@ import { cva } from 'class-variance-authority' export const styles = cva( [ - 'flex w-full items-start gap-md', - 'min-h-sz-44 p-md rounded-lg px-lg', + 'flex items-start gap-md', + 'min-h-sz-44 h-fit p-md rounded-lg px-lg', // outline styles 'ring-1 outline-none ring-inset focus-within:ring-2', ], diff --git a/packages/components/combobox/src/tests/multipleSelection.test.tsx b/packages/components/combobox/src/tests/multipleSelection.test.tsx index a7252531a..73b2b0c93 100644 --- a/packages/components/combobox/src/tests/multipleSelection.test.tsx +++ b/packages/components/combobox/src/tests/multipleSelection.test.tsx @@ -1,5 +1,6 @@ import { render, screen } from '@testing-library/react' import userEvent from '@testing-library/user-event' +import { useState } from 'react' import { describe, expect, it } from 'vitest' import { Combobox } from '..' @@ -328,6 +329,51 @@ describe('Combobox', () => { expect(getInput('Book')).toHaveFocus() }) + it('should update value in controlled mode', async () => { + const user = userEvent.setup() + + // Given we control value by outside state and selected value + const ControlledImplementation = () => { + const [value, setValue] = useState(['book-1']) + + return ( + + + + + + + + War and Peace + 1984 + Pride and Prejudice + + + + ) + } + + render() + + expect(getItem('War and Peace')).toHaveAttribute('aria-selected', 'true') + expect(getSelectedItem('War and Peace')).toBeInTheDocument() + + expect(screen.getByDisplayValue('')).toBeInTheDocument() + + // when the user select another item + await user.click(getInput('Book')) + await user.click(getItem('Pride and Prejudice')) + + // Then the selected values have been updated and input remain empty + expect(screen.getByDisplayValue('')).toBeInTheDocument() + + expect(getSelectedItem('War and Peace')).toBeInTheDocument() + expect(getItem('War and Peace')).toHaveAttribute('aria-selected', 'true') + + expect(getSelectedItem('Pride and Prejudice')).toBeInTheDocument() + expect(getItem('Pride and Prejudice')).toHaveAttribute('aria-selected', 'true') + }) + describe('blur behaviour', () => { it('should not clear input value when custom value is allowed', async () => { const user = userEvent.setup() diff --git a/packages/components/combobox/src/tests/singleSelection.test.tsx b/packages/components/combobox/src/tests/singleSelection.test.tsx index b3c4177d6..d92b9a4bf 100644 --- a/packages/components/combobox/src/tests/singleSelection.test.tsx +++ b/packages/components/combobox/src/tests/singleSelection.test.tsx @@ -1,5 +1,6 @@ import { render, screen } from '@testing-library/react' import userEvent from '@testing-library/user-event' +import { useState } from 'react' // import { useState } from 'react' import { describe, expect, it, vitest } from 'vitest' @@ -97,43 +98,44 @@ describe('Combobox', () => { expect(screen.getByLabelText('selected')).toBeVisible() }) - // it('should control value', async () => { - // const user = userEvent.setup() - - // // Given we control value by outside state and selected value - // const ControlledImplementation = () => { - // const [value, setValue] = useState('book-1') - // const [inputValue, setInputValue] = useState('') - - // return ( - // - // - // - // - // - // - // War and Peace - // 1984 - // Pride and Prejudice - // - // - // - // ) - // } - - // render() - - // expect(getItem('War and Peace')).toHaveAttribute('aria-selected', 'true') - - // expect(screen.getByDisplayValue('')).toBeInTheDocument() - - // // when the user select another item - // await user.click(getInput('Book')) - // await user.click(getItem('Pride and Prejudice')) - - // // Then the selected value has been updated - // expect(screen.getByDisplayValue('Pride and Prejudice')).toBeInTheDocument() - // }) + it('should update value in controlled mode', async () => { + const user = userEvent.setup() + + // Given we control value by outside state and selected value + const ControlledImplementation = () => { + const [value, setValue] = useState('book-1') + + return ( + + + + + + + War and Peace + 1984 + Pride and Prejudice + + + + ) + } + + render() + + expect(getItem('War and Peace')).toHaveAttribute('aria-selected', 'true') + + expect(screen.getByDisplayValue('War and Peace')).toBeInTheDocument() + + // when the user select another item + await user.click(getInput('Book')) + await user.click(getItem('Pride and Prejudice')) + + // Then the selected value has been updated + expect(screen.getByDisplayValue('Pride and Prejudice')).toBeInTheDocument() + expect(getItem('War and Peace')).toHaveAttribute('aria-selected', 'false') + expect(getItem('Pride and Prejudice')).toHaveAttribute('aria-selected', 'true') + }) it('should select item using autoFilter (keyboard)', async () => { const user = userEvent.setup() diff --git a/packages/components/combobox/src/useCombobox/multipleSelectionReducer.ts b/packages/components/combobox/src/useCombobox/multipleSelectionReducer.ts index 528c3191e..d9c7b147c 100644 --- a/packages/components/combobox/src/useCombobox/multipleSelectionReducer.ts +++ b/packages/components/combobox/src/useCombobox/multipleSelectionReducer.ts @@ -6,7 +6,7 @@ interface Props { allowCustomValue?: boolean selectedItems: ComboboxItem[] multiselect: UseMultipleSelectionReturnValue - setSelectedItems: (value: React.SetStateAction) => void + setSelectedItems: (items: ComboboxItem[]) => void triggerAreaRef: React.RefObject } diff --git a/packages/components/combobox/src/useCombobox/singleSelectionReducer.ts b/packages/components/combobox/src/useCombobox/singleSelectionReducer.ts index d48f0a1d0..9aec4c33f 100644 --- a/packages/components/combobox/src/useCombobox/singleSelectionReducer.ts +++ b/packages/components/combobox/src/useCombobox/singleSelectionReducer.ts @@ -5,15 +5,27 @@ import { ComboboxItem } from '../types' interface Props { allowCustomValue?: boolean filteredItems: ComboboxItem[] + setSelectedItem: (value: ComboboxItem | null) => void } -export const singleSelectionReducer = ({ filteredItems, allowCustomValue = false }: Props) => { +export const singleSelectionReducer = ({ + filteredItems, + allowCustomValue = false, + setSelectedItem, +}: Props) => { const reducer: UseComboboxProps['stateReducer'] = (state, { changes, type }) => { const exactMatch = filteredItems.find( item => item.text.toLowerCase() === state.inputValue.toLowerCase() ) switch (type) { + case useCombobox.stateChangeTypes.ItemClick: + case useCombobox.stateChangeTypes.InputKeyDownEnter: + if (changes.selectedItem) { + setSelectedItem(changes.selectedItem) + } + + return changes case useCombobox.stateChangeTypes.InputClick: return { ...changes, isOpen: true } case useCombobox.stateChangeTypes.ToggleButtonClick: @@ -21,10 +33,14 @@ export const singleSelectionReducer = ({ filteredItems, allowCustomValue = false if (allowCustomValue) return changes if (state.inputValue === '') { + setSelectedItem(null) + return { ...changes, selectedItem: null } } if (exactMatch) { + setSelectedItem(exactMatch) + return { ...changes, selectedItem: exactMatch, inputValue: exactMatch.text } }