Skip to content
This repository has been archived by the owner on Oct 8, 2021. It is now read-only.

Commit

Permalink
Merge pull request #2015 from mlitwin/bug_1992
Browse files Browse the repository at this point in the history 
Fixes #1992; Swipe threshold constants are configurable
  • Loading branch information
Scott Jehl committed Jul 10, 2011
2 parents e567742 + 1ef7313 commit 7b1966c
Showing 1 changed file with 12 additions and 4 deletions.
16 changes: 12 additions & 4 deletions js/jquery.mobile.event.js
View file
Expand Up @@ -115,6 +115,14 @@ $.event.special.tap = {

// also handles swipeleft, swiperight
$.event.special.swipe = {
scrollSupressionThreshold: 10, // More than this horizontal displacement, and we will suppress scrolling.

durationThreshold: 1000, // More time than this, and it isn't a swipe.

horizontalDistanceThreshold: 30, // Swipe horizontal displacement must be more than this.

verticalDistanceThreshold: 75, // Swipe vertical displacement must be less than this.

setup: function() {
var thisObject = this,
$this = $( thisObject );
Expand Down Expand Up @@ -144,7 +152,7 @@ $.event.special.swipe = {
};

// prevent scrolling
if ( Math.abs( start.coords[ 0 ] - stop.coords[ 0 ] ) > 10 ) {
if ( Math.abs( start.coords[ 0 ] - stop.coords[ 0 ] ) > $.event.special.swipe.scrollSupressionThreshold ) {
event.preventDefault();
}
}
Expand All @@ -154,9 +162,9 @@ $.event.special.swipe = {
$this.unbind( touchMoveEvent, moveHandler );

if ( start && stop ) {
if ( stop.time - start.time < 1000 &&
Math.abs( start.coords[ 0 ] - stop.coords[ 0 ] ) > 30 &&
Math.abs( start.coords[ 1 ] - stop.coords[ 1 ] ) < 75 ) {
if ( stop.time - start.time < $.event.special.swipe.durationThreshold &&
Math.abs( start.coords[ 0 ] - stop.coords[ 0 ] ) > $.event.special.swipe.horizontalDistanceThreshold &&
Math.abs( start.coords[ 1 ] - stop.coords[ 1 ] ) < $.event.special.swipe.verticalDistanceThreshold ) {

start.origin.trigger( "swipe" )
.trigger( start.coords[0] > stop.coords[ 0 ] ? "swipeleft" : "swiperight" );
Expand Down

5 comments on commit 7b1966c

@bergantine
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've had some difficulty understanding the documentation and source code on this and tried illustration so that we could talk about it to each other, we're still not sure if we have it right though. So far it has been a trial and error process. Would love some feedback from any of the contributors on this.

Events

In essence our understanding is that the swipe must break out of the box created by horizontalDistanceThreshold and verticalDistanceThreshold in a horizontal manner. And to scroll there can't be horizontal movement wider than the scrollSupressionThreshold. Is this a correct english interpretation of the math?

@jblas
Copy link
Contributor

@jblas jblas commented on 7b1966c Jan 30, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jbergantine

Basically what the code is trying to say is that if the following constraints were satisfied:

  • Touch event ended before the durationThreshod expired.
  • The x-direction (horizontal) exceeded the horizontalDistanceThreshold
  • The y-direction did NOT stray more than verticalDistanceThreshold from the original y value saved during the touchstart event

then a left/right swipe event is triggered.

In my opinion the scrollSupressionThreshold is un-necessary, and should be removed such that we always prevent the scroll. The reason I say this is because the threshold is 10px which is the same magic number that iOS and Android use to trigger/initiate scrolling. If we don't want scrolling, then we should just preventDefault on every touchmove event.

@bergantine
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! We were pretty close then I guess. scrollSupressionThreshold seems a bit weird still (we're using a number of 100 on that for an app that navigates between long scrolling pages via swipe where most people in our user testing want to be scrolling down the page not swiping on any given movement).

@jblas
Copy link
Contributor

@jblas jblas commented on 7b1966c Jan 31, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After thinking about this a bit more, I think we only want to supress scrolling if the user's touch has not exceeded the vertical threshold. Once they exceed this, then we should allow scrolling. This will allow folks to use swipe on longer pages.

@bergantine
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW this is the configuration we're currently using:

$.event.special.swipe.scrollSupressionThreshold = 100;
$.event.special.swipe.durationThreshold = 1000;
$.event.special.swipe.horizontalDistanceThreshold = 175;
$.event.special.swipe.verticalDistanceThreshold = 30;

We found the defaults way to prone to cause a swipe instead of a scroll. Additionally, when people got to the bottom of a long scroll the page had a tendency to "jump" to the next page which seemed to be a result of their fingers staying on the page etc.

Please sign in to comment.