Skip to content

Commit 75bce9a

Browse files
authored
Fix removing event listeners (#541)
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.
1 parent 33ba619 commit 75bce9a

1 file changed

Lines changed: 15 additions & 18 deletions

File tree

src/handlers.js

Lines changed: 15 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,19 +3,20 @@ import {zoom, zoomRect} from './core';
33
import {callback as call} from 'chart.js/helpers';
44
import {getState} from './state';
55

6-
function removeHandler(chart, target, type) {
6+
function removeHandler(chart, type) {
77
const {handlers} = getState(chart);
88
const handler = handlers[type];
9-
if (handler) {
10-
target.removeEventListener(type, handler);
9+
if (handler && handler.target) {
10+
handler.target.removeEventListener(type, handler);
1111
delete handlers[type];
1212
}
1313
}
1414

1515
function addHandler(chart, target, type, handler) {
1616
const {handlers, options} = getState(chart);
17-
removeHandler(chart, target, type);
17+
removeHandler(chart, type);
1818
handlers[type] = (event) => handler(chart, event, options);
19+
handlers[type].target = target;
1920
target.addEventListener(type, handlers[type]);
2021
}
2122

@@ -95,7 +96,7 @@ export function mouseUp(chart, event) {
9596
return;
9697
}
9798

98-
removeHandler(chart.canvas, 'mousemove', chart);
99+
removeHandler(chart, 'mousemove');
99100
const {mode, onZoomComplete, drag: {threshold = 0}} = state.options.zoom;
100101
const rect = computeDragRect(chart, mode, state.dragStart, event);
101102
const distanceX = directionEnabled(mode, 'x', chart) ? rect.width : 0;
@@ -184,26 +185,22 @@ export function addListeners(chart, options) {
184185
addHandler(chart, canvas, 'wheel', wheel);
185186
addDebouncedHandler(chart, 'onZoomComplete', onZoomComplete, 250);
186187
} else {
187-
removeHandler(chart, canvas, 'wheel');
188+
removeHandler(chart, 'wheel');
188189
}
189190
if (dragOptions.enabled) {
190191
addHandler(chart, canvas, 'mousedown', mouseDown);
191192
addHandler(chart, canvas.ownerDocument, 'mouseup', mouseUp);
192193
} else {
193-
removeHandler(chart, canvas, 'mousedown');
194-
removeHandler(chart, canvas, 'mousemove');
195-
removeHandler(chart, canvas.ownerDocument, 'mouseup');
194+
removeHandler(chart, 'mousedown');
195+
removeHandler(chart, 'mousemove');
196+
removeHandler(chart, 'mouseup');
196197
}
197198
}
198199

199200
export function removeListeners(chart) {
200-
const {canvas} = chart;
201-
if (!canvas) {
202-
return;
203-
}
204-
removeHandler(chart, canvas, 'mousedown');
205-
removeHandler(chart, canvas, 'mousemove');
206-
removeHandler(chart, canvas.ownerDocument, 'mouseup');
207-
removeHandler(chart, canvas, 'wheel');
208-
removeHandler(chart, canvas, 'click');
201+
removeHandler(chart, 'mousedown');
202+
removeHandler(chart, 'mousemove');
203+
removeHandler(chart, 'mouseup');
204+
removeHandler(chart, 'wheel');
205+
removeHandler(chart, 'click');
209206
}

0 commit comments

Comments
 (0)