Skip to content

fix(selection): prevent empty ListBox from stealing focus in Select Popover #9808

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

Open
albertdugba wants to merge 1 commit into
base: main
Choose a base branch
from
+30 −6
Open

fix(selection): prevent empty ListBox from stealing focus in Select Popover #9808

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
14 changes: 8 additions & 6 deletions packages/@react-aria/selection/src/useSelectableCollection.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,11 @@ export function useSelectableCollection(options: AriaSelectableCollectionOptions
const didAutoFocusRef = useRef(false);
useEffect(() => {
if (autoFocusRef.current) {
// Don't autofocus an empty collection. Wait until items are added.
Copy link
Member

@snowystinger snowystinger Mar 18, 2026

Choose a reason for hiding this comment

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

I think we can still autoFocus an empty collection, but it's hard to tell the difference between an intentionally empty collection and one that is waiting for the first set of items to appear due to the double render nature of collections.

In addition, this may now steal focus unintentionally if you go from 0 items to N items much later.

I think the solution to this would typically be, if the user knows that it's intentionally empty and they don't want to autoFocus the empty collection, they set autoFocus to false.

I realise this may be harder inside a Select, you could intercept the ListBoxContext and override that property.

Will have to think on this some more.

if (manager.collection.size === 0) {
return;
}

let focusedKey: Key | null = null;

// Check focus strategy to determine which item to focus
Expand Down Expand Up @@ -493,11 +498,8 @@ export function useSelectableCollection(options: AriaSelectableCollectionOptions
focusSafely(ref.current);
}

// Wait until the collection has items to autofocus.
if (manager.collection.size > 0) {
autoFocusRef.current = false;
didAutoFocusRef.current = true;
}
autoFocusRef.current = false;
didAutoFocusRef.current = true;
}
});

Expand Down Expand Up @@ -581,7 +583,7 @@ export function useSelectableCollection(options: AriaSelectableCollectionOptions
// This will be marshalled to either the first or last item depending on where focus came from.
let tabIndex: number | undefined = undefined;
if (!shouldUseVirtualFocus) {
tabIndex = manager.focusedKey == null ? 0 : -1;
tabIndex = manager.focusedKey == null && manager.collection.size > 0 ? 0 : -1;
}

let collectionId = useCollectionId(manager.collection);
Expand Down
22 changes: 22 additions & 0 deletions packages/react-aria-components/test/Select.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,28 @@ describe('Select', () => {
expect(option).toHaveTextContent('No results');
});

it('should not allow empty listbox to steal focus from other elements in the popover', async () => {
let {getByTestId, getByRole} = render(
<Select data-testid="select" allowsEmptyCollection>
<Label>Favorite Animal</Label>
<Button>
<SelectValue />
</Button>
<Popover>
<ListBox aria-label="Test" renderEmptyState={() => 'No results'}>
{[]}
</ListBox>
</Popover>
</Select>
);

let selectTester = testUtilUser.createTester('Select', {root: getByTestId('select')});
await selectTester.open();

let listbox = getByRole('listbox');
expect(listbox).toHaveAttribute('tabIndex', '-1');
});

it('should support render props', async () => {
let {getByTestId} = render(
<Select data-testid="select">
Expand Down
Loading