From 838d40b2c832e8ae1a8caeeba508714d7b647581 Mon Sep 17 00:00:00 2001 From: Jukka Kurkela Date: Thu, 18 Nov 2021 00:13:31 +0200 Subject: [PATCH] Synchronize data visibility with data changes (#9857) * Synchronize data visibility with data changes * avoid babel spread bug * Simpler? * one more * simple enough, cc? --- src/core/core.controller.js | 102 +++++++++++++++++++++++----- src/core/core.datasetController.js | 14 +++- test/specs/core.controller.tests.js | 93 +++++++++++++++++++++++++ 3 files changed, 190 insertions(+), 19 deletions(-) diff --git a/src/core/core.controller.js b/src/core/core.controller.js index cfa3ad937..e7621f8c9 100644 --- a/src/core/core.controller.js +++ b/src/core/core.controller.js @@ -69,6 +69,21 @@ const getChart = (key) => { return Object.values(instances).filter((c) => c.canvas === canvas).pop(); }; +function moveNumericKeys(obj, start, move) { + const keys = Object.keys(obj); + for (const key of keys) { + const intKey = +key; + if (intKey >= start) { + const value = obj[key]; + delete obj[key]; + if (move > 0 || intKey > start) { + obj[intKey + move] = value; + } + } + } +} + + class Chart { // eslint-disable-next-line max-statements @@ -123,6 +138,7 @@ class Chart { this._animationsDisabled = undefined; this.$context = undefined; this._doResize = debounce(mode => this.update(mode), options.resizeDelay || 0); + this._dataChanges = []; // Add the chart instance to the global namespace instances[this.id] = this; @@ -424,24 +440,11 @@ class Chart { config.update(); const options = this._options = config.createResolver(config.chartOptionScopes(), this.getContext()); - - each(this.scales, (scale) => { - layouts.removeBox(this, scale); - }); - const animsDisabled = this._animationsDisabled = !options.animation; - this.ensureScalesHaveIDs(); - this.buildOrUpdateScales(); - - const existingEvents = new Set(Object.keys(this._listeners)); - const newEvents = new Set(options.events); - - if (!setsEqual(existingEvents, newEvents) || !!this._responsiveListeners !== options.responsive) { - // The configured events have changed. Rebind. - this.unbindEvents(); - this.bindEvents(); - } + this._updateScales(); + this._checkEventBindings(); + this._updateHiddenIndices(); // plugins options references might have change, let's invalidate the cache // https://github.com/chartjs/Chart.js/issues/5111#issuecomment-355934167 @@ -493,6 +496,73 @@ class Chart { this.render(); } + /** + * @private + */ + _updateScales() { + each(this.scales, (scale) => { + layouts.removeBox(this, scale); + }); + + this.ensureScalesHaveIDs(); + this.buildOrUpdateScales(); + } + + /** + * @private + */ + _checkEventBindings() { + const options = this.options; + const existingEvents = new Set(Object.keys(this._listeners)); + const newEvents = new Set(options.events); + + if (!setsEqual(existingEvents, newEvents) || !!this._responsiveListeners !== options.responsive) { + // The configured events have changed. Rebind. + this.unbindEvents(); + this.bindEvents(); + } + } + + /** + * @private + */ + _updateHiddenIndices() { + const {_hiddenIndices} = this; + const changes = this._getUniformDataChanges() || []; + for (const {method, start, count} of changes) { + const move = method === '_removeElements' ? -count : count; + moveNumericKeys(_hiddenIndices, start, move); + } + } + + /** + * @private + */ + _getUniformDataChanges() { + const _dataChanges = this._dataChanges; + if (!_dataChanges || !_dataChanges.length) { + return; + } + + this._dataChanges = []; + const datasetCount = this.data.datasets.length; + const makeSet = (idx) => new Set( + _dataChanges + .filter(c => c[0] === idx) + .map((c, i) => i + ',' + c.splice(1).join(',')) + ); + + const changeSet = makeSet(0); + for (let i = 1; i < datasetCount; i++) { + if (!setsEqual(changeSet, makeSet(i))) { + return; + } + } + return Array.from(changeSet) + .map(c => c.split(',')) + .map(a => ({method: a[1], start: +a[2], count: +a[3]})); + } + /** * Updates the chart layout unless a plugin returns `false` to the `beforeLayout` * hook, in which case, plugins will not be called on `afterLayout`. diff --git a/src/core/core.datasetController.js b/src/core/core.datasetController.js index ccbba2320..8ba52e169 100644 --- a/src/core/core.datasetController.js +++ b/src/core/core.datasetController.js @@ -982,6 +982,9 @@ export default class DatasetController { meta.data.splice(start, count); } + /** + * @private + */ _sync(args) { if (this._parsing) { this._syncList.push(args); @@ -989,9 +992,9 @@ export default class DatasetController { const [method, arg1, arg2] = args; this[method](arg1, arg2); } + this.chart._dataChanges.push([this.index, ...args]); } - /** * @private */ @@ -1018,8 +1021,13 @@ export default class DatasetController { * @private */ _onDataSplice(start, count) { - this._sync(['_removeElements', start, count]); - this._sync(['_insertElements', start, arguments.length - 2]); + if (count) { + this._sync(['_removeElements', start, count]); + } + const newCount = arguments.length - 2; + if (newCount) { + this._sync(['_insertElements', start, newCount]); + } } /** diff --git a/test/specs/core.controller.tests.js b/test/specs/core.controller.tests.js index d9e7418f0..de338b580 100644 --- a/test/specs/core.controller.tests.js +++ b/test/specs/core.controller.tests.js @@ -1966,6 +1966,99 @@ describe('Chart', function() { chart.update(); expect(chart.getDataVisibility(1)).toBe(false); }); + + it('should maintain data visibility indices when data changes', function() { + var chart = acquireChart({ + type: 'pie', + data: { + labels: ['0', '1', '2', '3'], + datasets: [{ + data: [0, 1, 2, 3] + }, { + data: [0, 1, 2, 3] + }] + } + }); + + chart.toggleDataVisibility(3); + + chart.data.labels.splice(1, 1); + chart.data.datasets[0].data.splice(1, 1); + chart.data.datasets[1].data.splice(1, 1); + chart.update(); + + expect(chart.getDataVisibility(0)).toBe(true); + expect(chart.getDataVisibility(1)).toBe(true); + expect(chart.getDataVisibility(2)).toBe(false); + + chart.data.labels.unshift('-1', '-2'); + chart.data.datasets[0].data.unshift(-1, -2); + chart.data.datasets[1].data.unshift(-1, -2); + chart.update(); + + expect(chart.getDataVisibility(0)).toBe(true); + expect(chart.getDataVisibility(1)).toBe(true); + expect(chart.getDataVisibility(2)).toBe(true); + expect(chart.getDataVisibility(3)).toBe(true); + expect(chart.getDataVisibility(4)).toBe(false); + + chart.data.labels.shift(); + chart.data.datasets[0].data.shift(); + chart.data.datasets[1].data.shift(); + chart.update(); + + expect(chart.getDataVisibility(0)).toBe(true); + expect(chart.getDataVisibility(1)).toBe(true); + expect(chart.getDataVisibility(2)).toBe(true); + expect(chart.getDataVisibility(3)).toBe(false); + + chart.data.labels.pop(); + chart.data.datasets[0].data.pop(); + chart.data.datasets[1].data.pop(); + chart.update(); + + expect(chart.getDataVisibility(0)).toBe(true); + expect(chart.getDataVisibility(1)).toBe(true); + expect(chart.getDataVisibility(2)).toBe(true); + expect(chart.getDataVisibility(3)).toBe(true); + + chart.toggleDataVisibility(1); + chart.data.labels.splice(1, 0, 'b'); + chart.data.datasets[0].data.splice(1, 0, 1); + chart.data.datasets[1].data.splice(1, 0, 1); + chart.update(); + + expect(chart.getDataVisibility(0)).toBe(true); + expect(chart.getDataVisibility(1)).toBe(true); + expect(chart.getDataVisibility(2)).toBe(false); + expect(chart.getDataVisibility(3)).toBe(true); + }); + + it('should leave data visibility indices intact when data changes in non-uniform way', function() { + var chart = acquireChart({ + type: 'pie', + data: { + labels: ['0', '1', '2', '3'], + datasets: [{ + data: [0, 1, 2, 3] + }, { + data: [0, 1, 2, 3] + }] + } + }); + + chart.toggleDataVisibility(0); + + chart.data.labels.push('a'); + chart.data.datasets[0].data.pop(); + chart.data.datasets[1].data.push(5); + chart.update(); + + expect(chart.getDataVisibility(0)).toBe(false); + expect(chart.getDataVisibility(1)).toBe(true); + expect(chart.getDataVisibility(2)).toBe(true); + expect(chart.getDataVisibility(3)).toBe(true); + }); }); describe('isDatasetVisible', function() { -- 2.47.2