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

Fixes #2328 and #3823: added scrollTop tracking to swipe event #4478

Closed
wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented May 30, 2012

... to avoid having it triggered by touch-drag scrolls.

Passed unit tests, and I tested swipe, swipeleft and swiperight on iPad and iPhone.

@jaspermdegroot
Copy link
Contributor

Just for tracking: #2328 #3823

@SamDecrock
Copy link

could it be that this is not in the master branch? I can't seem to find that code when I compile the master...

edit: never mind. I just cloned your repository.
This should be added to the master branch of jQuery Mobile. It really fixes this annoying scroll+swipe issue!

@jaspermdegroot
Copy link
Contributor

@SamDecrock - Yes, that is correct. It is an open pull request that needs to be reviewed and tested. After that, if we decide to apply the suggested changes, it will be merged into master branch.

@BorisDutkin
Copy link

So I didn't really understand where I'm supose to add the code above...
I have only 2 .js file: jquery.mobile-1.1.0.min.js and myFile.js ?

@jaspermdegroot
Copy link
Contributor

@BorisDutkin - Your question is more something for the forum (https://forum.jquery.com/jquery-mobile)

@allanjackson
Copy link

Is there any timeline on getting this fix approved and incorporated into master?

@@ -170,7 +172,8 @@ $.event.special.swipe = {
if ( start && stop ) {
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 ) {
( Math.abs( start.coords[ 1 ] - stop.coords[ 1 ] ) + Math.abs( start.offset - stop.offset ))
Copy link
Contributor

Choose a reason for hiding this comment

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

Aren't you just doubling the change in the Y position by adding the coordinate difference to the scrollTop difference? Maybe I'm missing something here about the values that are reported.

Choose a reason for hiding this comment

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

I can confirm that it works with his patch and that it's NOT working without. Scrolling up (or down) diagonally makes jQuery mobile believe that there was as slide.

That's because, when you scroll up, the origin of the coordinate system scrolls up too so there is actually no vertical movement going on...

Therefore you have to add the scrollTop() difference to the vertical movement. scrollTop() not being part of the same coordinate system as the data.pageY.

@rahulteotia
Copy link

We can also confirm the following fix is also working for us and it no longer swipes in case of diagonal scroll.

@johnbender
Copy link
Contributor

Another issue that came up with this patch is that scrollTop causes a reflow on the page which is exceptionally expensive when used with something like touchmove. We're playing around trying to figure out a different solution or we may try to provide a hook so that that you can modify the calculation as necessary.

@jaspermdegroot
Copy link
Contributor

@ghost

Not sure you get this message since you are no longer an active Github user, but thanks for your work on this!

Like @johnbender mentioned the change you suggested has downsides so we are looking for another solution, so I am closing this PR.

Also note that the problem only seems to occur on iOS, not on Android. See #2328 (comment)

Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants