Fix detecting changed events (#9050)

* Fix detecting changed events

Because `this._listeners` may contain both event handlers from options and internal event handlers for responsive support, the `setsEqual` check would often fail, causing event handlers to be unnecessarily detached and reattached and fired.

If I'm understanding correctly, this is the root cause of #9049.

* Use a separate object for responsive listeners

Correctly update events when responsive property changes as well as when requested events change.

* Code review feedback
This commit is contained in:
Josh Kelley 2021-05-10 08:48:03 -04:00 committed by GitHub
parent c955ffad64
commit 1df4883aff
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

View File

@ -116,8 +116,9 @@ class Chart {
this.chartArea = undefined;
this._active = [];
this._lastEvent = undefined;
/** @type {{attach?: function, detach?: function, resize?: function}} */
this._listeners = {};
/** @type {?{attach?: function, detach?: function, resize?: function}} */
this._responsiveListeners = undefined;
this._sortedMetasets = [];
this.scales = {};
this.scale = undefined;
@ -478,8 +479,8 @@ class Chart {
const existingEvents = new Set(Object.keys(me._listeners));
const newEvents = new Set(me.options.events);
if (!setsEqual(existingEvents, newEvents)) {
// The events array has changed. Rebind it
if (!setsEqual(existingEvents, newEvents) || !!this._responsiveListeners !== me.options.responsive) {
// The configured events have changed. Rebind.
me.unbindEvents();
me.bindEvents();
}
@ -889,10 +890,47 @@ class Chart {
* @private
*/
bindEvents() {
this.bindUserEvents();
if (this.options.responsive) {
this.bindResponsiveEvents();
} else {
this.attached = true;
}
}
/**
* @private
*/
bindUserEvents() {
const me = this;
const listeners = me._listeners;
const platform = me.platform;
const _add = (type, listener) => {
platform.addEventListener(me, type, listener);
listeners[type] = listener;
};
const listener = function(e, x, y) {
e.offsetX = x;
e.offsetY = y;
me._eventHandler(e);
};
each(me.options.events, (type) => _add(type, listener));
}
/**
* @private
*/
bindResponsiveEvents() {
const me = this;
if (!me._responsiveListeners) {
me._responsiveListeners = {};
}
const listeners = me._responsiveListeners;
const platform = me.platform;
const _add = (type, listener) => {
platform.addEventListener(me, type, listener);
listeners[type] = listener;
@ -904,16 +942,7 @@ class Chart {
}
};
let listener = function(e, x, y) {
e.offsetX = x;
e.offsetY = y;
me._eventHandler(e);
};
each(me.options.events, (type) => _add(type, listener));
if (me.options.responsive) {
listener = (width, height) => {
const listener = (width, height) => {
if (me.canvas) {
me.resize(width, height);
}
@ -942,9 +971,6 @@ class Chart {
} else {
detached();
}
} else {
me.attached = true;
}
}
/**
@ -952,15 +978,16 @@ class Chart {
*/
unbindEvents() {
const me = this;
const listeners = me._listeners;
if (!listeners) {
return;
}
me._listeners = {};
each(listeners, (listener, type) => {
each(me._listeners, (listener, type) => {
me.platform.removeEventListener(me, type, listener);
});
me._listeners = {};
each(me._responsiveListeners, (listener, type) => {
me.platform.removeEventListener(me, type, listener);
});
me._responsiveListeners = undefined;
}
updateHoverStyle(items, mode, enabled) {