Skip to content

fix: Clear Autocomplete virtualFocus upon paste/undo/redo and other focus fixes #8438

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.

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
1 change: 0 additions & 1 deletion packages/@react-aria/autocomplete/src/useAutocomplete.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,6 @@ export function useAutocomplete(props: AriaAutocompleteOptions, state: Autocompl
collectionRef.current?.dispatchEvent(clearFocusEvent);
});


let lastInputType = useRef('');
useEvent(inputRef, 'input', e => {
let {inputType} = e as InputEvent;
Expand Down
5 changes: 2 additions & 3 deletions packages/@react-aria/selection/src/useSelectableCollection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -416,11 +416,10 @@ export function useSelectableCollection(options: AriaSelectableCollectionOptions
let updateActiveDescendant = useEffectEvent(() => {
let keyToFocus = delegate.getFirstKey?.() ?? null;

// If no focusable items exist in the list, make sure to clear any activedescendant that may still exist
// If no focusable items exist in the list, make sure to clear any activedescendant that may still exist and move focus back to
// the original active element (e.g. the autocomplete input)
if (keyToFocus == null) {
let previousActiveElement = getActiveElement();
// TODO: Bit gross, but we need the first moveVirtualFocus to clear the previous aria-activeDescendant, then
// we need to refocus the input element so the focus ring comes back...
moveVirtualFocus(ref.current);
dispatchVirtualFocus(previousActiveElement!, null);

Expand Down
91 changes: 91 additions & 0 deletions packages/react-aria-components/test/Autocomplete.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,97 @@ describe('Autocomplete', () => {
expect(input).not.toHaveAttribute('data-focused');
});

it('should restore focus visible styles back to the input when typing forward results in only disabled items', async function () {
let {getByRole} = render(
<AutocompleteWrapper>
<StaticMenu disabledKeys={['2']} />
</AutocompleteWrapper>
);

let input = getByRole('searchbox');
await user.tab();
expect(document.activeElement).toBe(input);
expect(input).toHaveAttribute('data-focused');
expect(input).toHaveAttribute('data-focus-visible');

await user.keyboard('Ba');
act(() => jest.runAllTimers());
let menu = getByRole('menu');
let options = within(menu).getAllByRole('menuitem');
let baz = options[1];
expect(baz).toHaveTextContent('Baz');
expect(input).toHaveAttribute('aria-activedescendant', baz.id);
expect(baz).toHaveAttribute('data-focus-visible');
expect(input).not.toHaveAttribute('data-focused');
expect(input).not.toHaveAttribute('data-focus-visible');

await user.keyboard('r');
act(() => jest.runAllTimers());
options = within(menu).getAllByRole('menuitem');
let bar = options[0];
expect(bar).toHaveTextContent('Bar');
expect(input).not.toHaveAttribute('aria-activedescendant');
expect(bar).not.toHaveAttribute('data-focus-visible');
expect(input).toHaveAttribute('data-focused');
expect(input).toHaveAttribute('data-focus-visible');
});

it('should maintain focus styles on the input if typing forward results in an completely empty collection', async function () {
let {getByRole} = render(
<AutocompleteWrapper>
<StaticMenu />
</AutocompleteWrapper>
);

let input = getByRole('searchbox');
await user.tab();
expect(document.activeElement).toBe(input);
expect(input).toHaveAttribute('data-focused');
expect(input).toHaveAttribute('data-focus-visible');

await user.keyboard('Q');
act(() => jest.runAllTimers());
let menu = getByRole('menu');
let options = within(menu).queryAllByRole('menuitem');
expect(options).toHaveLength(0);
expect(input).toHaveAttribute('data-focused');
expect(input).toHaveAttribute('data-focus-visible');
expect(input).not.toHaveAttribute('aria-activedescendant');
});

it('should restore focus visible styles back to the input if the user types forward and backspaces in quick succession', async function () {
let {getByRole} = render(
<AutocompleteWrapper>
<StaticMenu />
</AutocompleteWrapper>
);

let input = getByRole('searchbox');
await user.tab();
expect(document.activeElement).toBe(input);
expect(input).toHaveAttribute('data-focused');
expect(input).toHaveAttribute('data-focus-visible');

await user.keyboard('F');
// If 500ms hasn't elapsed the aria-activedecendant hasn't been updated
act(() => jest.advanceTimersByTime(300));
Comment on lines +502 to +503
Copy link
Member

Choose a reason for hiding this comment

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

500 v 300?

Copy link
Member Author

Choose a reason for hiding this comment

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

yep, if I advance more than 500ms, the aria-activedescendant will be set which is the typical flow tested by the other tests via runAllTimers. The buggy flow was if the user then backspaces before that 500ms finishes resulting in a state where the input "blurred" away from itself, but then interrupts updating the activedescendant, resulting in a state where getVirtuallyFocusedElement doesn't return a value and moveVirtualFocus will think that virtual focus didn't move away from the input.

let menu = getByRole('menu');
let options = within(menu).getAllByRole('menuitem');
let foo = options[0];
expect(foo).toHaveTextContent('Foo');
expect(input).not.toHaveAttribute('aria-activedescendant');
expect(foo).toHaveAttribute('data-focus-visible');
expect(input).not.toHaveAttribute('data-focused');
expect(input).not.toHaveAttribute('data-focus-visible');

await user.keyboard('{Backspace}');
act(() => jest.runAllTimers());
expect(input).toHaveAttribute('data-focused');
expect(input).toHaveAttribute('data-focus-visible');
expect(input).not.toHaveAttribute('aria-activedescendant');
expect(foo).not.toHaveAttribute('data-focus-visible');
});

it('should work inside a Select', async function () {
let {getByRole} = render(
<Select>
Expand Down