Fix Autocomplete focus styles when clicking the input after virtual focus#9767
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes an Autocomplete virtual-focus edge case where clicking an already-focused input doesn’t restore the input’s focused styling (and doesn’t clear aria-activedescendant) after virtual focus has moved to an option.
Changes:
- Clear virtual focus on
pointerdownwhen the input is already the active element (excluding touch), so focused styling is restored on click. - Add regression tests covering hover-driven and keyboard-driven virtual focus recovery scenarios.
- Add a Storybook repro story for manual verification of focus recovery behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/@react-aria/autocomplete/src/useAutocomplete.ts | Adds an onPointerDown handler (virtual-focus mode) to clear virtual focus when clicking an already-focused input. |
| packages/react-aria-components/test/Autocomplete.test.tsx | Adds regression tests ensuring focused styles and aria-activedescendant are restored/cleared when clicking the input after virtual focus moves to options. |
| packages/react-aria-components/stories/Autocomplete.stories.tsx | Adds a Storybook scenario to manually reproduce and validate focus recovery after virtual focus. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
snowystinger
left a comment
snowystinger
left a comment
There was a problem hiding this comment.
Thanks for the PR. One oddity about this, the mouse shouldn't actually cause a visible focus ring usually. There should probably be a "data-focus-visible" condition to the css as well.
| let menu = getByRole('menu'); | ||
| let options = within(menu).getAllByRole('menuitem'); | ||
| await user.hover(options[1]); | ||
| options = within(menu).getAllByRole('menuitem'); |
There was a problem hiding this comment.
| options = within(menu).getAllByRole('menuitem'); |
There was a problem hiding this comment.
Also removed that extra re-fetch.
| return; | ||
| } | ||
|
|
||
| if (getEventTarget(e) === inputRef.current && getActiveElement(getOwnerDocument(inputRef.current)) === inputRef.current) { |
There was a problem hiding this comment.
why the check for activeElement? I was trying out the change in https://react-aria.adobe.com/Autocomplete (though my local copy of yarn start:s2-docs and if I mouse over an option, then click outside the menu/input but still inside the dialog (so it doesn't dismiss), then I see that clicking on the input after that won't get the focus ring.
There was a problem hiding this comment.
You were right here. I removed the activeElement check, added a regression test for the outside-click case, and switched the starter input ring to data-focus-visible.
snowystinger
left a comment
snowystinger
left a comment
There was a problem hiding this comment.
That looks like it's behaving correctly. Thanks for updating that and the extra test!
devongovett
left a comment
devongovett
left a comment
There was a problem hiding this comment.
Another concern is that this means the behavior of mouse vs keyboard is inconsistent. For mouse, if you click away and then back into the input, it would clear. But if you tab away and return it will not clear. Is this a problem? 🤷
| // interactions restore the input state before the click's focus handling runs. | ||
| // Touch is excluded because touch interactions should not move focus back to the input. | ||
| let onPointerDown = (e: ReactPointerEvent) => { | ||
| if (e.button !== 0 || e.pointerType === 'touch' || queuedActiveDescendant.current == null || inputRef.current == null) { |
There was a problem hiding this comment.
Fair point. I kept it excluded because removing that guard changes touch behavior too - re-tapping would start clearing virtual focus on touch as well, and I wasn't sure that was intended.
There was a problem hiding this comment.
I looked into making the return-to-input behavior consistently clear, but it affects more than this PR's original scope (virtual focus restoration, submenu/subdialog cases, etc).
Would it make sense to keep this PR focused on the original focus recovery / styling issue, and tackle the broader preserve-vs-clear question separately? I can follow up in either direction once the intended behavior is clear.
starters/docs/src/TextField.css
Outdated
| } | ||
|
|
||
| &[data-focused] { | ||
| &[data-focus-visible] { |
There was a problem hiding this comment.
I think we should keep the styles unchanged. There should be a focus state even for mouse
There was a problem hiding this comment.
Reverted this in 8c4d7f4 so the starter input keeps the existing mouse focus styling.
Closes #8198
Fixes the Autocomplete case where virtual focus moves to an option and clicking the already-focused input does not restore the input's focused state. This adds a small fix, regression tests, and a storybook repro for manual verification. No documentation changes seem necessary for this behavior fix.
✅ Pull Request Checklist:
📝 Test Instructions:
Automated:
yarn jest packages/react-aria-components/test/Autocomplete.test.tsx --runInBandyarn jest packages/@react-aria/autocomplete/test/useSearchAutocomplete.test.js --runInBandyarn jest packages/react-aria-components/test/ComboBox.test.js --runInBandyarn jest packages/@react-aria/combobox/test/useComboBox.test.js --runInBandyarn eslint packages/@react-aria/autocomplete/src/useAutocomplete.ts packages/react-aria-components/test/Autocomplete.test.tsx packages/react-aria-components/stories/Autocomplete.stories.tsxyarn check-typesManual:
yarn start.React Aria Components/Autocomplete.Autocomplete focus recovery after virtual focus.🧢 Your Project: