Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: tapIgnoreEvent doesn't check for data-tap-disabled #2132

Closed
emertechie opened this issue Sep 3, 2014 · 6 comments
Closed

bug: tapIgnoreEvent doesn't check for data-tap-disabled #2132

emertechie opened this issue Sep 3, 2014 · 6 comments

Comments

@emertechie
Copy link

Type: bug

Platform: mobile webview

For various reasons I had to use another scrolling library in one area of my app. To disable the Ionic scroll stuff, I used overflow-scroll="true" on the ion-content and also data-tap-disabled="true" on a container element. However the Ionic tap library was still interfering (causing events not to fire in the other library).

It boiled down to the tapIgnoreEvent method in tap.js:

function tapIgnoreEvent(e) {
  if(e.isTapHandled) return true;
  e.isTapHandled = true;

  if( ionic.scroll.isScrolling && ionic.tap.containsOrIsTextInput(e.target) ) {
    e.preventDefault();
    return true;
  }
}

It's not checking if the element, or a parent, has the "data-tap-disabled" attribute set.
It should be using the isElementTapDisabled method like other tap handlers. Changing first line to the following fixes issues:

function tapIgnoreEvent(e) {
  if(e.isTapHandled || ionic.tap.isElementTapDisabled(e.target)) return true;
  ...

@adamdbradley
Copy link
Contributor

Would you be able to provide a simple codepen replicating the issue. I want to make sure I can break and fix with the same html structure you're using. Thanks

@adamdbradley adamdbradley added the needs: reply the issue needs a response from the user label Sep 8, 2014
@emertechie
Copy link
Author

Thanks for taking a look Adam. Ok, sure, I'll try get something together. I'm travelling at the moment though so will be towards end of this week before I can update.

@emertechie
Copy link
Author

@adamdbradley Ok got a repro for you here: http://codepen.io/emertechie/pen/sopig/

As you'll see I'm using the iScroll library. To repro, focus into one of the text fields and try to scroll up or down. You won't be able to. If you swap the script out to point to my custom version on dropbox which has the change proposed above, it works.

@Ionitron Ionitron removed the needs: reply the issue needs a response from the user label Jan 19, 2015
@Ionitron
Copy link
Collaborator

We do not recommend using two scroll libraries in the same app.

@anemitoff
Copy link

I don't understand why the proposed "fix" was rejected. The documentation states that the "data-tap-disabled" can be used to bypass tap handling for designated elements and that isn't 100% true.

I too have a non-supported use-case (IE Browser for Desktop with HTML5 Video Element whose control clicks are being intercepted by the tap handler), but I don't see why I shouldn't be able to use "data-tap-disabled" sucessfuly.

mleoking added a commit to mleoking/ionic that referenced this issue Nov 3, 2015
This fix enables the data-tap-disabled tag to completely bypass tap handling for designated elements. 

This is a bug fix for 

ionic-team#2132
http://forum.ionicframework.com/t/ionic-tap-event-conflicting-with-d3-event/7369
http://stackoverflow.com/questions/30113156/d3-zoom-in-ionic

and more.

Without this fix, the tap event handling of ionic conflicts with d3 and many other js libs.
mlynch pushed a commit that referenced this issue Dec 6, 2015
This fix enables the data-tap-disabled tag to completely bypass tap handling for designated elements. 

This is a bug fix for 

#2132
http://forum.ionicframework.com/t/ionic-tap-event-conflicting-with-d3-event/7369
http://stackoverflow.com/questions/30113156/d3-zoom-in-ionic

and more.

Without this fix, the tap event handling of ionic conflicts with d3 and many other js libs.
@mlynch
Copy link
Contributor

mlynch commented Dec 6, 2015

Fixed in 3119679

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Sep 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants