-
Notifications
You must be signed in to change notification settings - Fork 24.9k
fix: Android hitSlop prop TalkBack selection #54199
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
base: main
Are you sure you want to change the base?
fix: Android hitSlop prop TalkBack selection #54199
Conversation
if (ev.isFromSource(android.view.InputDevice.SOURCE_CLASS_POINTER) && | ||
(ev.action == MotionEvent.ACTION_HOVER_ENTER || ev.action == MotionEvent.ACTION_HOVER_MOVE)) { |
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.
Hover actions are not only used by TalkBack, but also by many other platforms (eg VR).
Is there a way to scope this changes more, or ideally, reuse logic from TouchTargetHelper
?
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.
Oh sure that makes sense. I will take another look and see what I can find.
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.
I think the simplest way to do this here was just to check if touch exploration is enabled and keep the rest of the changes, like this: 3550da1.
I looked at TouchTargetHelper
and I see there's methods to do hitSlop checking, which we could reuse, but I think we'd have to make them public? If that works, or you had something else in mind, let me know and I'll keep iterating.
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.
Let's make the change to expose the API - once we have code thats working and not duplicating logic, we can figure out how to reduce visibility changes.
We shouldn't be getting the accessibilitymanager or calling isTouchExplorationEnabled
per frame though, as that will cause us to drop frames.
val accessibilityManager = context.getSystemService(Context.ACCESSIBILITY_SERVICE) as? AccessibilityManager | ||
if (accessibilityManager?.isTouchExplorationEnabled == 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.
This is still pretty expensive to do on a per-input basis (I think it needs to a Binder call)
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.
Ah yeah that makes sense. I've added some internal state to ReactViewGroup
and a TouchExplorationStateChangeListener to manage it.
fix: clean up unncessary bounds checking fix: use listener instead
@javache - thanks again for your time reviewing. I exposed the |
Summary:
Fixes #54187 by intercepting TalkBack hover events inside
dispatchGenericMotionEvent
and checking if the tap falls inside the child's hitSlop area. If the tap occurs in the hitSlop zone, the method now requests accessibility focus on that child view.Changelog:
Pick one each for the category and type tags:
[ANDROID] [FIXED] - fix hitSlop prop not providing full hitSlop boundaries to TalkBack
Test Plan:
You can test this in the "Pressable Hit Slop" segment of the
rn-tester
app on Android.yarn install
yarn android
Screen recording of fixed behavior in this branch
fixed.mov
Screen recording of broken behavior on
main
broken.mov