Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

stopPropagation() not working with tap event #237

Closed
wlow84 opened this Issue · 18 comments

6 participants

wai yong Benny Kimmo Puputti Jorik Tangelder Jarno Rantanen Joshua Muheim
wai yong

I can't seem to get stopPropagation to work. I want to make the child div trigger the alert without causing the link to be called. I tried putting stopPropagation() and preventDefault() on all the events - I tried this in chrome and IOS.

  <a href='http://www.google.com' target="_blank">boo
    <div id='test_el' style='background:red;color:blue'>Hello world</div>
  </a>
 var element = document.getElementById('test_el');
    var hammertime = Hammer(element,{
        prevent_default:true
    }).on("tap", function(event) {
        event.stopPropagation();
        event.preventDefault();
        event.gesture.stopPropagation();
        event.gesture.preventDefault();
        event.gesture.srcEvent.stopPropagation();
        event.gesture.srcEvent.preventDefault();
        event.gesture.startEvent.stopPropagation();
        event.gesture.startEvent.preventDefault();
        alert('hello!');
    });

Here is plunker

http://plnkr.co/edit/Pr5pqy?p=preview

Benny

var element = document.getElementById('test_el');
var hammertime = Hammer(element).on('tap', function(event) { element.parentElement.href = null; alert('hello!'); });

// Ftfy

Kimmo Puputti

Any generic way to do that? Shouldn't event.stopPropagation() work?

wai yong

I agree - shouldn't event.stopPropagation() work? Furthermore the solution changes the href value, making it no longer usable.

Jorik Tangelder
Owner

stopPropagation just stops the event from bubbling, not the other events from firing. Like when you call stopPropagation on a mouseover event, it doesnt stops mouseout or something. Preventing a link from getting clicked you can only do this with calling preventDefault on the Click event. (correct me if im wrong)

the ev.preventDefault() doesnt make any sense actually in Hammer.js, since the browsers dont have a tap event implemented, but it is created by the domEvents that hammer uses. The ev.gesture.preventDefault() makes more sense, it calls the mouse or touch event that hammer used for the detection. This could be touchstart or mousemove etc.

element.on("tap", function(ev) {
// stops bubbling the tap event to its parents. so you can create nested events.
ev.stopPropagation();    

// prevents the browser from doing it's native 'tap' implementation.
// doesnt make any sense, only for the drag events, since most browsers 
// support dragstart-drag-dragend
// it is in hammer, because document.createEvent adds these, and Hammer uses 
// this for creating DOM events.
ev.preventDefault(); 

// stops the source event (in ev.gesture.srcEvent) from bubbling. 
// the source event could be touchstart, touchmove, mousemove etc.
ev.gesture.stopPropagation();

// prevents the source event (in ev.gesture.srcEvent) from doing it's native behavior.
// when you use this, you can make the element blocking, 
// because touchstart-touchmove let the browser scroll
// if you dont prevent the default action. this could be called when you are 
// using the drag and transform events,
// but hammer does this for you in most cases
ev.gesture.preventDefault();

});
Jorik Tangelder jtangelder closed this
wai yong

Just to clarify - is the solution to call ev.gesture.preventDefault()? In the plunker above, even if I do that - the link click still gets propagated.

Thank you for your fast reply!

Benny

@kpuputti @wlow84 The link is still getting propagated because you're stopping propagation in a tap handler, and expecting that to affect a link href value. It doesn't.

As @jtangelder said above, stopPropagation() is a method available on the DOM Event API, not necessarily on links and their href values. So, if you want to interrupt bubbling on a link (e.g. <a href="foo.html"></a>) then you're going to have to handle that "event" type (which isn't the same as click or tap). You can do that with one of the following solutions:

// Nullify the href value on tap registration (see my above comment)

OR

// Don't use href at all, and bind your elements to click (or tap) events universally:

// Your link and test element markup
<a id="clicks-not-hrefs">
    Link with URL
    <div id="test_el">Element with content</div>
</a>

// Sets the link to a `click` instead of `href`
document.getElementById('clicks-not-hrefs').onclick = function(e) { 
    window.location.href = 'http://www.google.com'; 
};

// Prevents the above navigation, but still alerts
document.getElementById('test_el').onclick = function(e) {
    e.stopPropagation();
    alert('hello!');
};

Notes:

  • I'm using click for browser demo purposes; but this will work the same as tap in hammer.js.

  • I'm using the more modern onclick=fn(e) over addEventListener('click', fn(e)) to prevent memory leaks associate with removing listeners (very common in most code you see) -- my assumption is that hammer.js has taken care of this in their .on() method, but I haven't confirmed that.

http://jsfiddle.net/C4AsA/ (jsfiddle prevents navigating away, but you can see the event registration in the console)

The most important takeaway here is that this is not an issue with hammer.js at all. This is how JavaScript works.

Kimmo Puputti

@bennyschmidt

Wouldn't just adding a click handler to the child element and preventing the event from bubbling up to the parent element be a less hacky solution?

Also, please don't say that element.onclick is a more modern solution than using element.addEventListener, that's clearly not the case. You can only attach one listener for the click event with element.onclick. Hammer is using addEventListener:

https://github.com/EightMedia/hammer.js/blob/master/src/instance.js#L53

Benny
  1. No, it does not (that's what this whole issue is about)
    • onclick literally is more modern than addEventListener
    • addEventListener is leaky:
      • It doesn't overwrite previously added listeners
      • In order to remove a listener, they must share the identical handler
      • Thus, it cannot handle events with anonymous handlers without leaking (but you knew that)
      • Either the var self = this or handler this binding seem more hacky than what you referenced
      • One must use this above mentioned method in order to imply proper scope

Oh yeah, but on the other hand you can "add multiple event listeners in one call". Because I just do that all the time.

Kimmo Puputti

You're right with 1.

With 2. I disagree. My take:

  • You should be able to add multiple listeners to the same event and not override previously added listeners
  • To remove a listener, you should know which listener to remove, this is ok
  • For anonymous listener functions, my impression was that leaking memory of keeping handlers of removed elements was a problem in oldIE and moder browsers can GC unreferenced functions fine. Correct me if I'm wrong
  • Handling the receiver of the function call has nothing to do with the DOM, that's just how this works in JavaScript

I haven't seen anyone recommend el.onclick over el.addEventListener for years, and it seems all the libraries are using addEventListener (or attachEvent as a fallback) anyways. Do you have any references?

Jarno Rantanen

For what it's worth, agree with @kpuputti; listener binding using on* properties instead of addEventListener() was the biggest turnoff for me when shopping around for a gesture/touch lib a while back. Also, not being able to touch listeners for which you don't have a reference (or indeed not even being aware of them) is an important tool in creating encapsulation between DOM components.

Benny

@kpuputti: With respect, I don't think you quite understand:

Of course you can add multiple listeners to the same event. Neither method prevents this. For example, click: document can be clicked; submitButton can be clicked; and someObject can be clicked -- all can be handled individually with either method.

But with addEventListener, if I register a new event listener where the element and event type have not changed, nothing in the API informs the previous, infinitely-open loop of "listening" to end, and you will have memory allocated for multiple events, when you might only intend to listen for one. That is called a memory leak.

Consider what addEventListener is. It's a function that takes some arguments -- one of which is a string which should correspond to a valid event type. Simply invoking the function again does not "clear" any previous invocations; it only adds new ones (I know, you're probably thinking "But I want this behavior!", read on).

Now consider what onclick is. It ramifies memory allocations much more like a property value that's overwritten (the same way you might overwrite a Backbone.View.initialize property method. It doesn't just open "new" listening loops.

To test this, lets add a click listener to an element with both methods, and see what happens in a more realistic scenario: http://jsfiddle.net/KbFrQ/

I am all ears to learn why, if after this, you still believe that legacy methods are superior.

Benny

@jareware "listener binding using on* properties instead of addEventListener() was the biggest turnoff for me when shopping around for a gesture/touch lib a while back."

As mentioned above, addEventListener is used in hammer.js:

// method to add a listener
on: function onEvent(gesture, handler){
    var gestures = gesture.split(' ');
    for(var t=0; t<gestures.length; t++) {
        this.element.addEventListener(gestures[t], handler, false);
    }
    return this;
},
Kimmo Puputti

@bennyschmidt

Still don't understand your point or your definition or "modern" or "legacy".

From MDN:

Why use addEventListener?

addEventListener is the way to register an event listener as specified in W3C DOM. Its benefits are as follows:

  • It allows adding more than a single handler for an event. This is particularly useful for DHTML libraries or Mozilla extensions that need to work well even if other libraries/extensions are used.
  • It gives you finer-grained control of the phase when the listener gets activated (capturing vs. bubbling)
  • It works on any DOM element, not just HTML elements.

https://developer.mozilla.org/en-US/docs/DOM/EventTarget.addEventListener

And it specifically mentions the "older way to register event listeners" here:

https://developer.mozilla.org/en-US/docs/DOM/EventTarget.addEventListener#Older_way_to_register_event_listeners

which is the solution you refer to as "modern".

wai yong

@bennyschmidt

First off - thank you for your explanation about links - it clarifies a previous misconception that I had.
Secondly, with addEventListener, while you don't have the ability to remove anonymous functions, you do have the ability to remove named functions.

Following your plunker example:

var hello = function(e){
    console.log('hello');
}
foo.addEventListener('click', hello);
foo.removeEventListener('click',hello);

This allows us to bind the same event to multiple handler functions, and remove them as well. This could possibly be useful in writing handler functions in a somewhat more modular way. A contrived example would be in IOS web apps, you have a navigation menu that slides out from the left/right. Now, imagine this scenarios -

  1. The side navigation menu is open
  2. If you tap the "close menu" icon, you would call the closeMenuHandler() which closes the menu
  3. If you tapped some other button in the navigation menu, you might want it to call both the closeMenuHandler() and the doSomeActionHandler() which performs some action

With a onclick function, I'm not sure if you can't really achieve this kind of modularity.

Furthermore, with the addEventListener, you can specify a third argument (https://developer.mozilla.org/en-US/docs/DOM/EventTarget.addEventListener) which gives you the option of deciding whether you want to use event bubbling or event capture. I personally can't think of a reason why you would want event capture, but it's there if you need it. I don't believe onclick has that functionality.

In the end, it all boils down to picking the right tool for the job. Onclick does a job, and guarantees you won't have memory leaks - addEventListener requires the programmer to understand what he's doing, and the consequences of binding anonymous functions, but gives slightly more flexibility.

Benny

@kpuputti: Modern doesn't mean "newer". Use whatever method you want, I obviously don't need to convince you; but realize that you have an open issue with hammer.js that is directly caused by your misunderstanding of addEventListener and has nothing to do with the library.

@wlow84: I don't mean to oppose the addEventListener method completely, but instead point out why this behavior is happening in the two open issues in hammer.js surrounding event handling. Also, your code does not work. Try it here: http://jsfiddle.net/wRZdE/

The reason it doesn't work is because you're not invoking hello; you're only passing a reference to the object. That won't make it handle the event. What you probably mean to do is something like this:

var foo = document.getElementById('foo');
var hello = {
    handleEvent: function(e) { console.log(e); }
};

foo.addEventListener('click', hello.handleEvent);
foo.removeEventListener('click', hello.handleEvent);

You can test that here: http://jsfiddle.net/wRZdE/1/
This is a leak-proof alternative and leverages the DOM's native "automatic handleEvent invocation" (which I'm sure you already knew about).

My point is, I'm not against addEventListener -- I use it all the time for custom events. But onclick is more succinct, more modern because it supports more browsers (it's a DOM 0 implementation), is inherently leak-proof, and also solves the problem mentioned in two hammer.js open issues -- both of which are not hammer.js issues (or even issues at all).

wai yong

@bennyschmidt

The first fiddle works for me - not sure why it doesn't work for you - am I missing something?
I'd be happy to take this discussion offline - as you pointed out, this really isn't a hammer.js issue and probably shouldn't exist here.

@jtangelder
Thank you once again for your great library and taking the time to answer questions. I really appreciate your work.

Rob Harper robharper referenced this issue from a commit in robharper/hammer.js
Rob Harper robharper Makes hammer event's srcEvent equal to currently handled DOM event
(rather than some previous event that had touches).

This addresses #237 since it allows preventDefault to be called on touchEnd.
8b3afae
Joshua Muheim

Thank you for your explanation, @bennyschmidt, your comment lead us into the correct direction of a problem of ours. :+1:

We had incredible problems getting a very simple JS dropdown (a slightly modified Bootstrap one) to work with iOS, because it always propagated the click on the link within the dropdown up to a link that was sibling with the dropdown. No idea, why. I guess it has something to do with the guessing-mechanisms iOS uses to let the user press on links on a very small screen. We had to remove the a tag that triggered the dropdown to open and replaced it with a span. Then it worked. But: A newly added CSS rule for the span to behave similar to a link when hovering over it prevented it again! I had to refactor it, and after some slight modifications it worked again.

Just wanted to mention this, maybe somebody else finds it useful - we wasted 2 or 3 full work days for this incredibly dumb issue.

Benny

@jmuheim Happy to help :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.