fix(ngMobile): prevent ngClick when item disabled

[revolunet]

• the ngClick attribute was always triggered, regardless the ngDisabled/disabled attributes
• we now check the ngDisabled status before triggering the original click event
• and also check the DOM disabled attribute to be homogenous with the regular HTML disabled behavior (disable presence makes the element and click disabled)
• deals with ngMobile's overridden ngClick directive ignores the disabled attribute of a button element #3124 ngMobile's overridden ngClick directive fails to bust the click event #3132 /cc @shepheb

[revolunet]

sample plnkr with fix inside : http://plnkr.co/edit/x0ErHC

[davgothic]

Firstly after looking at the PR, I'm not sure this fixes the #3132 issue.

I tested the plunker you provided with the iPhone 6.1 Simulator and this does prevent the event from firing as expected, but it doesn't trigger the alert for any of the buttons, where I believe it should for:

ng-click + ng-disabled="false"
ng-click + ng-disabled=""
ng-click + ng-disabled

I've not had a chance to test on the actual device yet, but I see not reason why the results should differ.

[revolunet]

ok this PR needs some more work. closing it right now :/

[revolunet]

@davgothic please help me resolve this.

Ive updated the plnkr : http://run.plnkr.co/plunks/x0ErHC/ works ok for me

There was a CLICKBUSTER_THRESHOLD in the code which was causing some issues (touchend+click triggered twice or click only on the edge...) and i cant get how this can help, so i set it to 0 and now its works better imho. maybe @shepheb or @jeffbcross could shed some lights here ?

[davgothic]

I've just tested the revised plunk in the iPhone 6.1 and Android 4.2.2 Simulators and it seems the previous issues are now fixed and all buttons appear to behave as expected.

I don't know the ngMobile module to well so I've no idea if setting CLICKBUSTER_THRESHOLD to 0 will have any undesired effects.

Thanks @revolunet

[jeffbcross]

My assumption is that the CLICKBUSTER_THRESHOLD is used to distinguish between a tap and a click on mobile (since double tap is used frequently in mobile apps to zoom). But that's without diving into the code :). ngMobile is very early and very likely to change.

[revolunet]

Would be great if the original author could give a hand here. This buggy
disabled state is embarrasing ;)
Le 8 juil. 2013 17:43, "Jeff Cross" notifications@github.com a écrit :

My assumption is that the CLICKBUSTER_THRESHOLD is used to distinguish
between a tap and a click on mobile (since double tap is used frequently in
mobile apps to zoom). But that's without diving into the code :). ngMobile
is very early and very likely to change.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/3137#issuecomment-20614492
.

[bshepherdson]

Jeff's guess at CLICKBUSTER_THRESHOLD is incorrect. The intention of the CLICKBUSTER_THRESHOLD is to block clicks from being busted if they're farther than 25px from the touchstart and touchend. This is because some platforms (eg. iOS) sometimes adjust the location of the click slightly away from the touch itself, trying to find nearby buttons and make them easier to tap with a clumsy fingertip. Therefore if we insist on the click coming exactly where the touchstart and touchend were, sometimes we'll fail to prevent a click that we should have blocked.

@revolunet can you expand on what was going wrong with the original value of 25? I don't understand what issues you were seeing.

The disabling part of this PR looks good to me.

[revolunet]

Thanks @shepheb cool to see you back here :)

When you have CLICKBUSTER_THRESHOLD at 25px, the click event is fired twice if you click 25px around the button. (like on the edged)

[revolunet]

Another question : should'nt we trigger angular.element(tapElement).triggerHandler('ngClick', event); instead of executing the ngClick attribute directly ?
This would allow other ngClick overrides

[bshepherdson]

In answer to your question, that's a dupe of bug #3218 and PR #3219 fixes it. I'm hoping to merge it today.

I'm investigating the CLICKBUSTER_THRESHOLD problems. What device(s) are you seeing and not seeing this on?

[revolunet]

Here's an example for iPad : http://plnkr.co/edit/2nGH8g?p=preview
This use the original ngMobile code.

• When you tap on the first button, the event is sometimes (rarely) fired twice (one touchend then one click)
• The disabled state only disable clicks, not touchends

[IgorMinar]

the part of the PR that handles disabled elements has landed as e034024

[bshepherdson]

I'm closing this. @revolunet feel free to open another bug/PR about the CLICKBUSTER_THRESHOLD problem. Please include which devices you see the problem on. I couldn't reproduce this today on Nexus 10 or HTC One (Chrome).