fix: #9239 Focus on the first available option in the dropdown by jsmitrah · Pull Request #10236 · adobe/react-spectrum

jsmitrah · 2026-06-19T15:27:05Z

✅ Pull Request Checklist:

Included link to corresponding React Spectrum GitHub Issue.
Added/updated unit tests and storybook for this change (for new code or code which already has tests).
Filled out test instructions.
Updated documentation (if it already exists for this component).
Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

Configure the component data so that the first 2 or 3 items in the list are disabled.
Open the component and verify that the initial focus outline snaps straight to the first available option automatically, without needing an extra keypress.

🧢 Your Project:

snowystinger

Thanks for the PR. I'd like to see a storybook story which recreates the bug that was reported in the issue which included a Combobox and a long list of options which scrolled where the first non-disabled element was past the scrolling region.

snowystinger · 2026-06-25T06:11:47Z

-  getFirstKey(): Key | null {
-    let key = this.collection.getFirstKey();
-    return this.findNextNonDisabled(key, key => this.collection.getKeyAfter(key));
+  getFirstKey(key?: Key, includeDisabled?: boolean): Key | null {


What are these arguments for? From what I can see, key isn't used in this function, and nothing in this PR calls getFirstKey with includeDisabled set.

The only place I see it set is in useSelectableCollection, but it appears to be more of an accident, as it would only "include" if the Ctrl key is pressed, which I don't think we want to be a condition for Home/End as it has to do with selection and something called "global" that determines row vs cell focus

finally, reverting the changes in this file still pass the added tests

You're completely right. I have reverted ListKeyboardDelegate.ts to its original state and kept the check entirely within useSelectableCollection.ts by checking "manager.disabledKeys" directly during the autofocus lifecycle. Thanks for the guidance

snowystinger · 2026-06-25T06:15:34Z

+
      manager.setFocused(true);
-      manager.setFocusedKey(focusedKey);
+      manager.setFocusedKey(focusedKey); // Fires consistently now (even if null)


what does this code comment mean? what is it referencing? when did it not fire?

That inline comment was a leftover trace from a temporary intermediate change. Now I have removed that

snowystinger · 2026-06-25T06:20:05Z


+      // Safety check: If the resolved target item is explicitly disabled,
+      // fallback immediately to the first available enabled option.
+      if (focusedKey && manager.disabledKeys.has(focusedKey)) {


what is this block of code for? all tests pass without it.

Looking at the loop above, this line should make sure that the item isn't disabled. Did you see otherwise? please include a test.

if (manager.canSelectItem(key)) {

snowystinger · 2026-06-25T06:20:26Z

+  // =========================================================================
+  // ADDED: Tests for skipping disabled items automatically on mount/focus
+  // =========================================================================


Suggested change

// =========================================================================

// ADDED: Tests for skipping disabled items automatically on mount/focus

// =========================================================================

Applied the suggestion

snowystinger · 2026-06-25T06:21:41Z

+      // Let any asynchronous layout microtasks complete smoothly
+      await new Promise(resolve => setTimeout(resolve, 0));


What is this about? the test passes without it from what i can see

You're right. I originally included this timeout helper to address a microtask timing race condition during an earlier iteration of the fix. Now I removed that.

snowystinger · 2026-06-25T06:22:01Z

+      // Wrap the simulated user tabbing action cleanly in an async act block
+      // to handle the synchronous JSDOM focus state mutation


Suggested change

// Wrap the simulated user tabbing action cleanly in an async act block

// to handle the synchronous JSDOM focus state mutation

This comment means nothing

Now I have removed that.

snowystinger · 2026-06-25T06:22:49Z

+      manager.setFocusedKey(focusedKey); // Fires consistently now (even if null)

-      // If no default focus key is selected, focus the collection itself.
+      // If no default focus key is selected, focus the collection container itself.


Suggested change

// If no default focus key is selected, focus the collection container itself.

// If no default focus key is selected, focus the collection itself.

Applied the suggestion for the comment

snowystinger · 2026-06-25T06:24:05Z

        navigateToKey(manager.lastSelectedKey ?? delegate.getLastKey?.());
      } else {
-        navigateToKey(manager.firstSelectedKey ?? delegate.getFirstKey?.());
+        let firstKey = manager.firstSelectedKey ?? delegate.getFirstKey?.();


something is wrong the tests, they pass without this change as well

You're right, this block was redundant, so I have removed it and reverted this section.

…verted the listKeyboardDelegate file..

snowystinger · 2026-06-25T22:30:18Z

I'm not sure there is actually a bug to fix anymore. I can't run the original authors codesandbox, and in the repro I made here, https://stackblitz.com/edit/adpc19dw?file=src%2FExample.tsx it's scrolling the first non-disabled item into view. Were you able to reproduce the initial issue?

jsmitrah and others added 4 commits June 19, 2026 20:47

fix: added fix to focused on the first available option on the dropdown.

2982af3

Merge branch 'main' into fix/9239/focus-on-the-first-available-option

4499fcd

fix: updated the test case.

0fa95c8

fix: updated the test case.

716a6de

jsmitrah closed this Jun 19, 2026

jsmitrah reopened this Jun 22, 2026

jsmitrah and others added 4 commits June 22, 2026 12:31

fix: added condition to handle the disabled options.

49627aa

Merge branch 'main' into fix/9239/focus-on-the-first-available-option

1e705e6

Merge branch 'main' into fix/9239/focus-on-the-first-available-option

eb84341

Merge branch 'main' into fix/9239/focus-on-the-first-available-option

44e67b4

snowystinger reviewed Jun 25, 2026

View reviewed changes

jsmitrah added 3 commits June 25, 2026 19:31

fix: modified the condition to make include that while testing and re…

9502c69

…verted the listKeyboardDelegate file..

fix: added condition to avoid infinite loop.

7b72c16

fix: safely bypass disabled keys on autofocus mount

1a97e01

jsmitrah requested a review from snowystinger June 25, 2026 15:05

Merge branch 'main' into fix/9239/focus-on-the-first-available-option

bb7d915

jsmitrah closed this Jun 26, 2026

	// =========================================================================
	// ADDED: Tests for skipping disabled items automatically on mount/focus
	// =========================================================================

		// Let any asynchronous layout microtasks complete smoothly
		await new Promise(resolve => setTimeout(resolve, 0));

		// Wrap the simulated user tabbing action cleanly in an async act block
		// to handle the synchronous JSDOM focus state mutation

	// If no default focus key is selected, focus the collection container itself.
	// If no default focus key is selected, focus the collection itself.

Uh oh!

Conversation

jsmitrah commented Jun 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Pull Request Checklist:

📝 Test Instructions:

🧢 Your Project:

Uh oh!

snowystinger left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jsmitrah Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

snowystinger commented Jun 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jsmitrah commented Jun 19, 2026 •

edited

Loading

jsmitrah Jun 25, 2026 •

edited

Loading