erik-krogh / SudoSlider

The most versatile jQuery content slider
36 stars 24 forks source link

Improve scrolling/zooming/clicking on touch devices #13

Closed foobear closed 10 years ago

foobear commented 10 years ago

I encountered problems with links inside slides that were not properly clickable on touch devices (iPad Air in my case). Double clicking did follow the link, but a single tap almost never worked.

It seems that the allowScroll method was the culprit, as it prevents the touch event.

There are two changes that seemed necessary to me:

+ if (mathAbs(x) > mathAbs(y)) event.preventDefault();

When dragging horizontally, we want to prevent the browser's scroll event. This change also allows to start dragging horizontally and then moving downwards, as usually happens when sliding with the thumb on a phone, for example.

- isDirectionVertical(prevX, prevY, x, y) == option[7]/*vertical*/
+ isDirectionVertical(prevX, prevY, x, y) && option[7]/*vertical*/

I am not sure why it read == here. Seems we want to prevent the browser's scroll event only when the vertical option on the slider is set. But a horizontal drag on horizontal slides would also fulfil the condition before -- which does not sound right to me. This is why I believe we need a && here. :)

Since isMouseEvent was set to false anyway, I've removed it from the condition.

I am not entirely sure if my changes match your intent, since I could not find any tests. Happy to discuss if I broke anything or if it should be done differently.

webbiesdk commented 10 years ago

Nice idea for how to allow "start dragging horizontally and then moving downwards". I'm going to use that.

isMouseEvent was set to false because i forgot to remove some code after debugging. Stuff like that happens, even to the best of us.

I merged your change into the touch branch

I'm not putting this on master just yet, wan't to test it on more devices first (only got a S III mini here).

I tried to get it to work the way want with the vertical option, and i also pushed that on the touch branch 35a88c60ce4ba5555dbd811725f5f98c77cf563a

webbiesdk commented 10 years ago

Tested on a iPad, looking good, so i merged it to master.

Maybe i should have allowScroll as a option, so the behavior can be disabled for people that don't want it.

foobear commented 10 years ago

Great stuff. :)

Also just wanted to say thanks for that nice slider. It works really well for me, and I love how it does not force styles on my application. :+1: