Panning in mobile devices

[dy]

This prevents page scroll while panning 3d plots in mobile devices, related to #1411 (comment).

Also this disables event logging for 3d plots, akin to #2251.

• has-passive component https://www.npmjs.com/package/has-passive-events
• 3d-view-controls component passive: false Prevent page scrolling mikolalysenko/3d-view-controls#6
• mouse-wheel component passive: false Mute event logging mikolalysenko/mouse-wheel#7
• replace Lib.eventListenerOptionsSupported with has-passive

@alexcjohnson please review.

[alexcjohnson]

• tests
• verify manually on an actual iPad

Also two questions:

• we have event.preventDefault() in a few places, but then we also have Lib.pauseEvent(event) which does other things as well... are there browsers that still need this (in which case we should use it everywhere) or is preventDefault now sufficient on every platform we support?
  • Corollary to that, and nothing to do with this PR but I'm curious - why do I see a couple of d3.event.preventDefault() but other places d3.event.sourceEvent.preventDefault() - is one pattern broken, are these different in some way that we need the different syntax, or can we simplify the latter?
• Is gl2d/camera still in use anywhere, and if so, does it need this treatment as well?

[dy]

@alexcjohnson that might be difficult to test if CI window gets scroll changed while panning 3d/2d plots. Is that sort of test we ideally want?

[dy]

@alexcjohnson I don't see any difficulty in e.preventDefault() direct call − since that is supported in IE ≥9, and that is precise instruction to avoid page scroll. Not sure if stopping propagation of the touchstart event and keeping the rest of the handlers (mousedown etc) is correct pattern. So I would not force Lib.pauseEvent everywhere (I would not consider Lib.pauseEvent a necessary wrapper, working with DOM directly is safe here).
As for d3.event.preventDefault − considering the source, that seems to be always DOM event, and seems to have no difference with d3.event.sourceEvent (some delayed events or alike).

[alexcjohnson]

Right, it's a little difficult, but if we're fixing a bug we really should try to find a way to test it. Two ideas come to mind:

• attach an event handler to some parent element for the event (touchmove?) that would have led to scrolling, and verify that this handler is not called (but before the fix it is called)
• hold on to the touchmove event we generate, inspect it afterward and make sure it had preventdefault called on it.

Thanks for looking into pauseEvent - indeed it looks safe to 🔪 , wouldn't need to happen in this PR but you're welcome to if you feel like it.

[felipe257]

I've noticed on python module that when fixedrange is true for x and y it doesn't reset the behavior of touchmove (or touchstart?), i.e., the page won't scroll when touchmove on the graph. Is it on purpose?

[etpinard]

Is gl2d/camera still in use anywhere, and if so, does it need this treatment as well?

Unfortunately yes. All non-scattergl gl2d trace types use it.

[dy]

@etpinard gl2d events fixed with fa1db3b. Waiting for @mikolalysenko to approve PRs.

[dy]

@alexcjohnson seems that browsers do not allow to trigger default action by emulated events, like scrolling container/document by emulating wheel event. By that, trying to emulate touch drag or scroll do not cause container.scrollTop change. Emulated events can only trigger javascript handlers.

Not sure if we should stop propagation, but testing prevented default is a good thought.

[dy]

@alexcjohnson should be fixed, it remains merging mouse-wheel PR, although it should not affect this PR.

[etpinard]

it remains merging mouse-wheel PR,

We should wait for new mouse-wheel and 3d-view-control versions to be published so that we can bump our pacakge.json and get a proper run on CI.

[dy]

@etpinard I merged and published 3d-view-controls already, it is up to mouse-wheel, but that only affects single console warning, a patch version.

[etpinard]

but that only affects single console warning, a patch version.

Cool. Let's wait a few days. If we don't get any news from Mikola, we'll merge this thing before the next minor v1.34.0 early next week.

[etpinard]

@dfcreative Any news from Mikola? If not, would you mind fixing the merge conflicts and merge this in to include this in 1.34.0?

[etpinard]

Oh wait. This PR was opened before the new npm5 package-lock file was added. Can you regenerate the package-lock with the new deps?

[etpinard]

good catch here. Thanks!

[etpinard]

Well done @dfcreative 💃