-
Notifications
You must be signed in to change notification settings - Fork 27.4k
fix(ngTouch): fix ghost click on small touch delta #6995
Conversation
Thanks for the PR! Please check the items below to help us merge this faster. See the contributing docs for more information.
If you need to make changes to your pull request, you can update the commit with Thanks again for your help! |
ngClick was leaking ghost click events when the touch Startmoved a small distance. This is because if there was a touchMove event between the touchStart and touchEnd preventGhostClick would not be called. This also caused the MOVE_TOLERANCE threshold to be ignored because any change in position would stop preventGhostClick from being run. Cancelling a tap is now correctly based on the MOVE_TOLERANCE, and ghost clicks are not leaked. Closes angular#6251
I'm sorry, but I wasn't able to verify your Contributor License Agreement (CLA) signature. CLA signature is required for any code contributions to AngularJS. Please sign our CLA and ensure that the CLA signature email address and the email address in this PR's commits match. If you signed the CLA as a corporation, please let us know the company's name. Thanks a bunch! PS: If you signed the CLA in the past then most likely the email addresses don't match. Please sign the CLA again or update the email address in the commit of this PR. |
1 similar comment
I'm sorry, but I wasn't able to verify your Contributor License Agreement (CLA) signature. CLA signature is required for any code contributions to AngularJS. Please sign our CLA and ensure that the CLA signature email address and the email address in this PR's commits match. If you signed the CLA as a corporation, please let us know the company's name. Thanks a bunch! PS: If you signed the CLA in the past then most likely the email addresses don't match. Please sign the CLA again or update the email address in the commit of this PR. |
CLA signature verified! Thank you! Someone from the team will now triage your PR and it will be processed based on the determined priority (doc updates and fixes with tests are prioritized over other changes). |
CLA signature verified! Thank you! Someone from the team will now triage your PR and it will be processed based on the determined priority (doc updates and fixes with tests are prioritized over other changes). |
This is an important bug fix. Without it apps that require behavior like the given plnkr do not work. We've been using a forked version of angular with this change to make our game function correctly. |
This fix was/is super useful for internal projects. Would love to see it merged. |
02dc2aa
to
fd2d6c0
Compare
cad9560
to
f294244
Compare
e8dc429
to
e83fab9
Compare
4dd5a20
to
998c61c
Compare
Someone wrote to me asking why I closed this PR, here is my response in case anyone else is curious. The PR was open for over 1 year and didn't receive any attention from any maintainers of Angular. I think that whoever was originally in charge of the ngTouch module is no longer actively involved in Angular, and none of the other maintainers feel comfortable enough with the module to merge in the PR (though the specs pass, and the change makes sense). I saw that the PR had a merge conflict, and I don't feel it's worth the time to resolve it just to have the PR sit open for another year. It's unfortunate that the fix wasn't merged, I've seen how many people are affected by this problem in the issues(#6251). At Uken Games (where I work) we've been running with this patch in prod for over a year because without it our game has a number of bugs caused by click events firing twice. If you are feeling adventurous, feel free to make the PR merge-able and try to get it in :) |
Request Type: bug
How to reproduce: http://plnkr.co/w1q2AJ
Small movements of your finger or touch device when tapping causes both a touchEnd and Click event to trigger the ngClick callback causing it to fire twice. Rapidly tap the numbers from bottom to top to observe this happening occasionally. By tapping rapidly in an upward directions it's easier to get your finger to skid a little. Try out this plunker on a mobile device or with touch emulation enabled.
Component(s): ngTouch
Impact: medium
Complexity: small
This issue is related to:
Detailed Description:
ngClick was leaking ghost click events when a touch moved a small
distance. This is because if there was a touchMove event between the
touchStart and touchEnd preventGhostClick would not be called. This also
caused the MOVE_TOLERANCE threshold to be ignored because any change in
position would stop preventGhostClick from being run.
Cancelling a tap is now correctly based on the MOVE_TOLERANCE, and ghost
clicks are not leaked.
Other Comments:
Closes #6251