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

Set touch-action: none on the map container #4259

Closed
kavada opened this issue Feb 11, 2017 · 11 comments · Fixed by #4487
Closed

Set touch-action: none on the map container #4259

kavada opened this issue Feb 11, 2017 · 11 comments · Fixed by #4487

Comments

@kavada
Copy link

kavada commented Feb 11, 2017

mapbox-gl-js version:

Steps to Trigger Behavior

  1. While using mapbox gl-js on a mobile device or any device that uses (touch) to move the map, an error is thrown due to the event listener being treated as passive. I even looked into this on your own site, it does the same on any mapbox application using the gl-js version.

Expected Behavior

Move fluid

Actual Behavior

Moves slow, clunky

@Scarysize
Copy link

Could be related to the issue React has with recent Chrome (56): facebook/react#8968

@anandthakker
Copy link
Contributor

@Scarysize good find -- that appears to be the likely culprit.

Our touchmove and touchend event handlers for DragPanHandler, DragRotateHandler, and TouchZoomRotateHandler all do need to use preventDefault(), so we'll probably need to add {passive: false} for those. Possibly also for the wheel/mousewheel events used by ScrollZoomHandler.

Aside from those, I think we should also do an audit of any other touch event handlers we register and determine whether we (a) are using preventDefault, and (b) actually need to be. (For example, the touchstart events in the Drag*Handlers.) If the answer is yes, then we should likewise set {passive: false}; otherwise, it would still be good to be explicit and set {passive: true}.

@kavada
Copy link
Author

kavada commented Feb 19, 2017

@anandthakker Was this something that you guys figured out? I'm still consoling errors in my application. Thanks!

@bicubic
Copy link

bicubic commented Feb 20, 2017

@anandthakker possibly related to this is the fact that mapbox-gl-js stopped correctly working with windows 10 mult-touch gestures at some point not long ago. E.g. the demo at https://www.mapbox.com/mapbox-gl-js/examples/ will scale the entire page if I attempt to pinch to zoom in (although zoom out works correctly), and will also invoke chrome 'go back' gesture if I try to pan the map to the right.

Both of these scenarios worked fine a couple months ago, possible regression?

@johnlaur
Copy link
Contributor

We ran into this issue preparing for a trade show last week using some Win10 touch kiosks.

In order to keep touch events ultra-responsive the standards-makers are proposing and appearing to move to disallow 'active' event handlers -- that is to say that javascript event handlers will only process asyncronously with whatever the native touch handler is doing (zoom, pan, scroll, etc.)

The way to stop the native handlers is to set the touch-action: none in the CSS; this seems to be a good overall workaround unless you want to still support touch scrolling of the page until the map is 'activated' by a tap or something. Chrome 56 also has a bug where touch-action: none does not stop all instances of pinch-zoom. It's fixed in Chrome 57, currently in the Beta channel. There is absolutely no workaround that can be done in js/css/html for Chrome 56. The only fix is to completely disable pinch zoom via commandline argument.

I noticed but did not investigate why the touch events dont work at all in Edge anymore (did they ever?)

@kavada
Copy link
Author

kavada commented Feb 22, 2017

Interesting. Well thank you guys for the information. It's not the biggest issue, it's honestly just a bit annoying on mobile. Hopefully Chrome 57 will fix this.

@indus
Copy link
Contributor

indus commented Mar 8, 2017

@kavada it is a big issue for me in a fullscreen app on a touch table. While zooming the map GUI-Elements get zoomed as well until they are out of the viewport.

@bicubic
Copy link

bicubic commented Mar 8, 2017

@indus I was able to get touch working properly by setting style.touchAction = 'none' on the mapbox element as per @johnlaur suggestion. You should be able to do the same.

@anandthakker anandthakker changed the title Unable to preventDefault inside passive event listener due to target being treated as passive. Declare {passive: false} for any touch event handlers that use preventDefault Mar 13, 2017
@jfirebaugh jfirebaugh changed the title Declare {passive: false} for any touch event handlers that use preventDefault Set touch-action: none on the map container Mar 23, 2017
@jfirebaugh
Copy link
Contributor

Retitling to reflect the preferred solution of using touch-action: none as described here. Passing {passive: false} to addEventListener() when touch-action: none will do is discouraged.

Note that we should not unconditionally set touch-action: none on the map container, but rather use more targeted rules depending on which handlers are enabled, similar to what was implemented in Leaflet in this PR.

@jfirebaugh
Copy link
Contributor

Linking upstream issue for reference: WICG/interventions#18

@mollymerp mollymerp self-assigned this Mar 23, 2017
@mollymerp mollymerp mentioned this issue Mar 23, 2017
5 tasks
@Erutan409
Copy link

@bicubic .... YES Thank you!

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

Successfully merging a pull request may close this issue.

9 participants