Allow switching between wheel/drag modes of the zoom #196
Conversation
src/plugin.js
Outdated
| node.addEventListener('mousemove', chartInstance.zoom._mouseMoveHandler); | ||
|
|
||
| chartInstance.zoom._mouseUpHandler = function(event) { | ||
| if (chartInstance.options.zoom.drag && chartInstance.zoom._dragZoomStart) { |
There was a problem hiding this comment.
I'd probably prefer to do the inverse to save from having to indent the rest of the code as much:
if (!chartInstance.options.zoom.drag || !chartInstance.zoom._dragZoomStart) {
return;
}
There was a problem hiding this comment.
But probably only for a longer method like this one. For a method that's only a few lines, what you have now is shorter
There was a problem hiding this comment.
I fixed the mouseup handler as suggested, I also prefer that.
src/plugin.js
Outdated
| chartInstance.update(0); | ||
| } | ||
| }; | ||
| node.addEventListener('mousemove', chartInstance.zoom._mouseMoveHandler); |
There was a problem hiding this comment.
I forgot we have a mousemove handler. mousemove can fire hundreds of times a second constantly, so we should be a lot more careful than with mouseup, mousedown, etc. I'd be more comfortable for performance reasons if went the route of switching the listeners on and off instead eventhough it would be a bit more complicated.
There was a problem hiding this comment.
Good point! I fixed and now add the mousemove listener on mousedown. Other listeners stay initialized as it is.
Enable switching between wheel/drag modes of the zoom
2 participants
Closes #176
This PR allows switching between wheel/drag modes of the zoom (if
options.zoom.dragis changed, it will be taken into account in next events).beforeInitmethod, and theoptions.zoom.dragsetting is checked in the body of these listeners rather than at the initialization.Enable/disable drag modein thezoom-timesample to test this improvement.Reset Zoombutton in thezoom-timesample.