Using passive event handlers about d3-drag

[mbostock]

This is already some of the most complex and issue-prone code in D3, so I am wary of adding another code path for a subset of browsers.

Reading a little more about passive event listeners, it seems that you can have a passive event listener that cancels scroll by using the touch-action style property. So perhaps we can make them passive after all?

[jrencz]

@ernest-okot listeners added via drag.on will accept 3rd argument as you may see in drag.js@L147

The problem is that drag calls selection.on internally: namely: inside the drag function.

Until we get a better API for what's there inside drag function you may be interested in the following patch:

DON'T USE THIS TECHNIQUE. IT'S HERE FOR A REFERENCE. SEE COMMENTS BELOW

const clone = selection.sort(); 

// Not sure if it's the best way to clone a selection. I found no other.
// Cloning to prevent exposing patched `on` to the rest of the codebase

clone.on = (() => {
  const originalSelectionOn = selection.on.bind(selection);
  return (name, fn, options = {passive: true}) =>
    originalSelectionOn(name, fn, options);
})();

clone.call(
  drag()
    .on('start', () => {}, {passive: true})
    .on('drag', () => {}, {passive: true})
    .on('drag', () => {}, {passive: true})
);

This way I got rid of all [Violation] warnings Chrome Canary showed me: both those that I could pass the 3rd argument (drag.on) and those what drag calls internally.

[mbostock]

Passive listeners are not appropriate for the drag behavior as it must cancel scrolling.

[jrencz]

yeah, I noticed that, especially on mobile, it broke some things.

@mbostock do all handlers drag set up needs to be active or just some of them?

[mbostock]

You can see the table of events in the README. My guess is that the Violation warnings occur when you have a non-passive event listener that does not call event.preventDefault. Actually, it does occur to me that it might be possible to make the mousedown and touchstart event listeners passive, since due to the convoluted way in which these events need to be handled, the default behavior on these events is not prevented even though mousemove and touchmove are subsequently prevented.

[jrencz]

So, as I understand, you will think about making some of the internally attached handlers passive, but it's not as easy as saying "this one can be passive and that one not".

Then my patch snippet definitely does more harm than good. I'll leave a remark in the original comment in case someone finds it later.

[mbostock]

It seems that the Violation appears in the touchstart and touchmove event listeners in Chrome on macOS, even though those event listeners are never invoked (because it’s not a touch device). That seems unfortunate, since it implies that you get a warning even if your event listeners are necessarily non-passive. It might be possible to make the touchstart listener passive, but it’s not possible to make touchmove passive as we need to call event.preventDefault.

Also, per #9, I’d rather always call event.preventDefault. It’s just I can’t because it breaks capturing of mouseup outside the window and it breaks click emulation on touch devices.

[jrencz]

Thanks for the briefing

[mbostock]

This might be fixable with Pointer Events (#25) but browser support is still around 60% (behind a flag Firefox, and missing from Safari and iOS).

[jrencz]

Have you already considered using pointer events when they're available with a fallback to mouse events? I'm just thinking out loud, I'm not aware of if it's worth maintaining that or how complex may it become

[jrencz]

Too bad it's again Safari that does not respect that

[mbostock]

Okay. I think I have a good fix for the warning: don’t register touch event listeners on devices that don’t support touch.

[jrencz]

What about devices like MS Surface which may support touch in general but user has other options?

generally you will have 'ontouchstart' in window === true on Surface but it's not certain that the user will ever use touch. The current mean of interaction should also be detected (more or less like this):

let currentMeanOfInteractionIsTouch = false;

window.addEventListener('touchstart', () => {
  currentMeanOfInteractionIsTouch = true;
  // attach other touch event listeners
}, {passive: true})

window.addEventListener('mousedown', () => {
  currentMeanOfInteractionIsTouch = false;
  // remove touch event listeners but the one above, attach mouse
}, {passive: true})

I just can't tell up front if attaching events on demand won't be either too late or too expensive. How do you think?

[Herst]

@jrencz On convertible devices like the Surface you could also try to detect whether it is in tablet mode, e.g. see the discussion at https://stackoverflow.com/questions/32493093, but neither the solutions from there nor your solution belong inside d3 IMHO since it's too use-case specific.

On a side note, I came across this and related bug tracker entries while looking for solutions for issues I was having with pan and drag-drop with d3 in Microsoft Edge and IE11 even in demos from @mbostock. There I learned touch-action with which I was able to solve the problems by simply disallowing any touch action except for pinch-zoom inside the SVG, apparently the preventDefault method isn't working in these browsers.

[jrencz]

@Herst I do own Surface and I barely use tablet mode. Yet I tend to touch the screen a lot in desktop mode. It serves a purpose of lightweight task home laptop, not a tablet at all in my household. I'd bet I'm not alone in this mode of usage.

So detecting the mode (btw: how can it be done in JS?) is not the best choice IMO