-
Notifications
You must be signed in to change notification settings - Fork 27.5k
Input click triggered by label click immediately following a touch event is busted #6302
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! |
I love the tests! I need you to get this passing jshint, though, so that we can make sure the tests are passing on all of the supported browsers. Thanks~ |
Updated |
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. |
I've just submitted the CLA as a corporation, Aligned Software Solutions Inc. Cheers |
@@ -370,40 +370,128 @@ describe('ngClick (touch)', function() { | |||
})); | |||
|
|||
|
|||
it('should not cancel clicks that come long after', inject(function($rootScope, $compile) { | |||
element1 = $compile('<div ng-click="count = count + 1"></div>')($rootScope); | |||
describe('when clicking on label immediately following a touch event on other element', function() { |
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 description is really long and hard to understand, I think
Okay, so the number of tests is pretty good, but ideally we could do this better. The beforeEach is doing a scary amount of work, and this means that extending this suite will be more difficult later on, and it's not clear what each test is doing. I don't have a lot to say about the implementation, though. Unfortunately I don't have a device to verify this at the moment |
lastLabelClickCoordinates = null; | ||
} | ||
// remember label click coordinates to prevent click busting of trigger click event on input | ||
if (event.target.tagName.toLowerCase() === 'label') { |
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 seems oddly specific, is this the only situation that we'd care about?
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.
AFAICT yes
I like the fix. The test needs more love. Can you just fix up the tests (remove console.log and wrap the long line) and it should be good to be merged. |
I verified the CLA |
Fix click busting of input click triggered by a label click quickly following a touch event on a different element, in desktop and mobile WebKit To reproduce the issue fixed by this commit set up a page with - an element with ng-click - a radio button (with hg-model) and associated label In a quick sequence tap on the element and then on the label. The radio button will not be checked, unless PREVENT_DURATION has passed Fixes #6302
Hope the tests read better now. |
the description is still really long winded, but I can't really think of anything better. I'm going to merge it. |
Fix click busting of input click triggered by a label click quickly following a touch event on a different element, in desktop and mobile WebKit To reproduce the issue fixed by this commit set up a page with - an element with ng-click - a radio button (with hg-model) and associated label In a quick sequence tap on the element and then on the label. The radio button will not be checked, unless PREVENT_DURATION has passed Closes angular#6302
Fix click busting of input click triggered by a label click quickly following a touch event on a different element, in desktop and mobile WebKit To reproduce the issue fixed by this commit set up a page with - an element with ng-click - a radio button (with hg-model) and associated label In a quick sequence tap on the element and then on the label. The radio button will not be checked, unless PREVENT_DURATION has passed Closes #6302
Fix click busting of input click triggered by a label click quickly following a touch event on a different element, in desktop and mobile WebKit To reproduce the issue fixed by this commit set up a page with - an element with ng-click - a radio button (with hg-model) and associated label In a quick sequence tap on the element and then on the label. The radio button will not be checked, unless PREVENT_DURATION has passed Closes angular#6302
To reproduce
In a quick sequence tap on the element and then on the label.
The radio button will not be checked, unless PREVENT_DURATION has passed