kornelski / slip

Slip.js — UI library for manipulating lists via swipe and drag gestures
BSD 2-Clause "Simplified" License
2.44k stars 213 forks source link

Only regiter listener for selectionchange on IOS #64

Closed Jenselme closed 8 years ago

Jenselme commented 8 years ago

I have a strange problem with slip on chrome, IE and firefox with the dom.select_events.enabled set to true (as describe here): the onSelection callback is fired before the holdTimer can complete. That prevent the reordering to work. To get it working, I have to first click on on the draggable element and only then reorder. Here are examples of this:

From what I understand of the comment, this listener is only relevant for ios devices, so it shouldn't have an impact on other platforms. Strangely, your demo is not affected by this.

kornelski commented 8 years ago

Is there a solution that does not involve detecting a particular browser? Is that a timing issue? Can it be solved by setting a flag or deferring actions with setTimeout?

Jenselme commented 8 years ago

Can it be solved by setting a flag or deferring actions with setTimeout?

I think I could set a flag in setTimeout and use it not to execute the function associated with the selectionchange event. If I didn't use this solution, it was to interfere as little as possible with a function I don't fully understand the purpose. I tried to just remove the line, and everything looked fine (not tested on iOS though).

Do you prefer the flag approach? I understand that checking the user agent is not really a reliable way to detect a browser.

kornelski commented 8 years ago

Listening for selection change event (and preventDefault on it) is to prevent mouse movement during reordering from being interpreted as text selection gesture. Without it drag may still work, but browsers will be selecting all text along the way, which is ugly and confusing.

The second problem is that on iOS you can't prevent selection this way (you can with CSS only), but you can at least react to it happening, so Slip tries to cooperate with iOS in that case, and aborts the drag.

Does this line run in your case? (i.e. if you delete that line, does it work for you?) https://github.com/pornel/slip/blob/4fa9d23063e5b88e31850ae57dc5303daa48ba65/slip.js#L583

if so, it'd be better to add browser sniffing to this condition https://github.com/pornel/slip/blob/master/slip.js#L577

Jenselme commented 8 years ago

Listening for selection change event (and preventDefault on it) is to prevent mouse movement during reordering from being interpreted as text selection gesture. Without it drag may still work, but browsers will be selecting all text along the way, which is ugly and confusing.

Agreed.

Does this line run in your case? (i.e. if you delete that line, does it work for you?) https://github.com/pornel/slip/blob/4fa9d23063e5b88e31850ae57dc5303daa48ba65/slip.js#L583

It does.

if so, it'd be better to add browser sniffing to this condition

I'm fine with that since it will solve my problem. Do you have a reliable way to detect iOS?

kornelski commented 8 years ago

Do you have a reliable way to detect iOS?

No, unfortunately. Test for iPhone|iPad|iPod and not Android|Windows (to eliminate some everything-goes user-agents).

Jenselme commented 8 years ago

Just updated the PR.

kornelski commented 8 years ago

Thanks