Fix cleaning up metasets (#9656)

* Fix cleaning up metasets

I believe it's a mistake to only delete the metaset if it has a valid controller; see f191f2f5 for where this behavior was introduced.

This is a minimal fix for #9653; as discussed there, it may also be worth updating `updateHoverStyle`.

As of #7030, `this._metasets` should always be defined, so checking whether it's undefined is no longer necessary.

* Add a test covering metaset behavior

* Add a regression test for #9653; fix `toHaveSize` usage

* Fix test failure
This commit is contained in:
Josh Kelley
2021-10-23 11:46:33 -04:00
committed by GitHub
parent 1199415e25
commit aac0bef060
3 changed files with 67 additions and 6 deletions

View File

@@ -799,12 +799,11 @@ class Chart {
* @private
*/
_destroyDatasetMeta(datasetIndex) {
const meta = this._metasets && this._metasets[datasetIndex];
const meta = this._metasets[datasetIndex];
if (meta && meta.controller) {
meta.controller._destroy();
delete this._metasets[datasetIndex];
}
delete this._metasets[datasetIndex];
}
_stop() {
@@ -916,7 +915,7 @@ class Chart {
_remove('resize', listener);
// Stop animating and remove metasets, so when re-attached, the animations start from begining.
// Stop animating and remove metasets, so when re-attached, the animations start from beginning.
this._stop();
this._resize(0, 0);

View File

@@ -1121,7 +1121,7 @@ describe('Chart', function() {
});
});
it('should resize the canvas if attached to the DOM after construction with mutiple parents', function(done) {
it('should resize the canvas if attached to the DOM after construction with multiple parents', function(done) {
var canvas = document.createElement('canvas');
var wrapper = document.createElement('div');
var wrapper2 = document.createElement('div');
@@ -1848,7 +1848,7 @@ describe('Chart', function() {
expect(metasets[2].order).toEqual(3);
expect(metasets[3].order).toEqual(4);
});
it('should be moved when datasets are removed from begining', function() {
it('should be moved when datasets are removed from beginning', function() {
this.chart.data.datasets.splice(0, 2);
this.chart.update();
const metasets = this.chart._metasets;
@@ -1910,6 +1910,26 @@ describe('Chart', function() {
});
});
describe('_destroyDatasetMeta', function() {
beforeEach(function() {
this.chart = acquireChart({
type: 'line',
data: {
datasets: [
{label: '1', order: 2},
{label: '2', order: 1},
{label: '3', order: 4},
{label: '4', order: 3},
]
}
});
});
it('cleans up metasets when the chart is destroyed', function() {
this.chart.destroy();
expect(this.chart._metasets).toEqual([undefined, undefined, undefined, undefined]);
});
});
describe('data visibility', function() {
it('should hide a dataset', function() {
var chart = acquireChart({

View File

@@ -1555,6 +1555,48 @@ describe('Plugin.Tooltip', function() {
chart.tooltip.setActiveElements([{datasetIndex: 0, index: 0}], {x: 0, y: 0});
expect(chart.tooltip.getActiveElements()[0].element).toBe(meta.data[0]);
});
it('should update active elements when datasets are removed and added', async function() {
var dataset = {
label: 'Dataset 1',
data: [10, 20, 30],
pointHoverBorderColor: 'rgb(255, 0, 0)',
pointHoverBackgroundColor: 'rgb(0, 255, 0)'
};
var chart = window.acquireChart({
type: 'line',
data: {
datasets: [dataset],
labels: ['Point 1', 'Point 2', 'Point 3']
},
options: {
plugins: {
tooltip: {
mode: 'nearest',
intersect: true
}
}
}
});
var meta = chart.getDatasetMeta(0);
var point = meta.data[1];
var expectedPoint = jasmine.objectContaining({datasetIndex: 0, index: 1});
await jasmine.triggerMouseEvent(chart, 'mousemove', point);
expect(chart.tooltip.getActiveElements()).toEqual([expectedPoint]);
chart.data.datasets = [];
chart.update();
expect(chart.tooltip.getActiveElements()).toEqual([]);
chart.data.datasets = [dataset];
chart.update();
expect(chart.tooltip.getActiveElements()).toEqual([expectedPoint]);
});
});
describe('events', function() {