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

Use touches rather than changedTouches in touchstarted #176

Closed
wants to merge 1 commit into from

Conversation

robinhouston
Copy link
Contributor

@robinhouston robinhouston commented Jul 16, 2019

It seems that the behaviour of Chrome on Android differs from the behaviour of Safari on iOS in the following respect: when a multitouch event is initiated by touching two or more fingers to the screen near-simultaneously, iOS Safari will fire a single touchstart event that includes all the touches (as both event.touches and event.changedTouches) whereas Android Chrome will fire a sequence of touchstart events:

  • one with touches.length == 1 and changedTouches.length == 1
  • followed by one with touches.length == 2 and changedTouches.length == 1

In other words, the behaviour on Android Chrome when the screen is touched with two fingers simultaneously is the same as the behaviour on iOS Safari when you touch it first with one finger and then a second.

The change in PR #169 makes it possible to ignore single-touch gestures on iOS Safari, by returning a falsy value from the filter function when there is only a single touch, but this fails on Android Chrome because the first touch of a multitouch gesture is never recorded by d3-zoom: when the second touch is processed, the changedTouches array contains only the second touch.

This PR changes the behaviour of the touchstarted handler to consider all the touches in the event, rather than just the new touches, which appears to resolve the problem.

It seems that the behaviour of Chrome on Android differs from the
behaviour of Safari on iOS in the following respect: when a multitouch
event is initiated by touching two or more fingers to the screen
near-simultaneously, iOS Safari will fire a single touchstart event
that includes all the touches (as both event.touches and event.changedTouches)
whereas Android Chrome will fire a sequence of touchstart events:

- one with touches.length == 1 and changedTouches.length == 1
- followed by one with touches.length == 2 and changedTouches.length == 1

In other words, the behaviour on Android Chrome when the screen is
touched with two fingers simultaneously is the same as the behaviour
on iOS Safari when you touch it first with one finger and then a second.

The change in PR d3#169 makes it possible to ignore single-touch gestures
on iOS Safari, but this fails on Android Chrome because the first touch
is never recorded. When the second touch is processed, the changedTouches
array contains only the second touch.

This commit changes the behaviour of the touchstarted handler to consider
all the touches in the event, rather than just the new touches.
@mbostock
Copy link
Member

So if I understand correctly: you use zoom.filter to ignore the first touch, but when the second touch comes down, we need to consider not just event.changedTouches (which only includes the second touch) but event.touches (which includes both). That makes sense.

@mbostock
Copy link
Member

mbostock commented Aug 5, 2019

Merged as c4734c2.

@mbostock
Copy link
Member

And reverted in 176ad73 1.8.2.

This change broke the gesture it described on iOS Safari in the common case: when you touch it first with one finger and then a second. The problem is that when iterating over the event.touches in the second touchstart, it sets g.touch1 to be the same touch as the g.touch0 set on the first touchstart.

If you want to resubmit the PR, I think you’ll need something like this:

for (i = 0; i < n; ++i) {
  t = touches[i], p = touch(this, touches, t.identifier);
  p = [p, this.__zoom.invert(p), t.identifier];
  if (!g.touch0) g.touch0 = p, started = true, g.taps = 1 + !!touchstarting;
  else if (!g.touch1 && g.touch0[2] !== p[2]) g.touch1 = p, g.taps = 0;
}

This way, if the first touchstart is filtered, the second will get both touches, whereas if the first touchstart is not filtered, the second will only get one additional touch.

@mbostock
Copy link
Member

Okay, well, that works so it’ll be in 1.8.3…

mbostock added a commit that referenced this pull request Aug 10, 2019
@robinhouston
Copy link
Contributor Author

Ah! Sorry not to have caught this problem. Thanks for fixing it.

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

Successfully merging this pull request may close these issues.

None yet

2 participants