angular / angular.js

AngularJS - HTML enhanced for web apps!
https://angularjs.org
MIT License
58.91k stars 27.55k forks source link

ngClick using ngTouch fires twice #6251

Closed coma closed 8 years ago

coma commented 10 years ago

ngTouch 1.2.1 on Safari iOS 7 fires a second time by tapping wherever on the screen:

http://jsfiddle.net/coma/2hWWa/embedded/result/

http://stackoverflow.com/questions/21708730/why-are-multiple-click-events-fired-when-using-ngtouch/21766071#21766071

pre commented 10 years ago

I am also having hard times with this issue.

Arkkimaagi commented 10 years ago

Any ideas what's causing this, and if there's a workaround that I could use?

coma commented 10 years ago

I think it's something related to the event propagation, this is my workaround:

http://jsfiddle.net/coma/2hWWa

tuomaslahti commented 10 years ago

I have a similar problem.

My app has a popup dialog with a button that closes it. When the dialog's button is on top of a text input element, clicking the button on touch screen devices not only closes the dialog, but also focuses the input.

TomiHiltunen commented 10 years ago

Same problem here using angular w/ angular-touch 1.2.9.

There is no other clickable elements in the tree to propagate to. The very same click event is fired twice in a row. This does not happen with the swipes. Also, the problem of firing ngClick events twice disappears when I remove ngTouch from the project.

richardbrammer commented 10 years ago

I am also having this problem in ngTouch 1.2.13,

Depending on the situation, this is more than an optical bug, but in my case leads to rather annoying usability problems:

For example I change a View with activated ngTouch and with ng-click on a touch device for to open a modal, which lays it self above the clicked element. Within this modal a clickable element is at the same place as the previously clicked element (now laying above), then it could occur that the second event is fired and the button in modal is clicked/fired.

This could lead to unwanted behaviour of the application.

pre commented 10 years ago

Hi Richard, this is exactly same same kind of problem we are facing.

ernestoaparicio commented 9 years ago

Reference jquery (before angular.js) to stop the event propagation and prevent the double fires.

likesjx commented 9 years ago

That doesn't sound like a fix. We do not use jQuery in our application. Is this string saying that this will not be addressed prior to 1.3?

getvictor commented 9 years ago

This issue is making ngClick unusable. How are people working around this? Debounce? Fallback to original ngClick?

I added ngTouch to get swipe functionality, and now I'm saddled with this issue.

jayvan commented 9 years ago

We've been running our own fork of ng-touch with the changes in https://github.com/angular/angular.js/pull/6995 Other than that, I've seen projects doing manual debouncing where clicks within ~200ms of each other are disregarded.

jkingsbery commented 9 years ago

I'm running into this issue too. @coma's workaround seems to have fixed it.

visnup commented 9 years ago

I believe this same issue is fixed in fastclick already: issue and fix.

louiswilbrink commented 9 years ago

just started using angular-material and ran into this issue since material includes aria.

cstephe commented 9 years ago

+1 using material as well having the same issue.

bobbajs commented 9 years ago

any updates for this issue? and is there a workaround? I am using ngtouch 1.3.14 and still having this issue

eclipse1985 commented 9 years ago

+1

rahul0705 commented 9 years ago

Any update on this issue? Developing an app using angular-material and it works great on desktop but on mobile ng-click registers twice.

dominikstrasser commented 9 years ago

In my case jquery (as a dependency of ui-calendar) caused this problem. couldn´t find a fix...

lreindl commented 9 years ago

+1

NorikDavtian commented 9 years ago

+1

NorikDavtian commented 9 years ago

With above 2 commits, added some sort of debounce effect.. The threshold amount set fixes my ngTouch firing twice issues, please test and see if it also worked in your case..

mobilabgit commented 9 years ago

On GenyMotion on an Android 4.1 emulator I always get event.timeStamp undefined => no bounce occurs.

NorikDavtian commented 9 years ago

@mobilabgit Are you suggesting manually creating event timeStamps via new Date() is more environment proof than event.timeStamp .. ??

mobilabgit commented 9 years ago

not suggesting anything, just stating the fact that in the stated configuration the event.timeStamp returns undefined.

jgodi commented 9 years ago

Any update on this? This is causing havok in my application. Menus and buttons are firing clicks twice.

rexebin commented 9 years ago

I am suffering from this problem as well. button's fire ng-click twice.. make the application's data input almost unusable on ios, very frustrating indeed.

NorikDavtian commented 9 years ago

@mobilabgit @jgodi @rexebin Are you guys using Angular Material by any chance??

a14m commented 9 years ago

@NorikDavtian yes, and the good news is that they solved the issue on their master branch.. you can check these resources for more info on the issue https://github.com/angular/angular.js/pull/10766 and this codepen demonstrates the different versions... you can try to combine them to know what is working http://codepen.io/marcysutton/pen/YPxrLd

for example ngMaterial 0.7.1 with ngAria < 2.1.5 doesn't have the bug... waiting for ngMaterial > 0.8.3 ( currently on master ) which have fixed the bug also

rexebin commented 9 years ago

I have solved the problem by removing material design 0.83. 

I also removed bootstrap.js and in its place using bootstrap ui jus only.  as a result reversing back to angular a 1.2.28. Just in case bootstrap ui does not work somewhere with 1.3.x. Unknown bugs are scary.

I hated the bug so much that I also removed ng-touch too.

Now I do have 300ms delay on clicks, but it feels solid stable, I rate stable over the performance panaty by the delay.

The iOS Safari sub debug is not the best debug experice I am afraid. The bug costs me 3 days of work. Forced me to refactor me code, changing all material design element to the good old bootstrap setup. Beta is indeed not intended for production use.

Hope the above saved someone some time.

— Sent from Mailbox

On Fri, Mar 27, 2015 at 1:13 AM, Ahmed Abdel Razzak notifications@github.com wrote:

@NorikDavtian yes, and the good news is that they solved the issue on their master branch.. you can check these resources for more info on the issue https://github.com/angular/angular.js/pull/10766 and this codepen demonstrates the different versions... you can try to combine them to know what is working http://codepen.io/marcysutton/pen/YPxrLd for example the master branch on ngMaterial fine works and ngMaterial 0.7 with ngAria < 2.1.5 doesn't have the bug...

waiting for ngMaterial > 8.3 ( currently on master ) which have fixed the bug also

Reply to this email directly or view it on GitHub: https://github.com/angular/angular.js/issues/6251#issuecomment-86781164

NorikDavtian commented 9 years ago

Thanks @artmees .. Solving root cause of the problem was a better solution than adding extra checks in ng touch .. Everything working now.

Thanks @rexebin for the response.. I can totally relate, but we are not in production phase, so can tolerate few bugs here and there. But starting to be more selective with app dependency updates from this point on.

dangros commented 9 years ago

I'm new to angular and ran into this issue today. I just pulled via bower angular-touch. So I have the latest and it's not fixed there. Norik, can you clarify what I must do in order to get this working natively?

NorikDavtian commented 9 years ago

@dangros It's not the Angular Touch.. as @artmees mentioned it's the Angular Material library more specifically accessibility parts related to Aria..

Change your bower angular-material to

"angular-material": "master" or if you want to make sure your build will not break change it to current masters latest commit version.. Make sure to change to a stable release when the fix is merged into a new tag, else you will always be using master, which might cause other problems down the line..

Good luck.

dangros commented 9 years ago

I'm not using angular-material. Only angular-touch

NorikDavtian commented 9 years ago

Make these changes to your ng-touch file.. your bower version might look slightly different but the content of it should look the same. https://github.com/NorikDavtian/angular.js/blob/master/src/ngTouch/directive/ngClick.js

Let me know if it works.. If your problem persist I would be glad to do a screen session to diagnose your problem.

rexebin commented 9 years ago

Thanks @NorikDavtian. I loved Material Design for its grid layout system, directives, services and ease of use and its design philosophy. I also loved the idea to get rid of the 300ms delay. I will have a play with a new project again and see how it goes, but not with my main project as it has a strict timeline.

Please correct me if I am wrong: the 300ms delay will exist wether we use ng-touch or material design or not. ng-touch and material design will send click straight away and hijack and stop the send click send by the browser. By making amendment to ngClick.js in ng-touch or pull the master branch of material design with ngclick fixes, I suppose it also hijack the normal hyper links so the hyper links do not double click.

NorikDavtian commented 9 years ago

@rexebin try using https://github.com/ftlabs/fastclick to remove the 300ms delay if opting out of the ng-touch, want it or not want it if you want a super fast click you have to alter the native browser behavior, otherwise you are stuck with that delay.

rexebin commented 9 years ago

I tried fastclick. By using it, all ng-clicks will have to have special class on it to work properly. Otherwise it might click twice instantly and then two more 300ms later hopefully stopped by fastclick and ng-touch etc.

Sounds crazy. Is worth to go all the trouble to stop the 300ms by design on touch screens? I guess that depends..

— Sent from Mailbox

On Sat, Mar 28, 2015 at 10:19 AM, Norik Davtian notifications@github.com wrote:

@rexebin try using https://github.com/ftlabs/fastclick to remove the 300ms delay if opting out of the ng-touch, want it or not want it if you want a super fast click you have to alter the native browser behavior, otherwise you are stuck with that delay.

Reply to this email directly or view it on GitHub: https://github.com/angular/angular.js/issues/6251#issuecomment-87200661

marcysutton commented 9 years ago

Hi all, this issue predates ngAria and Angular Material. There were ng-click issues with ngAria in the past, but they should be fixed now. Angular Material is being updated to pull in angular-aria version 1.3.15, so that specific bug will go away. But considering this issue was opened in February 2014, any issues you may have had with Angular Material are unrelated.

parliament718 commented 8 years ago

This bug is also causing alot of havock in my application and I really don't want to resort to custom my-click directive for the most basic thing. I'm not using Angular Material libs.. only ng-touch. And I've just upgraded everything to angular 1.4 angular-touch 1.4 and very sad to see this bug affecting so many wasn't fixed in a milestone release :(

esetnik commented 8 years ago

I'm also affected by the same issue:

bower deps:

  "dependencies": {
    "angular-animate": "1.3.16",
    "angular-cookies": "1.3.16",
    "angular-touch": "1.3.16",
    "angular-sanitize": "1.3.16",
    "jquery": "2.1.4",
    "angular-resource": "1.3.16",
    "angular-ui-router": "0.2.15",
    "angular-material": "0.8.3",
    "angular": "1.3.16",
    "angular-leaflet-directive": "0.8.2",
    "angular-moment": "0.10.1",
    "ngDialog": "0.4.0",
    "angular-strap": "2.2.4",
    "bootstrap": "3.3.4",
    "angular-carousel": "0.3.10",
    "lodash": "3.9.3",
    "angular-validation-match": "1.5.0",
    "ng-file-upload": "5.0.1",
    "angular-sweetalert": "1.1.0",
    "Leaflet.awesome-markers": "2.0.2",
    "angulartics": "0.18.0"
  },
  "devDependencies": {
    "angular-mocks": "1.3.16"
  },
  "resolutions": {
    "angular": "~1.3.x"
  }
tcarlsen commented 8 years ago

Just ran into this myself and im hating it, hope this will be fixed soon.

My quick fix is to make ng-clickuse a scoped function and make that do a double click check like this:

doubleClickCheck = false

$scope.toggleShowPct = ->
  if !doubleClickCheck
    $scope.showPct = !$scope.showPct
    doubleClickCheck = true

    $timeout ->
      doubleClickCheck = false
    , 100
jbeuckm commented 8 years ago

@coma Thanks for your working workaround. I am using this with menu items that use the attribute directive "menu-close". How can I maintain that functionality with your workaround?

coma commented 8 years ago

Hey @jbeuckm! I don't know that directive, is something coming from some bootstrap/material/fancy library?

jbeuckm commented 8 years ago

Ah yes I guess it does come from a fancy library: Ionic. Here is the directive:

https://github.com/driftyco/ionic/blob/master/js/angular/directive/menuClose.js#L1

DaDanny commented 8 years ago

I too had this issue where any time I called a scope function, it was fired twice, and although you can do a workaround like @tcarlsen suggested, having to do this for every function call would be pretty tedious, unnecessary and slow.

Fortunately, I found out angular-material had a click Hijack method, and you are able to turn that off.

To disable angular-mateiral click hijack, you just need to put this into your config file:

.config(function( $mdGestureProvider ) {
    $mdGestureProvider.skipClickHijack();
  });

I've been testing this solution and it seems to resolve the issue. However, I've only tried it with ng-click. I'm not sure how it will affect any other angular-material gestures, but for now, my functions are only being called once :)

sterichards commented 8 years ago

Same issue here too.

I have a