Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some drags don't register #206

Closed
Download opened this issue Jun 19, 2015 · 9 comments
Closed

Some drags don't register #206

Download opened this issue Jun 19, 2015 · 9 comments

Comments

@Download
Copy link
Contributor

I'm using a horizontal Sly with the mouse and I'm noticing that every once in a while my drags don't register. I will try to drag the slider to the left but nothing happens. I happens about one in five drags or so. Enough to become annoying after a while.

I traced this down to a fragment of code inside function dragHandler:

if (!dragging.init) {
    // I found that the test below prevents drags from starting when the initial move
    // was in the 'wrong' direction... I may be spastic but for me that happened a lot.
    // It was an annoyance that did not help me so I turned it off. -Stijn
    //if (o.horizontal ? abs(dragging.pathX) > abs(dragging.pathY) : abs(dragging.pathX) < abs(dragging.pathY)) {
        dragging.init = 1;
    //} else {
    //    return dragEnd();
    //}
}

The test in there was probably added for a good reason, but I have not found any side-effects from disabling it and it does fix my problem...

Why is that test there? If I disable it like this will it hurt?

Thanks for building a great scroller!

@Download
Copy link
Contributor Author

I think, if the test is necessary (and I presume it is :) ) that maybe we need to give it a litlle treshold so the drag does not get cancelled if the first pixel movement is downward. So it could maybe see if the path was longer than say 5 pixels before cancelling...

@darsain
Copy link
Owner

darsain commented Jun 19, 2015

The dragging initiates only when you are dragging in the same direction as Sly. This initiation is strict, meaning it is determined at 1st pixel dragged, so if you drag 1 pixel up and than continue left, the 1st pixel up cancels the dragging.

It used to be 1st 10 pixels, but there were other issues related to touch ad mobile devices. It should be revisited, but I don't have devices to test the revisions on.

When you disable the check, you also disable page scrolling in direction opposite to Sly. So for example, people on mobile devices won't be able to initiate vertical scroll on horizontal Sly instance.

@Download
Copy link
Contributor Author

"When you disable the check, you also disable page scrolling in direction opposite to Sly. So for example, people on mobile devices won't be able to initiate vertical scroll on horizontal Sly instance."

Ah yes now I get it. In my case the Sly scroller is the only element on the page so I don't have that issue.

"It used to be 1st 10 pixels, but there were other issues related to touch ad mobile devices."

Could you elaborate on this?

The behavior where just a single pixel movement in the wrong direction cancels the drag is really a bit too strict. I notice it especially when quickly moving through the scroll area. If you swipe a couple of times quickly then one of them is bound to start with a vertical movement and gets cancelled and it makes Sly feel unresponsive.

Looking at the code, the disabling of the page scrolling probably happens due to the call to stopDefault right? Could there maybe be a middle ground where in the first 10 pixels we don't cancel drag, but also don't stopDefault, until we have sufficient data to determine whether the scroll is horizontal or vertical?

@darsain
Copy link
Owner

darsain commented Jun 19, 2015

Could you elaborate on this?

It's all about killing/bugging touch scrolling on mobile devices.

There is ton's of issues with this. The current implementation is I think 3rd iteration already. I won't be trying to rewrite it again until I have some devices to test it on, otherwise I'm just shooting in the dark, which is not fun.

@Download
Copy link
Contributor Author

Gotcha. I have versioned my fork as v1.5.1.1 (extra .1).

@Download
Copy link
Contributor Author

Some time has passed since I've opened this issue and I discovered a few things:

  • The 1px threshold currently used in Sly is too short
  • Not ignoring half the swipes (the ones in the wrong direction) fixes the not registering of drags, but actually aggrevates 'missed clicks' happening
  • Adding a 3px threshold basically solves everything for me.

The 1px threshold is definitely too short. I've had some issues with me tapping an element on the item that is in the scroller and the tap not getting picked up. I traced this back to Sly 'eating' the event because it was treated as a drag instead of a tap.

Not ignoring the swipes in the wrong direction actually makes that issue worse. However if we do ignore it, we sometimes miss drag events (the original topic of this issue). So it seems I have too choose which events I want to get eaten.... :(

I've found that if I add a 3px threshold, all these issues disappear. My swipes are picked up very well. If I swipe in the wrong direction, Sly ignores it so you can scroll the page vertically from a horizontal scroller for example. And all my taps are registered perfectly this way.

As of yet I have found no downsides to this change and it maintains the original goal of the code snippet of separating vertical from horizontal swipes so as to preserve page scroll behavior on horizontal Sly sliders.

So I've put my changes in a separate branch and made a pull request for that.
I understand you don't want to change this code without the option to test it on different devices first and I totally get that. But I will be going life with the changes I made anyway. So what I'll be doing is keeping track of this issue and reporting on how it's doing 'in the wild'. If/when I feel confident that it works well across many different devices and browsers, I will update you.

@Download
Copy link
Contributor Author

Thanks you for picking up my Pull Request. I also see that you made dist files etc and labeled the check-in Release 1.6.0. However, you did not yet create the Git tag... So it's not actually in the Releases list... Was this an oversight?

@darsain
Copy link
Owner

darsain commented Jul 19, 2015

Yes. Forgot push --tags :)

@Download
Copy link
Contributor Author

Ha ha yes it happens. Thank you for releasing it. Now I can finally get off my forked version :)

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

No branches or pull requests

2 participants