Skip to content
Merged
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
18 changes: 16 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,19 @@ export function useAutocomplete<T>(props: AriaAutocompleteOptions<T>, state: Aut
}
};

// Clicking an already-focused input won't emit a new focus event, so clear virtual focus
// on pointer down to restore the input's focused styling before the click completes.
// 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 && getActiveElement(getOwnerDocument(inputRef.current)) === inputRef.current) {
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

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 +444,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
62 changes: 62 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,68 @@ 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]);
options = within(menu).getAllByRole('menuitem');
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
options = within(menu).getAllByRole('menuitem');

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also removed that extra re-fetch.


expect(options[1]).toHaveAttribute('data-focused');
expect(input).not.toHaveAttribute('data-focused');
expect(input).toHaveAttribute('aria-activedescendant', options[1].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[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
Loading