-
Notifications
You must be signed in to change notification settings - Fork 3.2k
[TimePicker] Fix isInTapRegion check #4766
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: master
Are you sure you want to change the base?
[TimePicker] Fix isInTapRegion check #4766
Conversation
44f0355
to
94ee17f
Compare
Hey @pubiqq, thanks for posting these PRs, can you explain what the change does? Maybe provide a screenshot/video? |
I understand this, could you describe the change in behavior with a screenshot or video? |
|
Hey @pubiqq We we're looking at this and are still not sure of the change's intent. From the video it looks like the hand is snapping back to 5 when it looks like it should be 7. Could you elaborate on what this fixes? |
The hand is snapping back due to the touch slop. It's implemented in the code, but incorrectly, and this PR fixes it. |
I'm running the catalog at head vs with this change. With the change it seems more difficult to select numbers between 5 and 10. Here's a comparison of trying to drag to 6-7-8-9-10-8-6. The video of the catalog that includes the PR snaps back to 5 when I (as a user) don't think it should. Could it be that I release my pointer when I see the minute text field change to the value I'm targeting? I expect that value to stick even if that value isn't technically the closest to where my touch x/y UP event occurred.
|
Yes, it does, but that's a completely different issue that needs to be addressed separately (including for Compose btw). This PR only fixes the incorrect touch distance check. |
No description provided.