wrong event target with useDelegation=false

[wkornewald]

When having useDelegation=false getTarget() only returns a target if the event handler was registered on exactly the same element as event.target. That means that you can't register an event handler on a div and still allow clicking on elements within that div (e.g., in our case we have a div and a canvas within that div). Instead of looking at event.target it would be better to use the element that bind() was called on.

I'm in the middle of those changes right now, so this is not yet finished, but I'm thinking of something like this:

--- jquery.touchy-orig.js       Tue Aug 21 15:45:55 2012
+++ jquery.touchy.js    Tue Aug 21 15:44:24 2012
@@ -94,9 +94,8 @@
     var proxyHandlers = {

         handleTouchStart: function (e) {
-
             var eventType = this.context,
-                $target = getTarget(e, eventType);
+                $target = getTarget(e, eventType, this.target);

             if ($target) {
                 var event = e.originalEvent,
@@ -192,7 +191,7 @@

         handleTouchMove: function (e) {
             var eventType = this.context,
-                $target = getTarget(e, eventType);
+                $target = getTarget(e, eventType, this.target);

             if ($target) {
                 var event = e.originalEvent,
@@ -336,7 +335,7 @@

         handleGestureChange: function (e) {
             var eventType = this.context,
-                $target = getTarget(e, eventType);
+                $target = getTarget(e, eventType, this.target);

             if ($target) {
                 var $target = $(e.target),
@@ -393,7 +392,7 @@

         handleTouchEnd: function (e) {
             var eventType = this.context,
-                $target = getTarget(e, eventType);
+                $target = getTarget(e, eventType, this.target);

             if ($target) {
                 var event = e.originalEvent,
@@ -507,7 +506,7 @@
          *
         handleTouchCancel: function (e) {
             var eventType = this.context,
-                $target = getTarget(e, eventType);
+                $target = getTarget(e, eventType, this.target);

             if ($target) {
                 var target = e.target,
@@ -652,11 +651,11 @@
         return points;
     },

-    getTarget = function(e, eventType){
+    getTarget = function(e, eventType, origTarget){
         var $delegate,
             $target = false,
             i = 0,
-            len = boundElems[eventType].length
+            len = boundElems[eventType].length;
         if ($.touchyOptions.useDelegation) {
             for (; i < len; i += 1) {
                 $delegate = $(boundElems[eventType][i]).has(e.target);
@@ -666,8 +665,8 @@
                 }
             }
         }
-        else if (boundElems[eventType] && boundElems[eventType].index(e.target) != -1) {
-            $target = $(e.target)
+        else {
+            $target = origTarget;
         }
         return $target;
     },
@@ -775,9 +774,11 @@
                     $(this).data('touchy' + capitalizedKey, $.extend({}, $.touchyOptions[key].data));
                     $(this).data('touchy' + capitalizedKey).settings = $.extend({}, $.touchyOptions[key]);
                     delete $(this).data('touchy' + capitalizedKey).settings.data;
-                    if ( boundElems[key].length === 1 ) {
+                    var handler = $.touchyOptions.useDelegation ? $(document) : $(this);
+                    var context = $.extend({target: handler}, contexts[key]);
+                    if ( boundElems[key].length === 1 || !$.touchyOptions.useDelegation) {
                         $.each($.touchyOptions[key].proxyEvents, function(i, proxyEvent){
-                            $(document).bind(proxyEvent.toLowerCase() + '.touchy.' + key, $.proxy(proxyHandlers['handle' + proxyEvent], contexts[key]
));
+                            handler.bind(proxyEvent.toLowerCase() + '.touchy.' + key, $.proxy(proxyHandlers['handle' + proxyEvent], context));
                         });
                     }

@@ -785,9 +786,10 @@
                 teardown: function (namespaces) {
                     boundElems[key] = boundElems[key].not( this );
                     $(this).removeData('touchy' + capitalizedKey);
-                    if ( boundElems[key].length === 0 ) {
+                    var handler = $.touchyOptions.useDelegation ? $(document) : $(this);
+                    if ( boundElems[key].length === 0 || !$.touchyOptions.useDelegation) {
                         $.each($.touchyOptions[key].proxyEvents, function(i, proxyEvent){
-                            $(document).unbind(proxyEvent.toLowerCase() + '.touchy.' + key);
+                            handler.unbind(proxyEvent.toLowerCase() + '.touchy.' + key);
                         });
                     }
                 },

[fisherwebdev]

Hi Waldemar,

This is an interesting approach. I'll have to take a look at it a bit more closely when I have a moment (within the next 24 hours).

Thanks! Bill

[wkornewald]

Hi Bill, great, I'd love to find a nice solution for this. I'm also wondering if this approach could completely replace useDelegation. After all, the touchstart/etc. events do bubble, so if we let the low-level events bubble to the right DOM element we can then inject the high-level event in that element once it's recognized. Well, I'm just thinking aloud and I'm not yet sure if this will work if you want to prevent bubbling for the high-level event. We might have to manually detect calls to stopPropagation() and preventDefault() on the high-level event and call the respective methods on the low-level event, for example. What were the problems you faced when trying to implement event bubbling/delegation and why did you have to simulate it via useDelegation?

By the way, I'll continue working on those changes tomorrow, but I'll primarily focus on the useDelegation=false mode since that's all we need for our project (http://www.pentotype.com). I'll send you a patch once I'm finished.

Cheers, Waldemar

[fisherwebdev]

Hi Waldemar,

For what you want to accomplish, I think you want to set useDelegation to true, not false. I looked into this issue and noticed that this feature of Touchy was not working the way I had intended.

I have created a new remote branch where we can look into this issue a bit more. It's called "event-target". Please pull from that branch and take a look at some changes I have made. I'd appreciate your feedback very much. If you would like to send a pull request for any further improvements, and you want to work on that branch, please do.

There is also a new test page on this branch called use-delegation.html that you might find useful.

The following changes are in that branch:

• Bumped version to 1.1.
• Improved event delegation with the useDelegation setting. Now, while useDelegation is true, interacting with an inner element will trigger an event on an outer bound element. Depending on the way you use Touchy in your application, this may not be compatible with Touchy 1.0.
• To further enhance the useDelegation feature, the data object passed to the event handler now has a reference to the originalEvent and the originalTarget -- touchy-drag updates these to the current location of the user's finger, but the other events stick with the initial touch point.
• Test page called use-delegation.html
• Changed touchy-longpress to include the data object so that it may also be used in an event delegation pattern.
• Added a phase to touchy-pinch, and added a trigger for it on the end phase. This also breaks backwards compatibility with Touchy 1.0. While not relevant to the issue at hand, I noticed this oversight and decided to fix it.

Also, I should point out: I don't think the suggestions you had in your earlier post would work with Touchy's architecture. Please check out Ben Alman's article on special events. Touchy implements Ben's architecture, which is already based on a kind of event delegation. Touchy listens for events on window.document, and then determines where the event originated. The act of binding a Touchy event is actually the act of registering the DOM node to be part of the delegation scheme. If useDelegation is true, then Touchy determines if the original target is within any of the registered nodes.

Thanks for raising this issue and digging into the code! And like I said, if you could take a look and provide feedback, that would be great.

Bill

[wkornewald]

Hi Bill, Ben's method is intended for a different use-case in which you can't depend on bubbling of low-level events, but in our case we do have low-level events that bubble. So, the useDelegation feature shouldn't be necessary, at all. We can utilize the normal low-level events' delegation mechanism. Hammer.js binds directly on the element, too: https://github.com/EightMedia/hammer.js/blob/master/jquery.specialevent.hammer.js I'll try to see if I can refactor the code such that useDelegation can be removed.

Cheers, Waldemar

[fisherwebdev]

Hi Waldemar,

I apologize that I did not respond here also.

Ben's use case in sections 4.2 and 4.3 of his article is actually quite similar to what we are trying to do with Touchy. He wants to create a "tripleclick" event out of click events, and we want to create a "touchy-swipe" event out of touchstart, touchmove and touchend. It seems very similar to me.

And as Ben points out in the article, if you bind the special events to the elements themselves, there is no way to stop to propagation of the event -- it bubbles all the way up the DOM tree. Normally, one would use event.stopPropagation() to prevent the bubbling. But this is not possible if the special event is bound to the element. This would seem to be a bad thing if you want nested elements to behave differently. The donut.html test page, currently only on the event-target branch, tests this scenario.

Please let me know if there is some aspect of this that I don't understand.

It's an interesting aspect of hammer.js that you have pointed out. I am wondering if the hammer.js people have figured out a way to use stopPropagation with their special events. I might play around with hammer.js and see if they were able to achieve this.

[wkornewald]

Hi Bill, I think in practice stopPropagation isn't all that important so the hammer.js people just didn't implement it. But as I already explained two comments ago, you could override stopPropagation on the gesture event object to e.g. call stopPropagation on the low-level event. This wouldn't actually be sufficient, though because the other event handlers might need to reset their internal state (the data object). So, maybe instead stopPropagation could set an attribute on the event object and the event handlers would check against that attribute and then reset data and trigger some cancel event that makes sense.

[fisherwebdev]

Hi Waldemar,

We're looking into a solution where we can bring in your ideas. Stay tuned.

• Bill

[dapinitial]

I think this is similar to an issue I've detected with anchors in the wheel example... I think that you need to setup the touch events per: this.element.addEventListener( 'touchstart' , function touchStart( e ) { onMouseDown(this, e); } , true ); this.element.addEventListener( 'touchmove', function touchMove ( e ) { onMouseMove(this, e) }, true); this.element.addEventListener( 'touchend', function touchEnd ( e ) { onMouseUp(this, e) }, true); and then have a boolean flag like spinning = false; this.didSpin = false and if it is spinning then e.stopPropagation... that should deal with the bubble effectively. I've create scrollViews this way before however, your wheel example is nice... Maybe I'll just copy your transitions. (-: