Autocomplete: Only open menu on click (#4771)

* Only open menu on click instead of just focus * Update tests * Add changeset * Fix typo * Set `openOnFocus` to `true` * Add test, move `onFocus` function * Update docs * Adjust changeset * Remove `useCallback` * Add deprecated notice
primer · Jul 29, 2024 · 92e05f7 · 92e05f7
1 parent 4ebacb7
commit 92e05f7
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 34 deletions.
diff --git a/.changeset/four-shoes-yell.md b/.changeset/four-shoes-yell.md
@@ -0,0 +1,5 @@
+---
+'@primer/react': minor
+---
+
+Set `openOnFocus` default to `false`, making the menu closed initially rather than opening on focus of input
diff --git a/packages/react/src/Autocomplete/Autocomplete.docs.json b/packages/react/src/Autocomplete/Autocomplete.docs.json
@@ -23,8 +23,9 @@
         {
           "name": "openOnFocus",
           "type": "boolean",
-          "defaultValue": "true",
-          "description": "Whether the associated autocomplete menu should open on an input focus event"
+          "defaultValue": "false",
+          "description": "Whether the associated autocomplete menu should open on an input focus event",
+          "deprecated": true
         }
       ],
       "passthrough": {

diff --git a/packages/react/src/Autocomplete/AutocompleteInput.tsx b/packages/react/src/Autocomplete/AutocompleteInput.tsx
@@ -10,8 +10,11 @@ import useSafeTimeout from '../hooks/useSafeTimeout'
 type InternalAutocompleteInputProps = {
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
   as?: React.ComponentType<React.PropsWithChildren<any>>
-  // When false, the autocomplete menu will not render either on mouse click or
-  // keyboard focus.
+
+  /**
+   * @deprecated `openOnFocus` is deprecated and will be removed in v38.
+   * When `true`, autocomplete menu will show on focus or click.
+   */
   openOnFocus?: boolean
 }
 
@@ -28,7 +31,7 @@ const AutocompleteInput = React.forwardRef(
       onKeyUp,
       onKeyPress,
       value,
-      openOnFocus = true,
+      openOnFocus = false,
       ...props
     },
     forwardedRef,
@@ -52,15 +55,12 @@ const AutocompleteInput = React.forwardRef(
     const [highlightRemainingText, setHighlightRemainingText] = useState<boolean>(true)
     const {safeSetTimeout} = useSafeTimeout()
 
-    const handleInputFocus: FocusEventHandler<HTMLInputElement> = useCallback(
-      event => {
-        if (openOnFocus) {
-          onFocus?.(event)
-          setShowMenu(true)
-        }
-      },
-      [onFocus, setShowMenu, openOnFocus],
-    )
+    const handleInputFocus: FocusEventHandler<HTMLInputElement> = event => {
+      onFocus?.(event)
+      if (openOnFocus) {
+        setShowMenu(true)
+      }
+    }
 
     const handleInputBlur: FocusEventHandler<HTMLInputElement> = useCallback(
       event => {
@@ -78,16 +78,13 @@ const AutocompleteInput = React.forwardRef(
       [onBlur, setShowMenu, inputRef, safeSetTimeout],
     )
 
-    const handleInputChange: ChangeEventHandler<HTMLInputElement> = useCallback(
-      event => {
-        onChange && onChange(event)
-        setInputValue(event.currentTarget.value)
-        if (!showMenu) {
-          setShowMenu(true)
-        }
-      },
-      [onChange, setInputValue, setShowMenu, showMenu],
-    )
+    const handleInputChange: ChangeEventHandler<HTMLInputElement> = event => {
+      onChange && onChange(event)
+      setInputValue(event.currentTarget.value)
+      if (!showMenu) {
+        setShowMenu(true)
+      }
+    }
 
     const handleInputKeyDown: KeyboardEventHandler<HTMLInputElement> = useCallback(
       event => {
@@ -122,7 +119,6 @@ const AutocompleteInput = React.forwardRef(
     const onInputKeyPress: KeyboardEventHandler<HTMLInputElement> = useCallback(
       event => {
         onKeyPress && onKeyPress(event)
-
         if (showMenu && event.key === 'Enter' && activeDescendantRef.current) {
           event.preventDefault()
           event.nativeEvent.stopImmediatePropagation()

diff --git a/packages/react/src/__tests__/Autocomplete.test.tsx b/packages/react/src/__tests__/Autocomplete.test.tsx
@@ -1,4 +1,4 @@
-import {render as HTMLRender, fireEvent, waitFor, screen} from '@testing-library/react'
+import {render as HTMLRender, fireEvent, screen, waitFor} from '@testing-library/react'
 import userEvent from '@testing-library/user-event'
 import React from 'react'
 import type {AutocompleteInputProps} from '../Autocomplete'
@@ -130,14 +130,15 @@ describe('Autocomplete', () => {
       expect(onKeyPressMock).toHaveBeenCalled()
     })
 
-    it('opens the menu when the input is focused', () => {
+    it('opens the menu when the input is focused and arrow key is pressed', () => {
       const {getByLabelText} = HTMLRender(
         <LabelledAutocomplete menuProps={{items: [], selectedItemIds: [], ['aria-labelledby']: 'autocompleteLabel'}} />,
       )
       const inputNode = getByLabelText(AUTOCOMPLETE_LABEL)
 
       expect(inputNode.getAttribute('aria-expanded')).not.toBe('true')
-      fireEvent.focus(inputNode)
+      fireEvent.click(inputNode)
+      fireEvent.keyDown(inputNode, {key: 'ArrowDown'})
       expect(inputNode.getAttribute('aria-expanded')).toBe('true')
     })
 
@@ -148,13 +149,14 @@ describe('Autocomplete', () => {
       const inputNode = getByLabelText(AUTOCOMPLETE_LABEL)
 
       expect(inputNode.getAttribute('aria-expanded')).not.toBe('true')
-      fireEvent.focus(inputNode)
+      fireEvent.click(inputNode)
+      fireEvent.keyDown(inputNode, {key: 'ArrowDown'})
+
       expect(inputNode.getAttribute('aria-expanded')).toBe('true')
-      // eslint-disable-next-line github/no-blur
-      fireEvent.blur(inputNode)
 
-      // wait a tick for blur to finish
-      await waitFor(() => expect(inputNode.getAttribute('aria-expanded')).not.toBe('true'))
+      await userEvent.tab()
+
+      expect(inputNode.getAttribute('aria-expanded')).not.toBe('true')
     })
 
     it('sets the input value to the suggested item text and highlights the untyped part of the word', async () => {
@@ -306,7 +308,7 @@ describe('Autocomplete', () => {
       expect(onSelectedChangeMock).not.toHaveBeenCalled()
       if (inputNode) {
         fireEvent.focus(inputNode)
-        await user.type(inputNode, '{enter}')
+        await user.type(inputNode, '{arrowdown}{enter}')
       }
 
       expect(onSelectedChangeMock).toHaveBeenCalledWith([mockItems[0]])
@@ -329,6 +331,8 @@ describe('Autocomplete', () => {
       if (inputNode) {
         expect(inputNode.getAttribute('aria-expanded')).not.toBe('true')
         await user.click(inputNode)
+
+        fireEvent.keyDown(inputNode, {key: 'ArrowDown'})
         expect(inputNode.getAttribute('aria-expanded')).toBe('true')
         await user.click(getByText(mockItems[1].text))
         expect(inputNode.getAttribute('aria-expanded')).toBe('true')
@@ -352,6 +356,7 @@ describe('Autocomplete', () => {
       if (inputNode) {
         expect(inputNode.getAttribute('aria-expanded')).not.toBe('true')
         await user.click(inputNode)
+        fireEvent.keyDown(inputNode, {key: 'ArrowDown'})
         expect(inputNode.getAttribute('aria-expanded')).toBe('true')
         await user.click(getByText(mockItems[1].text))
         expect(inputNode.getAttribute('aria-expanded')).not.toBe('true')