Skip to content

Fix Select keyboard focus trap#7103

Closed
snowystinger wants to merge 5 commits into
mainfrom
fix-select-keyboard-focus-trap
Closed

Fix Select keyboard focus trap#7103
snowystinger wants to merge 5 commits into
mainfrom
fix-select-keyboard-focus-trap

Conversation

@snowystinger
Copy link
Copy Markdown
Member

@snowystinger snowystinger commented Sep 27, 2024

Closes #7019

Couple assumptions. The only way to focus the hidden select is via keyboard tab/shift+tab. If focus is coming from the document.body (such as if the user clicks near the Select, then hits tab), then focus should just go directly to the Select, not the extra button. We can't tell what the intent was and the behavior of a click followed by tab differs across browsers anyways.

✅ 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:

🧢 Your Project:

@rspbot
Copy link
Copy Markdown

rspbot commented Sep 27, 2024

@devongovett devongovett force-pushed the main branch 2 times, most recently from a79adcf to 3013156 Compare September 30, 2024 21:00
@snowystinger snowystinger force-pushed the fix-select-keyboard-focus-trap branch from c08096a to 3503d62 Compare October 14, 2024 06:06
@rspbot
Copy link
Copy Markdown

rspbot commented Oct 14, 2024

@rspbot
Copy link
Copy Markdown

rspbot commented Oct 14, 2024

@rspbot
Copy link
Copy Markdown

rspbot commented Oct 14, 2024

} else {
triggerRef.current.focus();
}
},
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Wonder if the above comment still applies, or whether we could make the input have tabIndex={-1} now. This is somewhat complicated...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'll make another branch we can test that on, because, agreed, this is complicated

@snowystinger
Copy link
Copy Markdown
Member Author

Closing in favor of #7200

@snowystinger
Copy link
Copy Markdown
Member Author

Reopening because the other option may break android

@snowystinger snowystinger reopened this Oct 17, 2024
@snowystinger
Copy link
Copy Markdown
Member Author

Closing in favor of #7200

@snowystinger snowystinger deleted the fix-select-keyboard-focus-trap branch October 24, 2024 21:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Select with additional buttons traps focus

3 participants