d3 / d3-zoom

Pan and zoom SVG, HTML or Canvas using mouse or touch input.
https://d3js.org/d3-zoom
ISC License
505 stars 143 forks source link

Touch move not triggering filter #124

Closed ChrisGatzo closed 5 years ago

ChrisGatzo commented 6 years ago

Hello,

I'm trying to implement a filter that would circumvent execution of touchmove.zoom event and I noticed in the source code that the touchmoved method does not do

if (!filter.apply(this, arguments)) return;

The obvious question is why? Will something break if make this change? So far it works fine.

The reason I want to do this is because I want zoom to only handle multi touch events and let the single touch bubble up to the browser, so that the user can still scroll the page with one finger and zoom/pan with two fingers. So then the filter can be something like

      d3
      .zoom()
      .filter(() => {
        return !(d3.event.type === 'touchmove' && d3.event.touches.length <= 1);
      })

Thanks

mbostock commented 6 years ago

It’s because zoom.filter only applies to events that can start zoom gestures, not events that occur on a gesture that’s already started. For the same reason, zoom.filter doesn’t apply to mousemove and mouseup events.

If you want to only allow multitouch zoom gestures, then ideally you’d use the existing zoom.filter application on the touchstart event rather than the touchmove event. However, the code currently cancels touchmove (preventing scrolling) even if the touchstart event didn’t initiate a zoom gesture. My guess is we want a fix like this?

function touchmoved() {
  if (!gestures.length) return; // No active gesture!
  var g = gesture(this, arguments),
      touches = event.changedTouches,
      n = touches.length, i, t, p, l;

  ... etc. remainder of existing touchmoved function …
}
hohlb commented 5 years ago

Was there any progress on this issue?

I have a similar situation: I would like to enable drag AND zoom behaviour for touch at the same time. The logic would be:

I tried to make it work by cancelling the drag event as soon as there is more than one finger on the screen, but it did not work smooth enough. Related to https://github.com/d3/d3-drag/issues/50

testower commented 5 years ago

I've added a PR with @mbostock`s suggested fix, which solves our problem.

robinhouston commented 5 years ago

If you want to only allow multitouch zoom gestures, then ideally you’d use the existing zoom.filter application on the touchstart event rather than the touchmove event.

We’ve found that to make this work correctly on Android Chrome we need another change as well, which is proposed as PR #176.

mbostock commented 5 years ago

Closing as this is covered by #169 #176.