Skip to content

Commit

Permalink
Fix removing event listeners (#541)
Browse files Browse the repository at this point in the history
The plugin only removed its event listeners if it could access the chart's canvas.  However, Chart.js clears its canvas before notifying plugins, so this didn't work:

https://github.com/chartjs/Chart.js/blob/v3.3.2/src/core/core.controller.js#L857

I noticed this when I destroyed and recreated a Chart.js object against the same HTML `<canvas>` element; the original chart's event handlers still fired and threw errors because the `getState` object had been removed and no longer had needed information.

To address this, I'm now saving the event listener's target as a custom property on the event listener function; that way, `removeHandler` always knows exactly what the target is.

I also noticed that `mouseUp` was calling `removeHandler` with incorrect parameters, so I fixed it.
  • Loading branch information
joshkel authored Jun 8, 2021
1 parent 33ba619 commit 75bce9a
Showing 1 changed file with 15 additions and 18 deletions.
33 changes: 15 additions & 18 deletions src/handlers.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,20 @@ import {zoom, zoomRect} from './core';
import {callback as call} from 'chart.js/helpers';
import {getState} from './state';

function removeHandler(chart, target, type) {
function removeHandler(chart, type) {
const {handlers} = getState(chart);
const handler = handlers[type];
if (handler) {
target.removeEventListener(type, handler);
if (handler && handler.target) {
handler.target.removeEventListener(type, handler);
delete handlers[type];
}
}

function addHandler(chart, target, type, handler) {
const {handlers, options} = getState(chart);
removeHandler(chart, target, type);
removeHandler(chart, type);
handlers[type] = (event) => handler(chart, event, options);
handlers[type].target = target;
target.addEventListener(type, handlers[type]);
}

Expand Down Expand Up @@ -95,7 +96,7 @@ export function mouseUp(chart, event) {
return;
}

removeHandler(chart.canvas, 'mousemove', chart);
removeHandler(chart, 'mousemove');
const {mode, onZoomComplete, drag: {threshold = 0}} = state.options.zoom;
const rect = computeDragRect(chart, mode, state.dragStart, event);
const distanceX = directionEnabled(mode, 'x', chart) ? rect.width : 0;
Expand Down Expand Up @@ -184,26 +185,22 @@ export function addListeners(chart, options) {
addHandler(chart, canvas, 'wheel', wheel);
addDebouncedHandler(chart, 'onZoomComplete', onZoomComplete, 250);
} else {
removeHandler(chart, canvas, 'wheel');
removeHandler(chart, 'wheel');
}
if (dragOptions.enabled) {
addHandler(chart, canvas, 'mousedown', mouseDown);
addHandler(chart, canvas.ownerDocument, 'mouseup', mouseUp);
} else {
removeHandler(chart, canvas, 'mousedown');
removeHandler(chart, canvas, 'mousemove');
removeHandler(chart, canvas.ownerDocument, 'mouseup');
removeHandler(chart, 'mousedown');
removeHandler(chart, 'mousemove');
removeHandler(chart, 'mouseup');
}
}

export function removeListeners(chart) {
const {canvas} = chart;
if (!canvas) {
return;
}
removeHandler(chart, canvas, 'mousedown');
removeHandler(chart, canvas, 'mousemove');
removeHandler(chart, canvas.ownerDocument, 'mouseup');
removeHandler(chart, canvas, 'wheel');
removeHandler(chart, canvas, 'click');
removeHandler(chart, 'mousedown');
removeHandler(chart, 'mousemove');
removeHandler(chart, 'mouseup');
removeHandler(chart, 'wheel');
removeHandler(chart, 'click');
}

0 comments on commit 75bce9a

Please sign in to comment.