Skip to content
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.

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

Fix Select keyboard focus trap #7103

Closed
wants to merge 5 commits into from

Conversation

snowystinger
Copy link
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

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

rspbot commented Oct 14, 2024

@rspbot
Copy link

rspbot commented Oct 14, 2024

@rspbot
Copy link

rspbot commented Oct 14, 2024

} else {
triggerRef.current.focus();
}
},
Copy link
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
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
Member Author

Closing in favor of #7200

@snowystinger
Copy link
Member Author

Reopening because the other option may break android

@snowystinger snowystinger reopened this Oct 17, 2024
@snowystinger
Copy link
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