Fix dragger on touch devices #405
Conversation
|
Here is an example that uses a patched version of |
|
Any chance to see this merged? Is there anything else I can do? |
|
I'll see if I can find someone who knows more about touch events than I do. It would probably also help if the commit description was more detailed: Fix it how? What was broken about it? etc. |
Currently, on touch devices, goog.fx.Dragger calls "init" on the browser event, to re-configures it with something (a Touch object) it can calculate clientX, clientY, screenX and screenY values from. And, in handleMove_, goog.fx.Dragger calls "preventDefault" on the browser event, to prevent the page to scroll while dragging. This doesn't work on touch devices, because the browser event was re-initialized with something (a Touch object) that doesn't have a "preventDefault" method. This commit fixes the problem by making goog.events.BrowserEvent aware of touch events. In init, when a touch event is detected, the clientX, clientY, screenX and screenY values are calculated from the "relevant" Touch object. And with this, the "maybeReinitTouchEvent_" hack can be removed from goog.fx.Dragger. Closes #162
|
@MatrixFrog, thanks. I rebased the commit onto the latest master, and I added a more detailed description of the problem and of the solution in the commit message. I hope this helps. |
Fix dragger on touch devices
|
@MatrixFrog, out of curiosity, did you finally have someone else review this? |
|
No, I ended up reviewing it myself. I'm not an expert on this stuff but it seems pretty clear that you know what you're doing. Having a good commit description is very helpful :) Sorry for the delay, thanks for your patience. |
|
No problem, I know what it is. |
|
long ago goog.labs.events.touch was introduced to merge our touch/pointer handling. I would hope that we could start solidifying on that. |
|
@elemoine @MatrixFrog I don't have hard evidence for you, but I've just tested this for my closure project and dragging now works on iOS 7, but not on iOS 8. Any thoughts? |
|
This is probably related: http://developer.telerik.com/featured/scroll-event-change-ios-8-big-deal/ |
I agree that a better event architecture integrating touch/pointer handling would be a good thing. |
|
@mtsgrd is there any error messages from Safari? |
|
Thanks @fredj. I'll have a look. |
There was a problem hiding this comment.
After more thinking, and looking at #337, I think it's a mistake to rely on targetTouches. With multiple touches on the touch surface targetTouches[0] is unlikely to be the "relevant touch". See my comments in #337 for more details, and see the targetTouches definition on mdn. So as done in #337 I'm tempted to rewrite this whole block as follows:
/**
* On touch devices use the first "changed touch" as the relevant touch.
* @type {Touch}
*/
var relevantTouch = e.changedTouches ? e.changedTouches[0] : null;There was a problem hiding this comment.
Here's a branch implementing this: https://github.com/elemoine/closure-library/tree/targettouches. @mtsgrd, I'd be curious to know if that makes a difference with iOS 8.
There was a problem hiding this comment.
@mtsgrd this example is based on my targettouches branch: http://erilem.net/ngeo/layerorder-touch/layerorder.html. Does it work for you in iOS 8?
6 participants
Closes #162