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

Touch move not triggering filter #124

Closed
ChrisGatzo opened this issue Nov 29, 2017 · 5 comments
Closed

Touch move not triggering filter #124

ChrisGatzo opened this issue Nov 29, 2017 · 5 comments

Comments

@ChrisGatzo
Copy link

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
Copy link
Member

mbostock commented May 17, 2018

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
Copy link

hohlb commented Oct 24, 2018

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:

  • if one finger is down: drag
  • if more fingers are down: zoom

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 d3/d3-drag#50

@testower
Copy link
Contributor

testower commented Feb 5, 2019

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

@robinhouston
Copy link
Contributor

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
Copy link
Member

Closing as this is covered by #169 #176.

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

Successfully merging a pull request may close this issue.

5 participants