Skip to content
This repository was archived by the owner on Aug 1, 2024. It is now read-only.

Conversation

@fredj
Copy link
Contributor

@fredj fredj commented Aug 28, 2014

targetTouches is not defined for touchend or touchcancel events. changedTouches is always defined.

With the current implementation (after 116609a) the position is always [0, 0]

closes #41

fredj referenced this pull request Aug 28, 2014
… can have a zero-length targetTouches list, so we check for the array length before derefencing.

-------------
Created by MOE: http://code.google.com/p/moe-java
MOE_MIGRATED_REVID=72774880
targetTouches is not defined for touchend and touchcancel events.
changedTouches is always defined.
@fredj
Copy link
Contributor Author

fredj commented Mar 5, 2015

Any change for this to get reviewed? Thanks

@elemoine
Copy link
Contributor

elemoine commented Mar 5, 2015

This change makes sense!

The goog.style.getClientPosition function may be used to return the client position of an event. In the case of a touch event it doesn't make sense to rely on the targetTouches.

This is the definition of targetTouches (from mdn):

« A TouchList listing all the Touch objects for touch points that are still in contact with the touch surface and whose touchstart event occurred inside the same target element as the current target element. »

So, on a touchstart, targetTouches[0] may not refer to the new touch point, but to a previous touch that is still in contact with the touch surface! So I think that currenly goog.style.getClientPosition may only work for the fist touchstart.

@fredj's patch fixes that problem. It also makes goog.style.getClientPosition work on touchend and touchcancel events, as mentioned in the issue description.

Potential caveat: when multiple touches are simultaneously added/removed to/from the touch surface then goog.style.getClientPosition doesn't make sense, because there are multiple "client positions" in that case. @fredj uses the touch object at index 0 in the changedTouches list. I think this is acceptable. @fredj what do you think about documenting this behavior in the comment block above the function?

@fredj
Copy link
Contributor Author

fredj commented Mar 5, 2015

I'll add a comment, thanks

@elemoine
Copy link
Contributor

elemoine commented Mar 6, 2015

@fredj, in fact, with #405, goog.style.getClientPosition could look like this:

goog.style.getClientPosition = function(el) {
  goog.asserts.assert(el);
  if (el.nodeType == goog.dom.NodeType.ELEMENT) {
    return goog.style.getClientPositionForElement_(
        /** @type {!Element} */ (el));
  } else {
    var targetEvent = el.changedTouches ? el.changedTouches[0] : el;
    return new goog.math.Coordinate(
        targetEvent.clientX,
        targetEvent.clientY);
  }
};

Indeed, with #405, if el is a goog.events.BrowserEvent then el.clientX and el.clientY should be correct already. So the only specific case is that of a native touch Event.

Thoughts?

@elemoine
Copy link
Contributor

elemoine commented Mar 6, 2015

See master...elemoine:41.

@fredj
Copy link
Contributor Author

fredj commented Mar 9, 2015

@elemoine please create a pull request with your branch (and add fixes #337 in the description)

@elemoine
Copy link
Contributor

elemoine commented Mar 9, 2015

See #424.

@joeltine
Copy link
Contributor

joeltine commented Aug 4, 2015

#424 is the canonical

@joeltine joeltine closed this Aug 4, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

goog.style.getClientPosition error with touchend event

4 participants