From adceef95876cd345317234683d9f6fe496e2eb50 Mon Sep 17 00:00:00 2001 From: Josh Kelley Date: Thu, 16 Sep 2021 15:43:39 -0400 Subject: [PATCH 1/4] 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. --- src/core/core.controller.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/core/core.controller.js b/src/core/core.controller.js index c5b77a0f989..bf23252780f 100644 --- a/src/core/core.controller.js +++ b/src/core/core.controller.js @@ -800,12 +800,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() { From 44094c3c4f96b4677f4ce60ece387ab71631c793 Mon Sep 17 00:00:00 2001 From: Josh Kelley Date: Tue, 12 Oct 2021 23:29:29 -0400 Subject: [PATCH 2/4] Add a test covering metaset behavior --- src/core/core.controller.js | 2 +- test/specs/core.controller.tests.js | 24 ++++++++++++++++++++++-- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/src/core/core.controller.js b/src/core/core.controller.js index bf23252780f..b9376617eb2 100644 --- a/src/core/core.controller.js +++ b/src/core/core.controller.js @@ -916,7 +916,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); diff --git a/test/specs/core.controller.tests.js b/test/specs/core.controller.tests.js index cb4eba08c07..48c1586a8cd 100644 --- a/test/specs/core.controller.tests.js +++ b/test/specs/core.controller.tests.js @@ -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.length).toHaveSize(0); + }); + }); + describe('data visibility', function() { it('should hide a dataset', function() { var chart = acquireChart({ From e27bdec97bca2a41a18029a6910d3ba27fe2dcc9 Mon Sep 17 00:00:00 2001 From: Josh Kelley Date: Wed, 13 Oct 2021 23:27:42 -0400 Subject: [PATCH 3/4] Add a regression test for #9653; fix `toHaveSize` usage --- test/specs/core.controller.tests.js | 2 +- test/specs/plugin.tooltip.tests.js | 42 +++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/test/specs/core.controller.tests.js b/test/specs/core.controller.tests.js index 48c1586a8cd..3b165b3b7dc 100644 --- a/test/specs/core.controller.tests.js +++ b/test/specs/core.controller.tests.js @@ -1926,7 +1926,7 @@ describe('Chart', function() { }); it('cleans up metasets when the chart is destroyed', function() { this.chart.destroy(); - expect(this.chart._metasets.length).toHaveSize(0); + expect(this.chart._metasets).toEqual([]); }); }); diff --git a/test/specs/plugin.tooltip.tests.js b/test/specs/plugin.tooltip.tests.js index 9cf619c5efa..ef0945d9537 100644 --- a/test/specs/plugin.tooltip.tests.js +++ b/test/specs/plugin.tooltip.tests.js @@ -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() { From e030d611275ad1e5a636df1e9858a57abfd35b76 Mon Sep 17 00:00:00 2001 From: Josh Kelley Date: Wed, 13 Oct 2021 23:57:52 -0400 Subject: [PATCH 4/4] Fix test failure --- test/specs/core.controller.tests.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/specs/core.controller.tests.js b/test/specs/core.controller.tests.js index 3b165b3b7dc..d9e7418f00b 100644 --- a/test/specs/core.controller.tests.js +++ b/test/specs/core.controller.tests.js @@ -1926,7 +1926,7 @@ describe('Chart', function() { }); it('cleans up metasets when the chart is destroyed', function() { this.chart.destroy(); - expect(this.chart._metasets).toEqual([]); + expect(this.chart._metasets).toEqual([undefined, undefined, undefined, undefined]); }); });