Merge pull request #1962 from adevinta/combobox-controlled-value

feat(combobox): combobox controlled value behaviour

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
 
 <Canvas of={stories.Default} />
 
+### Controlled
+
+Use `value` and `onValueChange` props to control the value of the combobox.
+
+<Canvas of={stories.Controlled} />
+
 ### 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
 
 <Canvas of={stories.MultipleSelection} />
 
+### Controlled
+
+<Canvas of={stories.MultipleSelectionControlled} />
+
 ### Read only
 
 Use `readOnly` prop to indicate the combobox is only readable.

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<string | undefined>('book-2')
+
+  return (
+    <div className="flex flex-wrap gap-lg pb-[300px]">
+      <Combobox autoFilter={false} value={value} onValueChange={setValue}>
+        <Combobox.Trigger className="grow">
+          <Combobox.LeadingIcon>
+            <PenOutline />
+          </Combobox.LeadingIcon>
+          <Combobox.Input aria-label="Book" placeholder="Pick a book" />
+          <Combobox.ClearButton aria-label="Clear input" />
+          <Combobox.Disclosure openedLabel="Close popup" closedLabel="Open popup" />
+        </Combobox.Trigger>
+
+        <Combobox.Popover>
+          <Combobox.Items>
+            <Combobox.Empty>No results found</Combobox.Empty>
+            <Combobox.Item value="book-1">To Kill a Mockingbird</Combobox.Item>
+            <Combobox.Item value="book-2">War and Peace</Combobox.Item>
+            <Combobox.Item value="book-3">The Idiot</Combobox.Item>
+            <Combobox.Item value="book-4">A Picture of Dorian Gray</Combobox.Item>
+            <Combobox.Item value="book-5">1984</Combobox.Item>
+            <Combobox.Item value="book-6">Pride and Prejudice</Combobox.Item>
+          </Combobox.Items>
+        </Combobox.Popover>
+      </Combobox>
+
+      <RadioGroup value={value || ''} onValueChange={setValue} className="grow">
+        <RadioGroup.Radio value="book-1">To Kill a Mockingbird</RadioGroup.Radio>
+        <RadioGroup.Radio value="book-2">War and Peace</RadioGroup.Radio>
+        <RadioGroup.Radio value="book-3">The Idiot</RadioGroup.Radio>
+        <RadioGroup.Radio value="book-4">A Picture of Dorian Gray</RadioGroup.Radio>
+        <RadioGroup.Radio value="book-5">1984</RadioGroup.Radio>
+        <RadioGroup.Radio value="book-6">Pride and Prejudice</RadioGroup.Radio>
+      </RadioGroup>
+    </div>
+  )
+}
+
 export const CustomItem: StoryFn = _args => {
   return (
     <div className="pb-[300px]">
@@ -395,6 +437,44 @@ export const MultipleSelection: StoryFn = _args => {
   )
 }
 
+export const MultipleSelectionControlled: StoryFn = _args => {
+  const [value, setValue] = useState<string[]>(['book-2'])
+
+  return (
+    <div className="flex flex-col gap-lg pb-[300px]">
+      <CheckboxGroup value={value} onCheckedChange={setValue} className="grow">
+        <Checkbox value="book-1">To Kill a Mockingbird</Checkbox>
+        <Checkbox value="book-2">War and Peace</Checkbox>
+        <Checkbox value="book-3">The Idiot</Checkbox>
+        <Checkbox value="book-4">A Picture of Dorian Gray</Checkbox>
+        <Checkbox value="book-5">1984</Checkbox>
+        <Checkbox value="book-6">Pride and Prejudice</Checkbox>
+      </CheckboxGroup>
+
+      <Combobox multiple autoFilter={false} value={value} onValueChange={setValue}>
+        <Combobox.Trigger>
+          <Combobox.SelectedItems />
+          <Combobox.Input aria-label="Book" placeholder="Pick a book" />
+          <Combobox.ClearButton aria-label="Clear input" />
+          <Combobox.Disclosure openedLabel="Close popup" closedLabel="Open popup" />
+        </Combobox.Trigger>
+
+        <Combobox.Popover>
+          <Combobox.Items>
+            <Combobox.Empty>No results found</Combobox.Empty>
+            <Combobox.Item value="book-1">To Kill a Mockingbird</Combobox.Item>
+            <Combobox.Item value="book-2">War and Peace</Combobox.Item>
+            <Combobox.Item value="book-3">The Idiot</Combobox.Item>
+            <Combobox.Item value="book-4">A Picture of Dorian Gray</Combobox.Item>
+            <Combobox.Item value="book-5">1984</Combobox.Item>
+            <Combobox.Item value="book-6">Pride and Prejudice</Combobox.Item>
+          </Combobox.Items>
+        </Combobox.Popover>
+      </Combobox>
+    </div>
+  )
+}
+
 export const MultipleSelectionDisabled: StoryFn = _args => {
   return (
     <div className="pb-[300px]">

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<string | undefined>('')
   const triggerAreaRef = useRef<HTMLDivElement>(null)
   const innerInputRef = useRef<HTMLInputElement>(null)
 
+  const [comboboxValue] = useCombinedState(controlledValue, defaultValue)
+
   // Items state
   const [itemsMap, setItemsMap] = useState<ItemsMap>(getItemsFromChildren(children))
   const [filteredItemsMap, setFilteredItems] = useState(
     autoFilter ? getFilteredItemsMap(itemsMap, inputValue) : itemsMap
   )
+
+  const [selectedItem, setSelectedItem] = useState<ComboboxItem | null>(
+    itemsMap.get(comboboxValue as string) || null
+  )
+
   const [selectedItems, setSelectedItems] = useState<ComboboxItem[]>(
-    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<ComboboxItem>({
     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<ComboboxItem>({
     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,
       }}
     >
       <WrapperComponent {...wrapperProps}>{children}</WrapperComponent>

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',
   ],

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 (
+          <Combobox multiple value={value} onValueChange={setValue} autoFilter={false}>
+            <Combobox.Trigger>
+              <Combobox.SelectedItems />
+              <Combobox.Input aria-label="Book" />
+            </Combobox.Trigger>
+            <Combobox.Popover>
+              <Combobox.Items>
+                <Combobox.Item value="book-1">War and Peace</Combobox.Item>
+                <Combobox.Item value="book-2">1984</Combobox.Item>
+                <Combobox.Item value="book-3">Pride and Prejudice</Combobox.Item>
+              </Combobox.Items>
+            </Combobox.Popover>
+          </Combobox>
+        )
+      }
+
+      render(<ControlledImplementation />)
+
+      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()