ionic-team / ionic-framework

A powerful cross-platform UI toolkit for building native-quality iOS, Android, and Progressive Web Apps with HTML, CSS, and JavaScript.
https://ionicframework.com
MIT License
51.14k stars 13.5k forks source link

Prevent ng-click When Scrolling #745

Closed calendee closed 10 years ago

calendee commented 10 years ago

When scrolling a list, items with ng-click are firing the click event. They really should not fire as this was a scroll rather than a click / tap.

Example : http://codepen.io/calendee/pen/yqefG

Reference : http://forum.ionicframework.com/t/disable-click-onscroll-list/1334

emonidi commented 10 years ago

http://docs.angularjs.org/api/ngTouch may help you. It handles ng-click as it is supposed to be handeled on mobile devices and alswo gives you ngSwipe directives as a bonus...:) Would be a good idea to be incorporated if it has not been done already...:)

perrygovier commented 10 years ago

Seeing this quite a bit on Android KitKat devices. Its to the point on those devices that the app is unusable.

adamdbradley commented 10 years ago

Thanks for reporting that @perrygovier, I'll be sure to heavily test my fixes on Android 4.4.

perrygovier commented 10 years ago

A little more info: I'm not seeing this when using ionic scrolling (but I do see slightly choppy animation then), only when I set the overflow-scroll="true" option. Adding ngTouch appears to have no affect though.

adamdbradley commented 10 years ago

Right now I do not recommend using ngTouch, and I blogged about it here: http://ionicframework.com/blog/hybrid-apps-and-the-curse-of-the-300ms-delay/

perrygovier commented 10 years ago

Could the KitKat problem be related to Chrome 32+? Applying fastclick logic when they're no longer necessary? http://updates.html5rocks.com/2013/12/300ms-tap-delay-gone-away

adamdbradley commented 10 years ago

I committed a change to the nightly build to fix this issue. Would you be able to test it out with the latest nightly build and let me know if its working for you now? Thanks http://code.ionicframework.com/#nightly

calendee commented 10 years ago

This seems to have done the trick.

Scrolling no longer fires the ng-click. Clicking works properly.

Sample : http://codepen.io/calendee/pen/osrLH

Thanks Adam!

Will consider closed.

calendee commented 10 years ago

Unfortunately, this issue has cropped up again.

See this sample : http://codepen.io/calendee/pen/gHxde

If you scroll or swipe, ng-click fires.

adamdbradley commented 10 years ago

I just quickly checked and this isn't an issue with touchmove, but only mousemove. Can you double check that?

Right now this is a design decision due to the fact that scrolling a webpage by holding the mouse down is not natural way to scroll on desktop, where as it is for touch. The main reason for not supporting mousemove canceling a click is due to Android firing both touchmove and mousemove events.

calendee commented 10 years ago

Interestingly, I just checked this on device and browser with the nightlies. The ng-click is not firing on device or browser with scroll attempts.

So.... all good?

DruRly commented 10 years ago

The latest works for me. Thx.

adamdbradley commented 10 years ago

I just pushed a large tap refactor to solve many of these issues: https://github.com/driftyco/ionic/blob/master/test/unit/angular/service/tap.unit.js#L4

Would you be able to test the nightly build (1712 or later) and let us know if it solved this issue? http://code.ionicframework.com/#nightly

Thanks

Related: #862 #740 #726 #691 #689 #365 #26 #1134 #1120 #1105 #1078 #772 #745

calendee commented 10 years ago

Just testing on iOS. ng-click events don't work on device anymore. Works fine in chome.

Tested with the side menu demo app and this code:

<ion-view title="Playlists">
  <ion-nav-buttons side="left">
    <button menu-toggle="left" class="button button-icon icon ion-navicon"></button>
  </ion-nav-buttons>
  <ion-content class="has-header">
    <ion-list>
      <ion-item ng-click="clickEvent()">Click Me</ion-item>

      <ion-item ng-repeat="playlist in playlists" href="#/app/playlists/{{playlist.id}}">
        {{playlist.title}}
      </ion-item>
      <ion-item ng-click="clickEvent()">Click Me</ion-item>
      <ion-item ng-click="clickEvent()">Click Me</ion-item>
      <ion-item ng-click="clickEvent()">Click Me</ion-item>
      <ion-item ng-click="clickEvent()">Click Me</ion-item>
      <ion-item ng-click="clickEvent()">Click Me</ion-item>
      <ion-item ng-click="clickEvent()">Click Me</ion-item>

    </ion-list>
  </ion-content>
</ion-view>
.controller('PlaylistsCtrl', function($scope) {
  $scope.playlists = [
    { title: 'Reggae', id: 1 },
    { title: 'Chill', id: 2 },
    { title: 'Dubstep', id: 3 },
    { title: 'Indie', id: 4 },
    { title: 'Rap', id: 5 },
    { title: 'Cowbell', id: 6 }
  ];

  $scope.clickEvent = function() {
    alert('Clicked!');
  }
})
calendee commented 10 years ago

One problem. Scrolling on these list items now highlights them during the scroll event. This problem came up a while back but then got solved. It's back.

What a nightmare y'all must be going through trying to get it to work everywhere. Fix here, breaks there. Whack-a-mole! Sorry guys.

adamdbradley commented 10 years ago

@calendee When I click the "click me" events in iOS7 I get the alert, but any of the ng-repeat items do not click because there's no ng-repeat directive on them.

Also, when tap and hold one of the items it turns gray, then when I scroll farther than 6px the gray goes away. You're saying that's not happening?

perrygovier commented 10 years ago

Substantially better! I'm not seeing the highlight problem when I upgrade my CSS as well as the JS. The grey color is removed after a bit of scrolling. There's some choppiness on my 4s, but very minor. Also, when pages load, and I start dragging right away (before the event handler binds, I presume), I have to release and start dragging again before the drag takes hold.

There's a very noticeable problem I'm seeing where pages show up blank every other time I navigate to them, but that's another ticket.

calendee commented 10 years ago

@adamdbradley I didn't notice that the highlight disappears after a bit of scroll. It does work though. Excellent. So I'm going to go ahead and close this. Thanks for the great work.

udeeps commented 8 years ago

@adamdbradley , I am using ionic v1.1.1. The list item doesn't get highlighted when I try scrolling in Google Chrome. But when I run the app in Galaxy S4, Android 4.4.2, the item gets highlighted for a while when I scroll. The flicker is kind of distracting. However, it doesn't behave so in iPhone 6. The scroll would have looked smoother if that highlight wouldn't trigger. Is there any way to achieve this?

cityzen commented 8 years ago

I'm seeing this issue as well on a Nexus 5 with KitKat 4.4.3 and iphone 6 with ios 9.3.1. I am wrapping the whole list item as a link so the link area is fairly large. Either way, it is quite distracting while scrolling through items.

EmreErdogan commented 7 years ago

I'm seeing the issue, too. Tested on iPhone 6 with iOS 10.2.1 I am using ionic bundle version is 1.3.2. I am also using cordova-plugin-crosswalk-webview@2.1.0 plugin.

For a workaround, I am using the solution offered in the following issue comment: https://github.com/driftyco/ionic/issues/1438#issuecomment-55434818

But the above workaround works only for list items. Of course it can be expanded to other elements, but you must bind the event handler to every clickable element in the page. Because every clickable element receives the click event when scrolling.

I am looking for a general approach that will apply to the whole application without code repetition.

Any ideas?

nidhi42 commented 6 years ago

can you all please help it out to find solution

EmreErdogan commented 6 years ago

The following solution may help: https://github.com/ionic-team/ionic/issues/1438#issuecomment-293602364

ionitron-bot[bot] commented 6 years ago

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.