Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Autocomplete: Only open menu on click #4771

Merged
merged 10 commits into from
Jul 29, 2024
Merged

Autocomplete: Only open menu on click #4771

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
64cce80
Only open menu on click instead of just focus
TylerJDev Jul 24, 2024
a909a64
Update tests
TylerJDev Jul 24, 2024
08fddb9
Add changeset
TylerJDev Jul 24, 2024
3a01d11
Fix typo
TylerJDev Jul 24, 2024
07f13d5
Set `openOnFocus` to `true`
TylerJDev Jul 26, 2024
a177078
Add test, move `onFocus` function
TylerJDev Jul 29, 2024
7e2fdf6
Update docs
TylerJDev Jul 29, 2024
08e6647
Adjust changeset
TylerJDev Jul 29, 2024
3544fe5
Remove `useCallback`
TylerJDev Jul 29, 2024
36b8074
Add deprecated notice
TylerJDev Jul 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/four-shoes-yell.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react': minor
---

Only show `Autocomplete` menu when clicked instead of focus.
12 changes: 6 additions & 6 deletions packages/react/src/Autocomplete/AutocompleteInput.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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'
Expand All @@ -21,7 +21,7 @@ const AutocompleteInput = React.forwardRef(
(
{
as: Component = TextInput,
onFocus,
onClick,
onBlur,
onChange,
onKeyDown,
Expand Down Expand Up @@ -52,14 +52,14 @@ const AutocompleteInput = React.forwardRef(
const [highlightRemainingText, setHighlightRemainingText] = useState<boolean>(true)
const {safeSetTimeout} = useSafeTimeout()

const handleInputFocus: FocusEventHandler<HTMLInputElement> = useCallback(
const handleInputClick: MouseEventHandler<HTMLInputElement> = useCallback(
event => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to ignore this since it's not really related to the changes proposed 👀

I wanted to ask if this useCallback is doing much for us or if it would be something we could refactor to a default event handler / function.

Copy link
Contributor Author

@TylerJDev TylerJDev Jul 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah we definitely can adjust this!

Edit: Would we want to do this for all the event handlers using useCallback?

if (openOnFocus) {
onFocus?.(event)
onClick?.(event)
setShowMenu(true)
}
},
[onFocus, setShowMenu, openOnFocus],
[onClick, setShowMenu, openOnFocus],
)

const handleInputBlur: FocusEventHandler<HTMLInputElement> = useCallback(
Expand Down Expand Up @@ -166,7 +166,7 @@ const AutocompleteInput = React.forwardRef(

return (
<Component
onFocus={handleInputFocus}
onClick={handleInputClick}
onBlur={handleInputBlur}
onChange={handleInputChange}
onKeyDown={handleInputKeyDown}
Expand Down
13 changes: 6 additions & 7 deletions packages/react/src/__tests__/Autocomplete.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -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')
})

Expand All @@ -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 () => {
Expand Down
7 changes: 7 additions & 0 deletions packages/react/src/__tests__/__snapshots__/Autocomplete.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]}
Expand Down Expand Up @@ -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]}
Expand Down Expand Up @@ -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]}
Expand Down Expand Up @@ -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]}
Expand Down Expand Up @@ -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]}
Expand Down Expand Up @@ -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]}
Expand Down Expand Up @@ -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]}
Expand Down
Loading