ngMobile / directive / ngClick.js - Dramatic performance #3296

FelikZ · 2013-07-22T11:36:16Z

I had to stop using mobile ngClick, because of its problems with performance.

In my case, the 300 elements that have nested 3-4 ngClick, while adding new elements, performance (especially in chrome) tend to zero - its not possible to work with the application then. What does not happen if you use a standard ngClick.

revolunet · 2013-07-25T08:26:28Z

could be interesting to have a plunker to play with

bshepherdson · 2013-07-25T15:16:50Z

Interesting. I'm not surprised there were problems here. When you touch to scroll, it will fire touchstart and then dozens or hundreds of touchmoves as you drag, hitting various elements. If they're nested 3-4 deep and densely packed next to each other, I'm not surprised that trying to call hundreds (thousands?) of touchmove callbacks per second is slow, even if they are short.

This doesn't happen with regular ngClick, since it isn't activated until you do click. Something you can try: use normal ngClick (ie. don't load ngMobile) and try fastclick.js. Does it have the same problem? If not, I'll take a look at its code and see if there's a way to make ngMobile faster in this case.

FelikZ · 2013-07-26T07:53:33Z

@shepheb I will try to check fastclick.js on weekends and post feedback.

FelikZ · 2013-08-09T13:50:30Z

@shepheb so the problem is in element.bind('<touch_event_here>') calls in ngMobile ngClick directive definition. I'd commented them and performance become smooth. On each call the performance of binding touch events corrupted incrementally. I trying to search implementation of that method, but no luck. Is that default jQuery's behavior?

bshepherdson · 2013-08-09T14:57:44Z

That doesn't surprise me. The number of calls to element.bind('foo') grows linearly with the number of ng-click="..." directives on the page. I'm not certain, but they should be removed properly when those elements go out of scope. The browser should remove event handlers from any elements that are destroyed when you switch views.

So if you have hundreds of ng-clickable elements on the page, there are going to be hundreds of event handlers created. The difference in performance between normal ngClick and ngMobile's ngClick is that the mobile version needs to handle touchmoves, of which there are very many while scrolling (on iOS at least), but desktop only cares about click events.

Did you try fastclick.js without ngMobile included? I want to know if it suffers from the same problem.

FelikZ · 2013-08-09T15:12:08Z

@shepheb no I doesn't try fastclick.js yet, BUT the issue is definitely in jQuery's bind! I try'ed, for test purposes, to replace touch* events to mouse* events and then there is no performance issues. I'm using jQuery 1.9.2 and then updated it to 1.10.2 but problem is still reproduced. I will try to debug deeply after my vacation.

revolunet · 2013-08-12T09:20:48Z

don't you think the ngClick directive should listen to scope.$destroy and then remove the listener ?

angular.js/src/ng/directive/ngEventDirs.js

Line 43 in 2bb27d4

ngEventDirectives[directiveName] = ['$parse', function($parse) {

FelikZ · 2013-08-26T14:20:40Z

@revolunet, I think it should.

BTW, I spend a few hours to debug deeper jQuery's bind and profiling my project and I'd found the endpoint issue! The problem is in native addEventListener function. There is a topic on stackoverflow. So, its reproduced only at desktop Chrome (Latest 29.0.1547.57 version).

The only thing I came up with is to call addEventListener only on visible to user items and call removeEventListener to not visible items when user's viewport changes.

Also I can't understand why ngMobile.ngClick binds those events even if device does not support touch events. I think it shouldn't bind them.

bshepherdson · 2013-08-26T14:56:10Z

BIzarre. Great work finding the root bug!

We can probably add a touch support check. I had been assuming that including ngMobile, or ngTouch as it's known these days, happens only on touchy devices anyway. But perhaps that was naive; I can add that snippet to check for touch event support before doing anything in ngTouch's ngClick.

FelikZ · 2013-08-26T15:39:19Z

@shepheb it would be awesome. As a quick solution I would rather use Modernizr.touch check before module initialization and do not require ngMobile if no touch was detected.

dehru · 2013-12-23T16:30:41Z

+1

sudhakar · 2013-12-30T07:43:26Z

+1

CWSpear · 2013-12-30T17:20:45Z

+1

akofman · 2014-01-06T11:09:00Z

+1

NathanWalker · 2014-02-19T16:00:07Z

This is an old discussion, but I must add that I've struggled with ngTouch for the better part of the last 6 months. Gone back/forth including it, then not including it, and the adverse affect it has on mobile scrolling makes it completely unacceptable. One good thing at the cost of standard user expectation on mobile makes the library, in its current state, completely worthless, outside of swipe handling, but again all that stuff it does are all nullified by the incredibly suckiness it brings to scrolling on mobile.

revolunet · 2014-02-19T16:39:31Z

yes we should be able to use the ngTouch module for the $swipe service but without the ngClick override so we can replace it with FastClick

This commit removes the ngClick directive from the ngTouch module. The directive was conceived to remove the 300ms delay for click events on mobile browsers, by sending a synthetic click event on touchstart. It also tried to make sure that the original click event that the browser sends after 300ms was "busted", so that no redundant "ghost-clicks" appear. There are various reasons why this feature has been removed: - "This is an ugly, terrible hack!" (says so in the source) - It is plagued by various bugs that are hard to fix / test for all platforms (see below) - Simply including ngTouch activates the ngClick override, which means even if you simply want to use ngSwipe, you may break parts of your app - There exist alternatives for removing the 300ms delay, that can be used very well with Angular: [FastClick](https://github.com/ftlabs/fastclick), [Tappy!](https://github.com/filamentgroup/tappy/) (There's also hammer.js for touch events / gestures) - The 300ms delay itself is on the way out - Chrome and Firefox for Android remove the 300ms delay when the usual `<meta name="viewport" content="width=device-width">` is set. In IE, the `touch-action` css property can be set to `none` or `manipulation` to remove the delay. Finally, since iOs 8, Safari doesn't delay "slow" taps anymore. There are some caveats though, which can be found in this excellent article on which this summary is based: http://developer.telerik.com/featured/300-ms-click-delay-ios-8/ Note that this change does not affect the `ngSwipe` directive. Issues with interactive elements (input, a etc.) when parent element has ngClick: Closes angular#13287 Closes angular#13558 Closes angular#12392 Closes angular#12153 Closes angular#11358 Closes angular#6432 Closes angular#12867 Closes angular#7231 Closes angular#13213 Closes angular#12082 Closes angular#4030 Closes angular#12545 Closes angular#6001 Closes angular#5307 Other issues: - incorrect event order - incorrect event propagation - ghost-clicks / failing clickbusting with corner cases - browser specific bugs - et al. Closes angular#12734 Closes angular#12455 Closes angular#11197 Closes angular#11261 Closes angular#12150 Closes angular#10918 Closes angular#10366 Closes angular#9872 Closes angular#9744 Closes angular#6251 Closes angular#4428 Closes angular#3999 Closes angular#3347 Closes angular#3296 Closes angular#7935 Closes angular#13272 Closes angular#13447 Closes angular#13122 Closes angular#12317 Closes angular#11577 Closes angular#11342 Closes angular#10211 Closes angular#9724 Closes angular#7134 Closes angular#6330 Closes angular#3447 BREAKING CHANGE: The `ngClick` override from the `ngTouch` module has been removed. This means that on touch-based devices, users might experience a 300ms delay before an ngClick event is fired. If you rely on this behavior, consider using [FastClick](https://github.com/ftlabs/fastclick) or [Tappy!](https://github.com/filamentgroup/tappy/). Also note that modern browsers remove the 300ms delay under some circumstances: - Chrome and Firefox for Android remove the 300ms delay when the well-known `<meta name="viewport" content="width=device-width">` is set - Internet Explorer removes the delay when `touch-action` css property is set to `none` or `manipulation` - Since iOs 8, Safari removes the delay on so-called "slow taps" See this article for more info on the topic: http://developer.telerik.com/featured/300-ms-click-delay-ios-8/

This commit removes the ngClick directive from the ngTouch module. The directive was conceived to remove the 300ms delay for click events on mobile browsers, by sending a synthetic click event on touchstart. It also tried to make sure that the original click event that the browser sends after 300ms was "busted", so that no redundant "ghost-clicks" appear. There are various reasons why this feature has been removed: - "This is an ugly, terrible hack!" (says so in the source) - It is plagued by various bugs that are hard to fix / test for all platforms (see below) - Simply including ngTouch activates the ngClick override, which means even if you simply want to use ngSwipe, you may break parts of your app - There exist alternatives for removing the 300ms delay, that can be used very well with Angular: [FastClick](https://github.com/ftlabs/fastclick), [Tappy!](https://github.com/filamentgroup/tappy/) (There's also hammer.js for touch events / gestures) - The 300ms delay itself is on the way out - Chrome and Firefox for Android remove the 300ms delay when the usual `<meta name="viewport" content="width=device-width">` is set. In IE, the `touch-action` css property can be set to `none` or `manipulation` to remove the delay. Finally, since iOs 8, Safari doesn't delay "slow" taps anymore. There are some caveats though, which can be found in this excellent article on which this summary is based: http://developer.telerik.com/featured/300-ms-click-delay-ios-8/ Note that this change does not affect the `ngSwipe` directive. Issues with interactive elements (input, a etc.) when parent element has ngClick: Closes angular#4030 Closes angular#5307 Closes angular#6001 Closes angular#6432 Closes angular#7231 Closes angular#11358 Closes angular#12082 Closes angular#12153 Closes angular#12392 Closes angular#12545 Closes angular#12867 Closes angular#13213 Closes angular#13558 Other issues: - incorrect event order - incorrect event propagation - ghost-clicks / failing clickbusting with corner cases - browser specific bugs - et al. Closes angular#3296 Closes angular#3347 Closes angular#3447 Closes angular#3999 Closes angular#4428 Closes angular#6251 Closes angular#6330 Closes angular#7134 Closes angular#7935 Closes angular#9724 Closes angular#9744 Closes angular#9872 Closes angular#10211 Closes angular#10366 Closes angular#10918 Closes angular#11197 Closes angular#11261 Closes angular#11342 Closes angular#11577 Closes angular#12150 Closes angular#12317 Closes angular#12455 Closes angular#12734 Closes angular#13122 Closes angular#13272 Closes angular#13447 BREAKING CHANGE: The `ngClick` override directive from the `ngTouch` module has been removed. This means that on touch-based devices, users might now experience a 300ms delay before a click event is fired. If you rely on this behavior, consider using [FastClick](https://github.com/ftlabs/fastclick) or [Tappy!](https://github.com/filamentgroup/tappy/). Also note that modern browsers remove the 300ms delay under some circumstances: - Chrome and Firefox for Android remove the 300ms delay when the well-known `<meta name="viewport" content="width=device-width">` is set - Internet Explorer removes the delay when `touch-action` css property is set to `none` or `manipulation` - Since iOs 8, Safari removes the delay on so-called "slow taps" See this article for more info on the topic: http://developer.telerik.com/featured/300-ms-click-delay-ios-8/

…ault This commit deprecates the ngClick directive from the ngTouch module. Additionally, it disables it by default. It can be enabled in the new $touchProvider with $touchProvider.ngClickOverrideEnabled() method. The directive was conceived to remove the 300ms delay for click events on mobile browsers, by sending a synthetic click event on touchstart. It also tried to make sure that the original click event that the browser sends after 300ms was "busted", so that no redundant "ghost-clicks" appear. There are various reasons why the directive is being deprecated. - "This is an ugly, terrible hack!" (says so in the source) - It is plagued by various bugs that are hard to fix / test for all platforms (see below) - Simply including ngTouch activates the ngClick override, which means even if you simply want to use ngSwipe, you may break parts of your app - There exist alternatives for removing the 300ms delay, that can be used very well with Angular: [FastClick](https://github.com/ftlabs/fastclick), [Tappy!](https://github.com/filamentgroup/tappy/) (There's also hammer.js for touch events / gestures) - The 300ms delay itself is on the way out - Chrome and Firefox for Android remove the 300ms delay when the usual `<meta name="viewport" content="width=device-width">` is set. In IE, the `touch-action` css property can be set to `none` or `manipulation` to remove the delay. Finally, since iOs 8, Safari doesn't delay "slow" taps anymore. There are some caveats though, which can be found in this excellent article on which this summary is based: http://developer.telerik.com/featured/300-ms-click-delay-ios-8/ Note that this change does not affect the `ngSwipe` directive. Issues with interactive elements (input, a etc.) when parent element has ngClick: Closes angular#4030 Closes angular#5307 Closes angular#6001 Closes angular#6432 Closes angular#7231 Closes angular#11358 Closes angular#12082 Closes angular#12153 Closes angular#12392 Closes angular#12545 Closes angular#12867 Closes angular#13213 Closes angular#13558 Other issues: - incorrect event order - incorrect event propagation - ghost-clicks / failing clickbusting with corner cases - browser specific bugs - et al. Closes angular#3296 Closes angular#3347 Closes angular#3447 Closes angular#3999 Closes angular#4428 Closes angular#6251 Closes angular#6330 Closes angular#7134 Closes angular#7935 Closes angular#9724 Closes angular#9744 Closes angular#9872 Closes angular#10211 Closes angular#10366 Closes angular#10918 Closes angular#11197 Closes angular#11261 Closes angular#11342 Closes angular#11577 Closes angular#12150 Closes angular#12317 Closes angular#12455 Closes angular#12734 Closes angular#13122 Closes angular#13272 Closes angular#13447 BREAKING CHANGE: The `ngClick` override directive from the `ngTouch` module is **deprecated and disabled by default**. This means that on touch-based devices, users might now experience a 300ms delay before a click event is fired. If you rely on this directive, you can still enable it with the `$touchProvider.ngClickOverrideEnabled()`method: ```js angular.module('myApp').config(function($touchProvider) { $touchProvider.ngClickOverrideEnabled(true); }); ``` For migration, we recommend using [FastClick](https://github.com/ftlabs/fastclick). Also note that modern browsers remove the 300ms delay under some circumstances: - Chrome and Firefox for Android remove the 300ms delay when the well-known `<meta name="viewport" content="width=device-width">` is set - Internet Explorer removes the delay when `touch-action` css property is set to `none` or `manipulation` - Since iOs 8, Safari removes the delay on so-called "slow taps" See this [article by Telerik](http://developer.telerik.com/featured/300-ms-click-delay-ios-8/) for more info on the topic. Note that this change does not affect the `ngSwipe` directive.

This commit deprecates the ngClick directive from the ngTouch module. Additionally, it disables it by default. It can be enabled in the new $touchProvider with $touchProvider.ngClickOverrideEnabled() method. The directive was conceived to remove the 300ms delay for click events on mobile browsers, by sending a synthetic click event on touchstart. It also tried to make sure that the original click event that the browser sends after 300ms was "busted", so that no redundant "ghost-clicks" appear. There are various reasons why the directive is being deprecated. - "This is an ugly, terrible hack!" (says so in the source) - It is plagued by various bugs that are hard to fix / test for all platforms (see below) - Simply including ngTouch activates the ngClick override, which means even if you simply want to use ngSwipe, you may break parts of your app - There exist alternatives for removing the 300ms delay, that can be used very well with Angular: [FastClick](https://github.com/ftlabs/fastclick), [Tappy!](https://github.com/filamentgroup/tappy/) (There's also hammer.js for touch events / gestures) - The 300ms delay itself is on the way out - Chrome and Firefox for Android remove the 300ms delay when the usual `<meta name="viewport" content="width=device-width">` is set. In IE, the `touch-action` css property can be set to `none` or `manipulation` to remove the delay. Finally, since iOs 8, Safari doesn't delay "slow" taps anymore. There are some caveats though, which can be found in this excellent article on which this summary is based: http://developer.telerik.com/featured/300-ms-click-delay-ios-8/ Note that this change does not affect the `ngSwipe` directive. Issues with interactive elements (input, a etc.) when parent element has ngClick: Closes angular#4030 Closes angular#5307 Closes angular#6001 Closes angular#6432 Closes angular#7231 Closes angular#11358 Closes angular#12082 Closes angular#12153 Closes angular#12392 Closes angular#12545 Closes angular#12867 Closes angular#13213 Closes angular#13558 Other issues: - incorrect event order - incorrect event propagation - ghost-clicks / failing clickbusting with corner cases - browser specific bugs - et al. Closes angular#3296 Closes angular#3347 Closes angular#3447 Closes angular#3999 Closes angular#4428 Closes angular#6251 Closes angular#6330 Closes angular#7134 Closes angular#7935 Closes angular#9724 Closes angular#9744 Closes angular#9872 Closes angular#10211 Closes angular#10366 Closes angular#10918 Closes angular#11197 Closes angular#11261 Closes angular#11342 Closes angular#11577 Closes angular#12150 Closes angular#12317 Closes angular#12455 Closes angular#12734 Closes angular#13122 Closes angular#13272 Closes angular#13447 BREAKING CHANGE: The `ngClick` override directive from the `ngTouch` module is **deprecated and disabled by default**. This means that on touch-based devices, users might now experience a 300ms delay before a click event is fired. If you rely on this directive, you can still enable it with the `$touchProvider.ngClickOverrideEnabled()`method: ```js angular.module('myApp').config(function($touchProvider) { $touchProvider.ngClickOverrideEnabled(true); }); ``` For migration, we recommend using [FastClick](https://github.com/ftlabs/fastclick). Also note that modern browsers remove the 300ms delay under some circumstances: - Chrome and Firefox for Android remove the 300ms delay when the well-known `<meta name="viewport" content="width=device-width">` is set - Internet Explorer removes the delay when `touch-action` css property is set to `none` or `manipulation` - Since iOs 8, Safari removes the delay on so-called "slow taps" See this [article by Telerik](http://developer.telerik.com/featured/300-ms-click-delay-ios-8/) for more info on the topic. Note that this change does not affect the `ngSwipe` directive.

ghost assigned bshepherdson Dec 19, 2013

btford removed the gh: issue label Aug 20, 2014

IgorMinar modified the milestones: Backlog, 1.3.0 Oct 1, 2014

petebacondarwin modified the milestones: Backlog, ngTouch Overhaul Jan 26, 2016

Narretz mentioned this issue Jan 26, 2016

fix(ngTouch): remove ngClick override #13852

Closed

Narretz mentioned this issue Jan 27, 2016

fix(ngTouch): deprecate ngClick override directive, disable it by def… #13857

Closed

Narretz closed this as completed in 0dfc1df Jan 27, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ngMobile / directive / ngClick.js - Dramatic performance #3296

ngMobile / directive / ngClick.js - Dramatic performance #3296

FelikZ commented Jul 22, 2013

revolunet commented Jul 25, 2013

bshepherdson commented Jul 25, 2013

FelikZ commented Jul 26, 2013

FelikZ commented Aug 9, 2013

bshepherdson commented Aug 9, 2013

FelikZ commented Aug 9, 2013

revolunet commented Aug 12, 2013

FelikZ commented Aug 26, 2013

bshepherdson commented Aug 26, 2013

FelikZ commented Aug 26, 2013

dehru commented Dec 23, 2013

sudhakar commented Dec 30, 2013

CWSpear commented Dec 30, 2013

akofman commented Jan 6, 2014

NathanWalker commented Feb 19, 2014

revolunet commented Feb 19, 2014

ngMobile / directive / ngClick.js - Dramatic performance #3296

ngMobile / directive / ngClick.js - Dramatic performance #3296

Comments

FelikZ commented Jul 22, 2013

revolunet commented Jul 25, 2013

bshepherdson commented Jul 25, 2013

FelikZ commented Jul 26, 2013

FelikZ commented Aug 9, 2013

bshepherdson commented Aug 9, 2013

FelikZ commented Aug 9, 2013

revolunet commented Aug 12, 2013

FelikZ commented Aug 26, 2013

bshepherdson commented Aug 26, 2013

FelikZ commented Aug 26, 2013

dehru commented Dec 23, 2013

sudhakar commented Dec 30, 2013

CWSpear commented Dec 30, 2013

akofman commented Jan 6, 2014

NathanWalker commented Feb 19, 2014

revolunet commented Feb 19, 2014