Skip to content

Mute event listener logging #2251

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jan 18, 2018
Merged

Mute event listener logging #2251

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5adb727
Mute event listener logging
dy Jan 15, 2018
26e6df1
Merge branch 'master' into passive-event-logging
dy Jan 17, 2018
a32a21a
Add passive events detection
dy Jan 18, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/components/dragelement/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,12 @@ dragElement.init = function init(options) {
element.style.pointerEvents = 'all';

element.onmousedown = onStart;
element.ontouchstart = onStart;

if(element._ontouchstart) {
element.removeEventListener('touchstart', element._ontouchstart);
}
element._ontouchstart = onStart;
element.addEventListener('touchstart', onStart, {passive: false});

function _clampFn(dx, dy, minDrag) {
if(Math.abs(dx) < minDrag) dx = 0;
Expand Down
10 changes: 8 additions & 2 deletions src/plots/cartesian/dragbox.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1041,8 +1041,14 @@ function calcLinks(constraintGroups, xIDs, yIDs) {

// still seems to be some confusion about onwheel vs onmousewheel...
function attachWheelEventHandler(element, handler) {
if(element.onwheel !== undefined) element.onwheel = handler;
else if(element.onmousewheel !== undefined) element.onmousewheel = handler;
var wheelEventName = element.onwheel !== undefined ? 'wheel' : 'mousewheel';

if(element._onwheel) {
element.removeEventListener(wheelEventName, element._onwheel);
}
element._onwheel = handler;

element.addEventListener(wheelEventName, handler, {passive: false});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, this might break older browsers, see:

https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener#Safely_detecting_option_support

I couldn't find a list of which older browsers does not support {passive: true|false}, but we should probably play it safe here.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch @etpinard - do we need a has-passive ala https://github.com/dfcreative/has-hover? Or even better, a module that would figure this out once and then export the correct 3rd arg to mark the listener active or passive:

var eventOpts = require('event-opts');
element.addEventListener(wheelEventName, handler, eventOpts.active); // false or {passive: false}
element.addEventListener(wheelEventName, handler, eventOpts.passive); // false or {passive: true}

Copy link
Contributor Author

@dy dy Jan 18, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a polyfill default-passive-events for that. We could use detection logic from there in a way @alexcjohnson suggests.

Copy link
Contributor Author

@dy dy Jan 18, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@etpinard @alexcjohnson a32a21a introduces eventListenerOptionsSupported method to detect proper way to attach events.
To make code cleaner it is possible to introduce an event binding function or use package, like emmy or similar.

}

module.exports = {
Expand Down