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

[react-interactions] Add TabFocusContainer and TabbableScope UI components #16732

Merged
merged 5 commits into from
Sep 11, 2019

Conversation

trueadm
Copy link
Contributor

@trueadm trueadm commented Sep 10, 2019

This PR adds two experimental components to be used for internal focus management handling. Furthermore, this PR fixes a bug with scope component refs correctly detaching.

TabFocusContainer contains tabbable keyboard focus to within its children, where tabbing gets wrapped to the beginning.

TabbableScope collects host components that are keyboard tabbable so that TabFocusContainer can handle the tab navigation through said components.

@sizebot
Copy link

sizebot commented Sep 10, 2019

No significant bundle size changes to report.

Generated by 🚫 dangerJS against 6ceff3e

@necolas
Copy link
Contributor

necolas commented Sep 10, 2019

FWIW I still don't like the "tab" nomenclature given that these nodes can be reached by means other than pressing the Tab key on a keyboard.

@trueadm
Copy link
Contributor Author

trueadm commented Sep 10, 2019

@necolas Maybe it should be better named as TabFocusContainer or something then, as this is exclusively for that operation only. Updated: renamed!

@trueadm trueadm changed the title [react-interactions] Add FocusContain and TabbableScope UI components [react-interactions] Add TabFocusContainer and TabbableScope UI components Sep 10, 2019
@necolas
Copy link
Contributor

necolas commented Sep 10, 2019

this is exclusively for that operation only

so this component wouldn't work with e.g. a directional pad on a remote control?

@trueadm
Copy link
Contributor Author

trueadm commented Sep 10, 2019

@necolas that’s right, it would not work with those with this component.

Copy link
Contributor

@necolas necolas left a comment

Choose a reason for hiding this comment

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

We might want to revisit that limitation in the future but this lgtm for now

@trueadm trueadm merged commit ae724be into facebook:master Sep 11, 2019
@trueadm trueadm deleted the focus-components branch September 11, 2019 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants