Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
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
19 changes: 17 additions & 2 deletions packages/@react-aria/autocomplete/src/useAutocomplete.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {dispatchVirtualBlur, dispatchVirtualFocus, getVirtuallyFocusedElement, m
import {getInteractionModality, getPointerType} from '@react-aria/interactions';
// @ts-ignore
import intlMessages from '../intl/*.json';
import {FocusEvent as ReactFocusEvent, KeyboardEvent as ReactKeyboardEvent, useCallback, useEffect, useMemo, useRef, useState} from 'react';
import {FocusEvent as ReactFocusEvent, KeyboardEvent as ReactKeyboardEvent, PointerEvent as ReactPointerEvent, useCallback, useEffect, useMemo, useRef, useState} from 'react';
import {useLocalizedStringFormatter} from '@react-aria/i18n';

export interface CollectionOptions extends DOMProps, AriaLabelingProps {
Expand Down Expand Up @@ -420,6 +420,20 @@ export function useAutocomplete<T>(props: AriaAutocompleteOptions<T>, state: Aut
}
};

// Clicking back into the input can happen after focus moved elsewhere in the dialog, while
// virtual focus is still on an option. Clear virtual focus on pointer down so mouse
// 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) {
Copy link
Member

Choose a reason for hiding this comment

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

Why is touch excluded?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

return;
}

if (getEventTarget(e) === inputRef.current) {
clearVirtualFocus();
}
};

// Only apply the autocomplete specific behaviors if the collection component wrapped by it is actually
// being filtered/allows filtering by the Autocomplete.
let inputProps = {
Expand All @@ -431,7 +445,8 @@ export function useAutocomplete<T>(props: AriaAutocompleteOptions<T>, state: Aut
onKeyDown,
'aria-activedescendant': state.focusedNodeId ?? undefined,
onBlur,
onFocus
onFocus,
onPointerDown
};

if (hasCollection) {
Expand Down
23 changes: 23 additions & 0 deletions packages/react-aria-components/stories/Autocomplete.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,29 @@ export const AutocompleteSearchfield: AutocompleteStory = {
}
};

export const AutocompleteFocusRecovery: AutocompleteStory = {
render: (args) => {
return (
<AutocompleteWrapper disableVirtualFocus={args.disableVirtualFocus}>
<div>
<TextField autoFocus data-testid="autocomplete-focus-recovery">
<Label style={{display: 'block'}}>Test</Label>
<Input />
<Text style={{display: 'block'}} slot="description">Focus the input, move virtual focus to an option, then click the input again.</Text>
</TextField>
<StaticMenu {...args} />
</div>
</AutocompleteWrapper>
);
},
name: 'Autocomplete focus recovery after virtual focus',
parameters: {
description: {
data: 'Manual check: focus the input, hover or keyboard navigate to an option, then click the input again. The input should regain focused styling and the active descendant should clear.'
}
}
};

// Note that the trigger items in this array MUST have an id, even if the underlying MenuItem might apply its own
// id. If it is omitted, we can't build the collection node for the trigger node and an error will throw
let dynamicAutocompleteSubdialog: MenuNode[] = [
Expand Down
94 changes: 94 additions & 0 deletions packages/react-aria-components/test/Autocomplete.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -496,6 +496,100 @@ describe('Autocomplete', () => {
expect(input).toHaveAttribute('data-focus-visible');
});

it('should restore focused styles to the input when clicking it after hovering an option', async () => {
let {getByRole} = render(
<AutocompleteWrapper>
<StaticMenu />
</AutocompleteWrapper>
);

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

let menu = getByRole('menu');
let options = within(menu).getAllByRole('menuitem');
await user.hover(options[1]);

expect(options[1]).toHaveAttribute('data-focused');
expect(input).not.toHaveAttribute('data-focused');
expect(input).toHaveAttribute('aria-activedescendant', options[1].id);

await user.click(input);

expect(document.activeElement).toBe(input);
expect(input).toHaveAttribute('data-focused');
expect(input).not.toHaveAttribute('data-focus-visible');
expect(input).not.toHaveAttribute('aria-activedescendant');
expect(options[1]).not.toHaveAttribute('data-focused');
});

it('should restore the input state after clicking outside the autocomplete and then back into the input', async () => {
let {getByRole} = render(
<>
<button type="button">Outside</button>
<AutocompleteWrapper>
<StaticMenu />
</AutocompleteWrapper>
</>
);

let input = getByRole('searchbox');
await user.click(input);

let menu = getByRole('menu');
let options = within(menu).getAllByRole('menuitem');
await user.hover(options[1]);

expect(input).toHaveAttribute('aria-activedescendant', options[1].id);
expect(options[1]).toHaveAttribute('data-focused');

await user.click(getByRole('button', {name: 'Outside'}));
expect(document.activeElement).toHaveTextContent('Outside');
expect(input).not.toHaveAttribute('data-focused');
expect(input).not.toHaveAttribute('data-focus-visible');

await user.click(input);

expect(document.activeElement).toBe(input);
expect(input).toHaveAttribute('data-focused');
expect(input).not.toHaveAttribute('data-focus-visible');
expect(input).not.toHaveAttribute('aria-activedescendant');
expect(options[1]).not.toHaveAttribute('data-focused');
});

it('should restore focused styles to the input when clicking it after keyboard focusing an option', async () => {
let {getByRole} = render(
<AutocompleteWrapper>
<StaticMenu />
</AutocompleteWrapper>
);

let input = getByRole('searchbox');
await user.click(input);
expect(document.activeElement).toBe(input);

await user.keyboard('{ArrowDown}');
act(() => jest.runAllTimers());

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

await user.click(input);
act(() => jest.runAllTimers());

expect(document.activeElement).toBe(input);
expect(input).toHaveAttribute('data-focused');
expect(input).not.toHaveAttribute('data-focus-visible');
expect(input).not.toHaveAttribute('aria-activedescendant');
expect(options[0]).not.toHaveAttribute('data-focused');
expect(options[0]).not.toHaveAttribute('data-focus-visible');
});

it('should not display focus in the virtually focused menu if focus isn\'t in the autocomplete input', async function () {
let {getByRole} = render(
<>
Expand Down
2 changes: 1 addition & 1 deletion starters/docs/src/TextField.css
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
color: var(--text-color-placeholder);
}

&[data-focused] {
&[data-focus-visible] {
Copy link
Member

Choose a reason for hiding this comment

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

I think we should keep the styles unchanged. There should be a focus state even for mouse

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reverted this in 8c4d7f4 so the starter input keeps the existing mouse focus styling.

outline: 2px solid var(--focus-ring-color);
outline-offset: -1px;
}
Expand Down
Loading