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

Brush doesn’t prevent scrolling on touch devices. #45

Closed
mbostock opened this issue Jun 6, 2019 · 7 comments · Fixed by #50
Closed

Brush doesn’t prevent scrolling on touch devices. #45

mbostock opened this issue Jun 6, 2019 · 7 comments · Fixed by #50

Comments

@mbostock
Copy link
Member

mbostock commented Jun 6, 2019

It’s impossible to move a brush vertically on touch devices because it doesn’t appear to be preventing the page from scrolling from moving the brush.

@mbostock
Copy link
Member Author

Possibly we need touch-action: none, too.

@mbostock
Copy link
Member Author

The only thing that we’re missing here is that we’re calling nopropagation on touchstart rather than noevent, and that’s due to d3/d3-drag#9. But as long as the user isn’t trying to support clicking and brushing simultaneously, it should be safe to noevent on touchstart and thus prevent scrolling. Brushing is essentially unusable on touch devices now, so I think we must start there.

@mbostock mbostock mentioned this issue Jul 30, 2019
29 tasks
@mbostock
Copy link
Member Author

mbostock commented Aug 2, 2019

as long as the user isn’t trying to support clicking and brushing simultaneously

It seems like that works, though. You can listen to click events as long as the click gesture didn’t have any effect on the brush (e.g., clicking on the background when the brush is empty, or clicking on the brush but not moving during click).

That means we might want to take the more conservative route and only preventDefault on mousedown for touch devices.

@mbostock
Copy link
Member Author

mbostock commented Aug 2, 2019

I must be missing something here: d3-drag also does not preventDefault on touchstart or mousedown, but it works on iOS.

@mbostock
Copy link
Member Author

mbostock commented Aug 2, 2019

I’ve isolated the issue: it’s because for d3-brush, the touchmove listener isn’t registered until touchstart, whereas for d3-drag and d3-zoom, the touchmove listener is registered once ahead of time. 🤦‍♂

https://observablehq.com/d/e3dc7fb1297975f8

@mbostock
Copy link
Member Author

mbostock commented Aug 2, 2019

Some history, since I was curious. d3.brush previously registered the touch listeners statically; they were switched to transient listeners in d3/d3@6ef0eda. Prior to that, d3.brush registered listeners statically on the window! That was removed in d3/d3@ec12de6. d3.drag adopted static touch listeners in d3/d3-drag@9496742. d3.zoom appears to have always used static touch listeners since the first commit, d3/d3@2d98bf1.

@mbostock mbostock mentioned this issue Aug 2, 2019
@yaahor
Copy link

yaahor commented May 6, 2020

It works properly if brush is applied to the <svg> element, but <g> element.

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.

2 participants