Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Swipe event: Trigger only one event per mouse sequence and let it bubble #6946

Closed
wants to merge 6 commits into from

3 participants

@gabrielschulhof
Collaborator

The element to which a mousedown handler is attached by the swipe setup() code
takes ownership of the next swipe event. This means that, if the same mousedown
event triggers other swipe setup() -attached mousedown handlers as it bubbles,
the subsequent ones will bail. Thus, if a swipe event results from the mousedown
then such a swipe event will be triggered on the deepest element with an
attached swipe handler, and it will be allowed to bubble from there. This
provides the tools that developers can use to avoid scenarios such as the one
mentioned in the referenced issue.

Fixes gh-6262

@coveralls

Coverage Status

Coverage decreased (-0.14%) when pulling 2ba7d5b on 6262-only-one-swipe-event into cfc1195 on master.

@coveralls

Coverage Status

Coverage increased (+0.29%) when pulling 65eda5f on 6262-only-one-swipe-event into cfc1195 on master.

gabrielschulhof added some commits
@gabrielschulhof gabrielschulhof Swipe event: Trigger only one event per mouse sequence and let it bubble
The element to which a mousedown handler is attached by the swipe setup() code
takes ownership of the next swipe event. This means that, if the same mousedown
event triggers other swipe setup() -attached mousedown handlers as it bubbles,
the subsequent ones will bail. Thus, if a swipe event results from the mousedown
then such a swipe event will be triggered on the deepest element with an
attached swipe handler, and it will be allowed to bubble from there. This
provides the tools that developers can use to avoid scenarios such as the one
mentioned in the referenced issue.

Fixes gh-6262
fbe9239
@gabrielschulhof gabrielschulhof Swipe event: Track mouse handlers and remove only the ones tracked f5f0c53
@gabrielschulhof gabrielschulhof Unit tests: Add bubble-checking for swipe bb3ada6
@gabrielschulhof gabrielschulhof Unit tests: Make swipe tests run in Chrome as well as phantomjs 5d00bc7
@gabrielschulhof gabrielschulhof Swipe event: Use $.data() to store context d7542f3
@gabrielschulhof gabrielschulhof Unit tests: Make sure swipe cleans up its event handlers and only those aaafe08
@arschmitz
Owner

i think this looks good now just make sure to squash :+1:

@gabrielschulhof gabrielschulhof deleted the branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 21, 2014
  1. @gabrielschulhof

    Swipe event: Trigger only one event per mouse sequence and let it bubble

    gabrielschulhof authored
    The element to which a mousedown handler is attached by the swipe setup() code
    takes ownership of the next swipe event. This means that, if the same mousedown
    event triggers other swipe setup() -attached mousedown handlers as it bubbles,
    the subsequent ones will bail. Thus, if a swipe event results from the mousedown
    then such a swipe event will be triggered on the deepest element with an
    attached swipe handler, and it will be allowed to bubble from there. This
    provides the tools that developers can use to avoid scenarios such as the one
    mentioned in the referenced issue.
    
    Fixes gh-6262
  2. @gabrielschulhof
  3. @gabrielschulhof
  4. @gabrielschulhof
  5. @gabrielschulhof
  6. @gabrielschulhof
This page is out of date. Refresh to see the latest.
View
90 js/events/touch.js
@@ -30,10 +30,14 @@ define( [ "jquery", "../jquery.mobile.vmouse", "../jquery.mobile.support.touch"
}
});
- function triggerCustomEvent( obj, eventType, event ) {
+ function triggerCustomEvent( obj, eventType, event, bubble ) {
var originalType = event.type;
event.type = eventType;
- $.event.dispatch.call( obj, event );
+ if ( bubble ) {
+ $.event.trigger( event, undefined, obj );
+ } else {
+ $.event.dispatch.call( obj, event );
+ }
event.type = originalType;
}
@@ -170,25 +174,47 @@ define( [ "jquery", "../jquery.mobile.vmouse", "../jquery.mobile.support.touch"
Math.abs( start.coords[ 1 ] - stop.coords[ 1 ] ) < $.event.special.swipe.verticalDistanceThreshold ) {
var direction = start.coords[0] > stop.coords[ 0 ] ? "swipeleft" : "swiperight";
- triggerCustomEvent( thisObject, "swipe", $.Event( "swipe", { target: origTarget, swipestart: start, swipestop: stop }) );
- triggerCustomEvent( thisObject, direction,$.Event( direction, { target: origTarget, swipestart: start, swipestop: stop } ) );
+ triggerCustomEvent( thisObject, "swipe", $.Event( "swipe", { target: origTarget, swipestart: start, swipestop: stop }), true );
+ triggerCustomEvent( thisObject, direction,$.Event( direction, { target: origTarget, swipestart: start, swipestop: stop } ), true );
return true;
}
return false;
},
+ // This serves as a flag to ensure that at most one swipe event event is
+ // in work at any given time
+ eventInProgress: false,
+
setup: function() {
- var thisObject = this,
- $this = $( thisObject );
+ var events,
+ thisObject = this,
+ $this = $( thisObject ),
+ context = {};
+
+ // Retrieve the events data for this element and add the swipe context
+ events = $.data( this, "mobile-events" );
+ if ( !events ) {
+ events = { length: 0 };
+ $.data( this, "mobile-events", events );
+ }
+ events.length++;
+ events.swipe = context;
+
+ context.start = function( event ) {
+
+ // Bail if we're already working on a swipe event
+ if ( $.event.special.swipe.eventInProgress ) {
+ return;
+ }
+ $.event.special.swipe.eventInProgress = true;
- $this.bind( touchStartEvent, function( event ) {
var stop,
start = $.event.special.swipe.start( event ),
origTarget = event.target,
emitted = false;
- function moveHandler( event ) {
+ context.move = function( event ) {
if ( !start ) {
return;
}
@@ -196,23 +222,57 @@ define( [ "jquery", "../jquery.mobile.vmouse", "../jquery.mobile.support.touch"
stop = $.event.special.swipe.stop( event );
if ( !emitted ) {
emitted = $.event.special.swipe.handleSwipe( start, stop, thisObject, origTarget );
+ if ( emitted ) {
+
+ // Reset the context to make way for the next swipe event
+ $.event.special.swipe.eventInProgress = false;
+ }
}
// prevent scrolling
if ( Math.abs( start.coords[ 0 ] - stop.coords[ 0 ] ) > $.event.special.swipe.scrollSupressionThreshold ) {
event.preventDefault();
}
- }
+ };
- $this.bind( touchMoveEvent, moveHandler )
- .one( touchStopEvent, function() {
+ context.stop = function() {
emitted = true;
- $this.unbind( touchMoveEvent, moveHandler );
- });
- });
+
+ // Reset the context to make way for the next swipe event
+ $.event.special.swipe.eventInProgress = false;
+ $document.off( touchMoveEvent, context.move );
+ context.move = null;
+ };
+
+ $document.on( touchMoveEvent, context.move )
+ .one( touchStopEvent, context.stop );
+ };
+ $this.on( touchStartEvent, context.start );
},
teardown: function() {
- $( this ).unbind( touchStartEvent ).unbind( touchMoveEvent ).unbind( touchStopEvent );
+ var events, context;
+
+ events = $.data( this, "mobile-events" );
+ if ( events ) {
+ context = events.swipe;
+ delete events.swipe;
+ events.length--;
+ if ( events.length === 0 ) {
+ $.removeData( this, "mobile-events" );
+ }
+ }
+
+ if ( context ) {
+ if ( context.start ) {
+ $( this ).off( touchStartEvent, context.start );
+ }
+ if ( context.move ) {
+ $document.off( touchMoveEvent, context.move );
+ }
+ if ( context.stop ) {
+ $document.off( touchStopEvent, context.stop );
+ }
+ }
}
};
$.each({
View
77 tests/unit/event/event_core.js
@@ -6,6 +6,7 @@
var libName = "jquery.mobile.events.js",
components = [ "events/touch.js", "events/throttledresize.js", "events/orientationchange.js" ],
absFn = Math.abs,
+ originalPageContainer = $.mobile.pageContainer,
originalEventFn = $.Event.prototype.originalEvent,
preventDefaultFn = $.Event.prototype.preventDefault,
events = ("touchstart touchmove touchend tap taphold " +
@@ -28,7 +29,12 @@
// the collections existence in non touch enabled test browsers
$.Event.prototype.touches = [{pageX: 1, pageY: 1 }];
+ $.mobile.pageContainer = originalPageContainer || $( "body" );
+
$($.mobile.pageContainer).unbind( "throttledresize" );
+ },
+ teardown: function() {
+ $.mobile.pageContainer = originalPageContainer;
}
});
@@ -344,14 +350,60 @@
});
var swipeTimedTest = function(opts){
- var swipe = false;
+ var newHandlerCount, origHandlerCount,
+ origHandleSwipe = $.event.special.swipe.handleSwipe,
+ handleSwipeAlwaysOnInner = true,
+ swipe = false,
+ bubble = false,
+ qunitFixture = $( "#qunit-fixture" ),
+ body = $( "body" ),
+ dummyFunction = function() {},
+ getHandlerCount = function( element ) {
+ var event, index,
+ eventNames = [ "touchstart", "touchmove", "touchend" ],
+ returnValue = {},
+ events = $._data( element, "events" );
+
+ for ( index in eventNames ) {
+ returnValue[ eventNames[ index ] ] = 0;
+ if ( events && events[ eventNames[ index ] ] ) {
+ returnValue[ eventNames[ index ] ] =
+ ( events[ eventNames[ index ] ].length || 0 );
+ }
+ }
+
+ return returnValue;
+ };
forceTouchSupport();
- $( "#qunit-fixture" ).bind('swipe', function(){
+ // Attach a dummy function to ensure that the swipe teardown leaves it attached
+ body.add( qunitFixture )
+ .on( "touchstart touchmove touchend", dummyFunction );
+
+ // Count handlers - this will include the function added above
+ origHandlerCount = {
+ body: getHandlerCount( body[ 0 ] ),
+ qunitFixture: getHandlerCount( qunitFixture[ 0 ] )
+ };
+
+ qunitFixture.one('swipe', function(){
swipe = true;
});
+ body.one( "swipe", function() {
+ bubble = true;
+ });
+
+ // Instrument method handleSwipe
+ $.event.special.swipe.handleSwipe =
+ function( start, stop, thisObject, origTarget ) {
+ if ( thisObject !== qunitFixture[ 0 ] ) {
+ handleSwipeAlwaysOnInner = false;
+ }
+ return origHandleSwipe.apply( this, arguments );
+ };
+
//NOTE bypass the trigger source check
$.Event.prototype.originalEvent = {
touches: [{
@@ -360,19 +412,34 @@
}]
};
- $( "#qunit-fixture" ).trigger("touchstart");
+ qunitFixture.trigger("touchstart");
//NOTE make sure the coordinates are calculated within range
// to be registered as a swipe
mockAbs(opts.coordChange);
setTimeout(function(){
- $( "#qunit-fixture" ).trigger("touchmove");
- $( "#qunit-fixture" ).trigger("touchend");
+ qunitFixture.trigger("touchmove");
+ qunitFixture.trigger("touchend");
}, opts.timeout + 100);
setTimeout(function(){
deepEqual(swipe, opts.expected, "swipe expected");
+ deepEqual( bubble, opts.expected, "swipe bubbles when present" );
+ deepEqual( handleSwipeAlwaysOnInner, true, "handleSwipe is always called on the inner element" );
+
+ // Make sure swipe handlers are removed in case swipe never fired
+ qunitFixture.off( "swipe" );
+ $( "body" ).off( "swipe" );
+
+ deepEqual({
+ body: getHandlerCount( body[ 0 ] ),
+ qunitFixture: getHandlerCount( qunitFixture[ 0 ] )
+ }, origHandlerCount, "exactly the swipe-related event handlers are removed." );
+
+ // Remove dummy event handler
+ body.add( qunitFixture )
+ .off( "touchstart touchmove touchend", dummyFunction );
start();
}, opts.timeout + 200);
View
3  tests/unit/event/index.html
@@ -13,12 +13,13 @@
</script>
<script src="../../../js/jquery.mobile.define.js"></script>
+ <script src="../../../js/jquery.mobile.ns.js"></script>
<script src="../../../js/jquery.mobile.support.touch.js"></script>
<script src="../../../js/jquery.mobile.events.js"></script>
+ <script src="../../../js/jquery.mobile.vmouse.js"></script>
<script src="../../../js/events/touch.js"></script>
<script src="../../../js/events/throttledresize.js"></script>
<script src="../../../js/events/orientationchange.js"></script>
- <script src="../../../js/"></script>
<link rel="stylesheet" href="../../../external/qunit/qunit.css"/>
<script src="../../../external/qunit/qunit.js"></script>
Something went wrong with that request. Please try again.