-
Notifications
You must be signed in to change notification settings - Fork 47.3k
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 Events: add onFocusVisibleChange to Focus #15516
Conversation
efb1149
to
dcee5f8
Compare
nativeEvent.key === 'Tab' && | ||
!(nativeEvent.metaKey || nativeEvent.altKey || nativeEvent.ctrlKey) | ||
) { | ||
isGlobalFocusVisible = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's probably not ideal to have global focus state being checked / modified in every instance. Should this be handled by a module outside of Flare (using native APIs) that we query for global focus visibility info? Any other ideas?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could add a context.isEventComponentWithinResponderScope
or context.isResponderTopLevel
API that checks if a parent event component in the tree has the same responder and bail-out of this logic?
Details of bundled changes.Comparing: cc5a493...f2776df react-events
Generated by 🚫 dangerJS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is good for now but we need to optimize this better in a follow up.
Called when focus visibility changes. Focus is only considered visible if a focus event occurs after keyboard navigation. This provides a way for people to provide visual focus styles for keyboard accessible UIs without those styles appearing if focus is triggered by mouse, touch, pen.
dcee5f8
to
f2776df
Compare
I noticed a couple of odd things happening while working on–and unrelated to changes in–this branch. First, the |
@necolas This wasn't merged before the other PR that should have addressed the double event issue (the PR for using proper keys for events). I wonder if that fixes this issue. The other bugs sounds like yet another Safari thing we need to handle as an edge case. :/ |
Called when focus visibility changes. Focus is only considered visible if a
focus event occurs after keyboard navigation. This provides a way for people to
provide visual focus styles for keyboard accessible UIs without those styles
appearing if focus is triggered by mouse, touch, pen.
Still working on incorporating a couple more details around when focus is considered visible.
Ref #15257