From 64cce8031c9af3048182b0e2cfdd2ca276a53b14 Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Wed, 24 Jul 2024 08:50:51 -0400 Subject: [PATCH 01/10] Only open menu on click instead of just focus --- packages/react/src/Autocomplete/AutocompleteInput.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/react/src/Autocomplete/AutocompleteInput.tsx b/packages/react/src/Autocomplete/AutocompleteInput.tsx index f114d881aee..5e6fcee1e4b 100644 --- a/packages/react/src/Autocomplete/AutocompleteInput.tsx +++ b/packages/react/src/Autocomplete/AutocompleteInput.tsx @@ -1,4 +1,4 @@ -import type {ChangeEventHandler, FocusEventHandler, KeyboardEventHandler} from 'react' +import type {ChangeEventHandler, FocusEventHandler, KeyboardEventHandler, MouseEventHandler} from 'react' import React, {useCallback, useContext, useEffect, useState} from 'react' import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic' import {AutocompleteContext} from './AutocompleteContext' @@ -52,7 +52,7 @@ const AutocompleteInput = React.forwardRef( const [highlightRemainingText, setHighlightRemainingText] = useState(true) const {safeSetTimeout} = useSafeTimeout() - const handleInputFocus: FocusEventHandler = useCallback( + const handleInputClick: MouseEventHandler = useCallback( event => { if (openOnFocus) { onFocus?.(event) @@ -166,7 +166,7 @@ const AutocompleteInput = React.forwardRef( return ( Date: Wed, 24 Jul 2024 13:40:45 -0400 Subject: [PATCH 02/10] Update tests --- .../react/src/Autocomplete/AutocompleteInput.tsx | 6 +++--- packages/react/src/__tests__/Autocomplete.test.tsx | 13 ++++++------- .../__snapshots__/Autocomplete.test.tsx.snap | 7 +++++++ 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/packages/react/src/Autocomplete/AutocompleteInput.tsx b/packages/react/src/Autocomplete/AutocompleteInput.tsx index 5e6fcee1e4b..980abaff06a 100644 --- a/packages/react/src/Autocomplete/AutocompleteInput.tsx +++ b/packages/react/src/Autocomplete/AutocompleteInput.tsx @@ -21,7 +21,7 @@ const AutocompleteInput = React.forwardRef( ( { as: Component = TextInput, - onFocus, + onClick, onBlur, onChange, onKeyDown, @@ -55,11 +55,11 @@ const AutocompleteInput = React.forwardRef( const handleInputClick: MouseEventHandler = useCallback( event => { if (openOnFocus) { - onFocus?.(event) + onClick?.(event) setShowMenu(true) } }, - [onFocus, setShowMenu, openOnFocus], + [onClick, setShowMenu, openOnFocus], ) const handleInputBlur: FocusEventHandler = useCallback( diff --git a/packages/react/src/__tests__/Autocomplete.test.tsx b/packages/react/src/__tests__/Autocomplete.test.tsx index 3ce3bbc3598..81f873f3a69 100644 --- 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' @@ -137,7 +137,7 @@ describe('Autocomplete', () => { const inputNode = getByLabelText(AUTOCOMPLETE_LABEL) expect(inputNode.getAttribute('aria-expanded')).not.toBe('true') - fireEvent.focus(inputNode) + fireEvent.click(inputNode) expect(inputNode.getAttribute('aria-expanded')).toBe('true') }) @@ -148,13 +148,12 @@ describe('Autocomplete', () => { const inputNode = getByLabelText(AUTOCOMPLETE_LABEL) expect(inputNode.getAttribute('aria-expanded')).not.toBe('true') - fireEvent.focus(inputNode) + fireEvent.click(inputNode) 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 () => { diff --git a/packages/react/src/__tests__/__snapshots__/Autocomplete.test.tsx.snap b/packages/react/src/__tests__/__snapshots__/Autocomplete.test.tsx.snap index 529e9781448..8efecb68827 100644 --- a/packages/react/src/__tests__/__snapshots__/Autocomplete.test.tsx.snap +++ b/packages/react/src/__tests__/__snapshots__/Autocomplete.test.tsx.snap @@ -132,6 +132,7 @@ exports[`snapshots renders a custom empty state message 1`] = ` id="autocompleteId" onBlur={[Function]} onChange={[Function]} + onClick={[Function]} onFocus={[Function]} onKeyDown={[Function]} onKeyPress={[Function]} @@ -307,6 +308,7 @@ exports[`snapshots renders a loading state 1`] = ` id="autocompleteId" onBlur={[Function]} onChange={[Function]} + onClick={[Function]} onFocus={[Function]} onKeyDown={[Function]} onKeyPress={[Function]} @@ -562,6 +564,7 @@ exports[`snapshots renders a menu that contains an item to add to the menu 1`] = id="autocompleteId" onBlur={[Function]} onChange={[Function]} + onClick={[Function]} onFocus={[Function]} onKeyDown={[Function]} onKeyPress={[Function]} @@ -1371,6 +1374,7 @@ exports[`snapshots renders a multiselect input 1`] = ` id="autocompleteId" onBlur={[Function]} onChange={[Function]} + onClick={[Function]} onFocus={[Function]} onKeyDown={[Function]} onKeyPress={[Function]} @@ -2082,6 +2086,7 @@ exports[`snapshots renders a multiselect input with selected menu items 1`] = ` id="autocompleteId" onBlur={[Function]} onChange={[Function]} + onClick={[Function]} onFocus={[Function]} onKeyDown={[Function]} onKeyPress={[Function]} @@ -2934,6 +2939,7 @@ exports[`snapshots renders a single select input 1`] = ` id="autocompleteId" onBlur={[Function]} onChange={[Function]} + onClick={[Function]} onFocus={[Function]} onKeyDown={[Function]} onKeyPress={[Function]} @@ -4006,6 +4012,7 @@ exports[`snapshots renders with an input value 1`] = ` id="autocompleteId" onBlur={[Function]} onChange={[Function]} + onClick={[Function]} onFocus={[Function]} onKeyDown={[Function]} onKeyPress={[Function]} From 08fddb9aa3fde5daab070a9d57c3cc83a03c86db Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Wed, 24 Jul 2024 13:51:00 -0400 Subject: [PATCH 03/10] Add changeset --- .changeset/four-shoes-yell.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/four-shoes-yell.md diff --git a/.changeset/four-shoes-yell.md b/.changeset/four-shoes-yell.md new file mode 100644 index 00000000000..2b587d90826 --- /dev/null +++ b/.changeset/four-shoes-yell.md @@ -0,0 +1,5 @@ +--- +'@primer/react': minor +--- + +Only show `Autocomplete` menu when click instead of focus. From 3a01d11360426cfa775516c31ccfb9952226a0db Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Wed, 24 Jul 2024 15:35:33 -0400 Subject: [PATCH 04/10] Fix typo --- .changeset/four-shoes-yell.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/four-shoes-yell.md b/.changeset/four-shoes-yell.md index 2b587d90826..3479eaa50bf 100644 --- a/.changeset/four-shoes-yell.md +++ b/.changeset/four-shoes-yell.md @@ -2,4 +2,4 @@ '@primer/react': minor --- -Only show `Autocomplete` menu when click instead of focus. +Only show `Autocomplete` menu when clicked instead of focus. From 07f13d5ea2b90979cdede65d2d8db608159e0dd3 Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Fri, 26 Jul 2024 08:43:08 -0400 Subject: [PATCH 05/10] Set `openOnFocus` to `true` --- .../src/Autocomplete/AutocompleteInput.tsx | 23 ++++++++----------- .../react/src/__tests__/Autocomplete.test.tsx | 16 +++++++++++-- .../__snapshots__/Autocomplete.test.tsx.snap | 7 ------ 3 files changed, 24 insertions(+), 22 deletions(-) diff --git a/packages/react/src/Autocomplete/AutocompleteInput.tsx b/packages/react/src/Autocomplete/AutocompleteInput.tsx index 980abaff06a..4569a9abf31 100644 --- a/packages/react/src/Autocomplete/AutocompleteInput.tsx +++ b/packages/react/src/Autocomplete/AutocompleteInput.tsx @@ -1,4 +1,4 @@ -import type {ChangeEventHandler, FocusEventHandler, KeyboardEventHandler, MouseEventHandler} from 'react' +import type {ChangeEventHandler, FocusEventHandler, KeyboardEventHandler} from 'react' import React, {useCallback, useContext, useEffect, useState} from 'react' import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic' import {AutocompleteContext} from './AutocompleteContext' @@ -21,14 +21,14 @@ const AutocompleteInput = React.forwardRef( ( { as: Component = TextInput, - onClick, + onFocus, onBlur, onChange, onKeyDown, onKeyUp, onKeyPress, value, - openOnFocus = true, + openOnFocus = false, ...props }, forwardedRef, @@ -52,15 +52,12 @@ const AutocompleteInput = React.forwardRef( const [highlightRemainingText, setHighlightRemainingText] = useState(true) const {safeSetTimeout} = useSafeTimeout() - const handleInputClick: MouseEventHandler = useCallback( - event => { - if (openOnFocus) { - onClick?.(event) - setShowMenu(true) - } - }, - [onClick, setShowMenu, openOnFocus], - ) + const handleInputFocus: FocusEventHandler = event => { + if (openOnFocus) { + onFocus?.(event) + setShowMenu(true) + } + } const handleInputBlur: FocusEventHandler = useCallback( event => { @@ -166,7 +163,7 @@ const AutocompleteInput = React.forwardRef( return ( { 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( , ) @@ -138,6 +138,7 @@ describe('Autocomplete', () => { expect(inputNode.getAttribute('aria-expanded')).not.toBe('true') fireEvent.click(inputNode) + fireEvent.keyDown(inputNode, {key: 'ArrowDown'}) expect(inputNode.getAttribute('aria-expanded')).toBe('true') }) @@ -149,6 +150,8 @@ describe('Autocomplete', () => { expect(inputNode.getAttribute('aria-expanded')).not.toBe('true') fireEvent.click(inputNode) + fireEvent.keyDown(inputNode, {key: 'ArrowDown'}) + expect(inputNode.getAttribute('aria-expanded')).toBe('true') await userEvent.tab() @@ -305,7 +308,13 @@ describe('Autocomplete', () => { expect(onSelectedChangeMock).not.toHaveBeenCalled() if (inputNode) { fireEvent.focus(inputNode) - await user.type(inputNode, '{enter}') + fireEvent.keyDown(inputNode, {key: 'ArrowDown'}) + + await user.keyboard('{Enter}') + fireEvent.keyDown(inputNode, {key: 'Enter'}) + + console.log(inputNode.value) + expect(inputNode.getAttribute('aria-expanded')).toBe('false') } expect(onSelectedChangeMock).toHaveBeenCalledWith([mockItems[0]]) @@ -328,6 +337,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') @@ -351,6 +362,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') diff --git a/packages/react/src/__tests__/__snapshots__/Autocomplete.test.tsx.snap b/packages/react/src/__tests__/__snapshots__/Autocomplete.test.tsx.snap index 8efecb68827..529e9781448 100644 --- a/packages/react/src/__tests__/__snapshots__/Autocomplete.test.tsx.snap +++ b/packages/react/src/__tests__/__snapshots__/Autocomplete.test.tsx.snap @@ -132,7 +132,6 @@ exports[`snapshots renders a custom empty state message 1`] = ` id="autocompleteId" onBlur={[Function]} onChange={[Function]} - onClick={[Function]} onFocus={[Function]} onKeyDown={[Function]} onKeyPress={[Function]} @@ -308,7 +307,6 @@ exports[`snapshots renders a loading state 1`] = ` id="autocompleteId" onBlur={[Function]} onChange={[Function]} - onClick={[Function]} onFocus={[Function]} onKeyDown={[Function]} onKeyPress={[Function]} @@ -564,7 +562,6 @@ exports[`snapshots renders a menu that contains an item to add to the menu 1`] = id="autocompleteId" onBlur={[Function]} onChange={[Function]} - onClick={[Function]} onFocus={[Function]} onKeyDown={[Function]} onKeyPress={[Function]} @@ -1374,7 +1371,6 @@ exports[`snapshots renders a multiselect input 1`] = ` id="autocompleteId" onBlur={[Function]} onChange={[Function]} - onClick={[Function]} onFocus={[Function]} onKeyDown={[Function]} onKeyPress={[Function]} @@ -2086,7 +2082,6 @@ exports[`snapshots renders a multiselect input with selected menu items 1`] = ` id="autocompleteId" onBlur={[Function]} onChange={[Function]} - onClick={[Function]} onFocus={[Function]} onKeyDown={[Function]} onKeyPress={[Function]} @@ -2939,7 +2934,6 @@ exports[`snapshots renders a single select input 1`] = ` id="autocompleteId" onBlur={[Function]} onChange={[Function]} - onClick={[Function]} onFocus={[Function]} onKeyDown={[Function]} onKeyPress={[Function]} @@ -4012,7 +4006,6 @@ exports[`snapshots renders with an input value 1`] = ` id="autocompleteId" onBlur={[Function]} onChange={[Function]} - onClick={[Function]} onFocus={[Function]} onKeyDown={[Function]} onKeyPress={[Function]} From a177078ea4e8d6405de791d96dc35d4e9ff71517 Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Mon, 29 Jul 2024 14:36:27 -0400 Subject: [PATCH 06/10] Add test, move `onFocus` function --- packages/react/src/Autocomplete/AutocompleteInput.tsx | 3 +-- packages/react/src/__tests__/Autocomplete.test.tsx | 8 +------- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/packages/react/src/Autocomplete/AutocompleteInput.tsx b/packages/react/src/Autocomplete/AutocompleteInput.tsx index 4569a9abf31..ca26de7db30 100644 --- a/packages/react/src/Autocomplete/AutocompleteInput.tsx +++ b/packages/react/src/Autocomplete/AutocompleteInput.tsx @@ -53,8 +53,8 @@ const AutocompleteInput = React.forwardRef( const {safeSetTimeout} = useSafeTimeout() const handleInputFocus: FocusEventHandler = event => { + onFocus?.(event) if (openOnFocus) { - onFocus?.(event) setShowMenu(true) } } @@ -119,7 +119,6 @@ const AutocompleteInput = React.forwardRef( const onInputKeyPress: KeyboardEventHandler = 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 index f5ad7b08617..34542a4ba2e 100644 --- a/packages/react/src/__tests__/Autocomplete.test.tsx +++ b/packages/react/src/__tests__/Autocomplete.test.tsx @@ -308,13 +308,7 @@ describe('Autocomplete', () => { expect(onSelectedChangeMock).not.toHaveBeenCalled() if (inputNode) { fireEvent.focus(inputNode) - fireEvent.keyDown(inputNode, {key: 'ArrowDown'}) - - await user.keyboard('{Enter}') - fireEvent.keyDown(inputNode, {key: 'Enter'}) - - console.log(inputNode.value) - expect(inputNode.getAttribute('aria-expanded')).toBe('false') + await user.type(inputNode, '{arrowdown}{enter}') } expect(onSelectedChangeMock).toHaveBeenCalledWith([mockItems[0]]) From 7e2fdf69132726d904960aef5542f14f09ca8a42 Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Mon, 29 Jul 2024 14:38:24 -0400 Subject: [PATCH 07/10] Update docs --- packages/react/src/Autocomplete/Autocomplete.docs.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/src/Autocomplete/Autocomplete.docs.json b/packages/react/src/Autocomplete/Autocomplete.docs.json index 80da9258ba4..d5318ebc899 100644 --- a/packages/react/src/Autocomplete/Autocomplete.docs.json +++ b/packages/react/src/Autocomplete/Autocomplete.docs.json @@ -23,7 +23,7 @@ { "name": "openOnFocus", "type": "boolean", - "defaultValue": "true", + "defaultValue": "false", "description": "Whether the associated autocomplete menu should open on an input focus event" } ], From 08e6647719c4ce2f15d6caf8de264ab2630cede2 Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Mon, 29 Jul 2024 14:42:37 -0400 Subject: [PATCH 08/10] Adjust changeset --- .changeset/four-shoes-yell.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/four-shoes-yell.md b/.changeset/four-shoes-yell.md index 3479eaa50bf..6920922ec27 100644 --- a/.changeset/four-shoes-yell.md +++ b/.changeset/four-shoes-yell.md @@ -2,4 +2,4 @@ '@primer/react': minor --- -Only show `Autocomplete` menu when clicked instead of focus. +Set `openOnFocus` default to `false`, making the menu closed initially rather than opening on focus of input From 3544fe58da9b3d71eaa8477d28b8a01211dbd6d4 Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Mon, 29 Jul 2024 14:47:49 -0400 Subject: [PATCH 09/10] Remove `useCallback` --- .../src/Autocomplete/AutocompleteInput.tsx | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/packages/react/src/Autocomplete/AutocompleteInput.tsx b/packages/react/src/Autocomplete/AutocompleteInput.tsx index ca26de7db30..eb67ac718de 100644 --- a/packages/react/src/Autocomplete/AutocompleteInput.tsx +++ b/packages/react/src/Autocomplete/AutocompleteInput.tsx @@ -75,16 +75,13 @@ const AutocompleteInput = React.forwardRef( [onBlur, setShowMenu, inputRef, safeSetTimeout], ) - const handleInputChange: ChangeEventHandler = useCallback( - event => { - onChange && onChange(event) - setInputValue(event.currentTarget.value) - if (!showMenu) { - setShowMenu(true) - } - }, - [onChange, setInputValue, setShowMenu, showMenu], - ) + const handleInputChange: ChangeEventHandler = event => { + onChange && onChange(event) + setInputValue(event.currentTarget.value) + if (!showMenu) { + setShowMenu(true) + } + } const handleInputKeyDown: KeyboardEventHandler = useCallback( event => { From 36b8074b562d43aa71c19b34293cf949f6a2e737 Mon Sep 17 00:00:00 2001 From: Tyler Jones Date: Mon, 29 Jul 2024 15:08:58 -0400 Subject: [PATCH 10/10] Add deprecated notice --- packages/react/src/Autocomplete/Autocomplete.docs.json | 3 ++- packages/react/src/Autocomplete/AutocompleteInput.tsx | 7 +++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/react/src/Autocomplete/Autocomplete.docs.json b/packages/react/src/Autocomplete/Autocomplete.docs.json index d5318ebc899..ff776c4ab01 100644 --- a/packages/react/src/Autocomplete/Autocomplete.docs.json +++ b/packages/react/src/Autocomplete/Autocomplete.docs.json @@ -24,7 +24,8 @@ "name": "openOnFocus", "type": "boolean", "defaultValue": "false", - "description": "Whether the associated autocomplete menu should open on an input focus event" + "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 index eb67ac718de..873781965b5 100644 --- 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> - // 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 }