fix(Autocomplete): Behave as controlled component

Previously, the Autocomplete `value` prop was only setting the initial value of the component, which was unexpected for a controlled component. In particular, this resulted in unexpected behaviour when the value or children changed externally. This makes the component controlled when `value` is set so it behaves as expected in these scenarios. `initialValue` was also added for uncontrolled scenarios.
Royal-Navy · Sep 20, 2022 · 4c497ec · 4c497ec
1 parent 8bcbb27
commit 4c497ec
Show file tree

Hide file tree

Showing 5 changed files with 130 additions and 14 deletions.
diff --git a/packages/react-component-library/src/components/Autocomplete/Autocomplete.stories.tsx b/packages/react-component-library/src/components/Autocomplete/Autocomplete.stories.tsx
@@ -1,4 +1,4 @@
-import React from 'react'
+import React, { useEffect } from 'react'
 import {
   IconAgriculture,
   IconAnchor,
@@ -71,7 +71,7 @@ Disabled.args = {
 export const NoClearButton = Template.bind({})
 NoClearButton.args = {
   hideClearButton: true,
-  value: 'two',
+  initialValue: 'two',
 }
 
 export const Open = Template.bind({})
@@ -87,7 +87,7 @@ WithError.args = {
 
 export const WithValue = Template.bind({})
 WithValue.args = {
-  value: 'two',
+  initialValue: 'two',
 }
 
 export const WithIconsAndBadges = TemplateWithIconsAndBadges.bind({})
diff --git a/packages/react-component-library/src/components/Autocomplete/Autocomplete.test.tsx b/packages/react-component-library/src/components/Autocomplete/Autocomplete.test.tsx
@@ -204,15 +204,93 @@ describe('Autocomplete', () => {
     })
   })
 
+  describe('when the component is controlled with "One" selected', () => {
+    const ControlledAutocomplete = () => {
+      const [value, setValue] = React.useState<string | null>('one')
+
+      return (
+        <Autocomplete
+          id="autocomplete-id"
+          label="Label"
+          onChange={(newValue) => setValue(newValue)}
+          value={value}
+        >
+          <AutocompleteOption value="one">One</AutocompleteOption>
+          <AutocompleteOption value="two">Two</AutocompleteOption>
+        </Autocomplete>
+      )
+    }
+
+    beforeEach(() => {
+      wrapper = render(<ControlledAutocomplete />)
+    })
+
+    it('has the value "One"', () => {
+      expect(wrapper.getByTestId('select-input')).toHaveValue('One')
+    })
+
+    describe('when the menu is opened and the second item clicked', () => {
+      beforeEach(async () => {
+        await userEvent.click(wrapper.getByTestId('select-arrow-button'))
+        return userEvent.click(wrapper.getByText('Two'))
+      })
+
+      it('has the value "Two"', () => {
+        expect(wrapper.getByTestId('select-input')).toHaveValue('Two')
+      })
+    })
+  })
+
+  describe('when the component is uncontrolled with "One" selected', () => {
+    beforeEach(() => {
+      wrapper = render(
+        <Autocomplete
+          id="autocomplete-id"
+          label="Label"
+          onChange={jest.fn()}
+          initialValue="one"
+        >
+          <AutocompleteOption value="one">One</AutocompleteOption>
+          <AutocompleteOption value="two">Two</AutocompleteOption>
+        </Autocomplete>
+      )
+    })
+
+    it('has the value "One"', () => {
+      expect(wrapper.getByTestId('select-input')).toHaveValue('One')
+    })
+
+    describe('when the menu is opened and the second item clicked', () => {
+      beforeEach(async () => {
+        await userEvent.click(wrapper.getByTestId('select-arrow-button'))
+        return userEvent.click(wrapper.getByText('Two'))
+      })
+
+      it('has the value "Two"', () => {
+        expect(wrapper.getByTestId('select-input')).toHaveValue('Two')
+      })
+    })
+  })
+
   describe('when options are added after the initial render', () => {
     beforeEach(() => {
       wrapper = render(
-        <Autocomplete id="autocomplete-id" label="Label" onChange={jest.fn()}>
+        <Autocomplete
+          id="autocomplete-id"
+          label="Label"
+          onChange={jest.fn()}
+          value="one"
+        >
           <>{}</>
         </Autocomplete>
       )
       wrapper.rerender(
-        <Autocomplete id="autocomplete-id" label="Label" onChange={jest.fn()}>
+        <Autocomplete
+          id="autocomplete-id"
+          label="Label"
+          onChange={jest.fn()}
+          value="one"
+        >
           <AutocompleteOption value="one">One</AutocompleteOption>
           <AutocompleteOption value="two">Two</AutocompleteOption>
         </Autocomplete>
@@ -227,20 +305,34 @@ describe('Autocomplete', () => {
       expect(options[1]).toHaveTextContent('Two')
       expect(options).toHaveLength(2)
     })
+
+    it('has the correct value', () => {
+      expect(wrapper.getByTestId('select-input')).toHaveValue('One')
+    })
   })
 
   describe('when options are added while the menu is open', () => {
     beforeEach(async () => {
       wrapper = render(
-        <Autocomplete id="autocomplete-id" label="Label" onChange={jest.fn()}>
+        <Autocomplete
+          id="autocomplete-id"
+          label="Label"
+          onChange={jest.fn()}
+          value="one"
+        >
           <>{}</>
         </Autocomplete>
       )
 
       await userEvent.click(wrapper.getByTestId('select-arrow-button'))
 
       wrapper.rerender(
-        <Autocomplete id="autocomplete-id" label="Label" onChange={jest.fn()}>
+        <Autocomplete
+          id="autocomplete-id"
+          label="Label"
+          onChange={jest.fn()}
+          value="one"
+        >
           <AutocompleteOption value="one">One</AutocompleteOption>
           <AutocompleteOption value="two">Two</AutocompleteOption>
         </Autocomplete>
@@ -254,6 +346,10 @@ describe('Autocomplete', () => {
       expect(options[1]).toHaveTextContent('Two')
       expect(options).toHaveLength(2)
     })
+
+    it('has the correct value', () => {
+      expect(wrapper.getByTestId('select-input')).toHaveValue('One')
+    })
   })
 
   describe('when the default `id` is used and the arrow button is clicked', () => {

diff --git a/packages/react-component-library/src/components/Autocomplete/Autocomplete.tsx b/packages/react-component-library/src/components/Autocomplete/Autocomplete.tsx
@@ -5,7 +5,7 @@ import { useCombobox } from 'downshift'
 import { itemToString, SelectBaseProps, SelectLayout } from '../SelectBase'
 import { NoResults } from './NoResults'
 import { useHighlightedIndex } from './hooks/useHighlightedIndex'
-import { useAutocomplete } from './hooks/useAutocomplete'
+import { ItemsMap, useAutocomplete } from './hooks/useAutocomplete'
 import { useMenuVisibility } from '../SelectBase/hooks/useMenuVisibility'
 import { useExternalId } from '../../hooks/useExternalId'
 import { useToggleButton } from './hooks/useToggleButton'
@@ -20,16 +20,25 @@ export interface AutocompleteProps extends SelectBaseProps {
    * Called when the input loses focus.
    */
   onBlur?: (event: React.FocusEvent) => void
+  /**
+   * The initially selected item when the component is uncontrolled.
+   */
+  initialValue?: string | null
+}
+
+function getSelectedItem(value: string | null | undefined, itemsMap: ItemsMap) {
+  return !isNil(value) && value in itemsMap ? value : null
 }
 
 export const Autocomplete: React.FC<AutocompleteProps> = ({
   children,
   id: externalId,
   initialIsOpen,
+  initialValue,
   isInvalid = false,
   onBlur,
   onChange,
-  value = null,
+  value,
   ...rest
 }) => {
   const {
@@ -50,6 +59,8 @@ export const Autocomplete: React.FC<AutocompleteProps> = ({
   } = useToggleButton(inputRef)
   const id = useExternalId('autocomplete', externalId)
 
+  const isControlled = value !== undefined
+
   const {
     getComboboxProps,
     getInputProps,
@@ -73,7 +84,6 @@ export const Autocomplete: React.FC<AutocompleteProps> = ({
         : '',
     onInputValueChange,
     onIsOpenChange,
-    initialSelectedItem: !isNil(value) && value in itemsMap ? value : null,
     onSelectedItemChange: (changes) => {
       onSelectedItemChange(changes)
 
@@ -85,6 +95,12 @@ export const Autocomplete: React.FC<AutocompleteProps> = ({
 
       focusToggleButton()
     },
+    ...{
+      [isControlled ? 'selectedItem' : 'initialSelectedItem']: getSelectedItem(
+        isControlled ? value : initialValue,
+        itemsMap
+      ),
+    },
   })
 
   const { onInputBlurHandler, onInputTabKeyHandler } = useHighlightedIndex(

diff --git a/packages/react-component-library/src/components/Autocomplete/hooks/useAutocomplete.ts b/packages/react-component-library/src/components/Autocomplete/hooks/useAutocomplete.ts
@@ -1,13 +1,13 @@
 import React, { useRef, useState } from 'react'
-import { UseComboboxStateChange } from 'downshift'
+import { useCombobox, UseComboboxStateChange } from 'downshift'
 
 import { findIndexOfInputValue } from '../helpers'
 import {
   SelectBaseOptionAsStringProps,
   SelectChildWithStringType,
 } from '../../SelectBase'
 
-type ItemsMap = {
+export type ItemsMap = {
   [id: string]: React.ReactElement<SelectBaseOptionAsStringProps>
 }
 
@@ -47,8 +47,12 @@ export function useAutocomplete(children: SelectChildWithStringType[]): {
   function onInputValueChange({
     inputValue,
     isOpen,
+    type,
   }: UseComboboxStateChange<string>) {
-    if (isOpen || !inputValue) {
+    const isControlledValueUpdate =
+      type === useCombobox.stateChangeTypes.ControlledPropUpdatedSelectedItem
+
+    if (!inputValue || (isOpen && !isControlledValueUpdate)) {
       setFilterValue(inputValue || '')
     }
 

diff --git a/packages/react-component-library/src/components/SelectBase/SelectBaseProps.ts b/packages/react-component-library/src/components/SelectBase/SelectBaseProps.ts
@@ -36,7 +36,7 @@ export interface SelectBaseProps extends ComponentWithClass {
    */
   onChange?: (value: string | null) => void
   /**
-   * Optional HTML `value` attribute to apply to the component.
+   * The selected value when the component is controlled.
    */
   value?: string | null
 }