Skip to content

Commit 838d40b

Browse files
authored
Synchronize data visibility with data changes (#9857)
* Synchronize data visibility with data changes * avoid babel spread bug * Simpler? * one more * simple enough, cc?
1 parent 0c5db49 commit 838d40b

File tree

3 files changed

+190
-19
lines changed

3 files changed

+190
-19
lines changed

src/core/core.controller.js

Lines changed: 86 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,21 @@ const getChart = (key) => {
6969
return Object.values(instances).filter((c) => c.canvas === canvas).pop();
7070
};
7171

72+
function moveNumericKeys(obj, start, move) {
73+
const keys = Object.keys(obj);
74+
for (const key of keys) {
75+
const intKey = +key;
76+
if (intKey >= start) {
77+
const value = obj[key];
78+
delete obj[key];
79+
if (move > 0 || intKey > start) {
80+
obj[intKey + move] = value;
81+
}
82+
}
83+
}
84+
}
85+
86+
7287
class Chart {
7388

7489
// eslint-disable-next-line max-statements
@@ -123,6 +138,7 @@ class Chart {
123138
this._animationsDisabled = undefined;
124139
this.$context = undefined;
125140
this._doResize = debounce(mode => this.update(mode), options.resizeDelay || 0);
141+
this._dataChanges = [];
126142

127143
// Add the chart instance to the global namespace
128144
instances[this.id] = this;
@@ -424,24 +440,11 @@ class Chart {
424440

425441
config.update();
426442
const options = this._options = config.createResolver(config.chartOptionScopes(), this.getContext());
427-
428-
each(this.scales, (scale) => {
429-
layouts.removeBox(this, scale);
430-
});
431-
432443
const animsDisabled = this._animationsDisabled = !options.animation;
433444

434-
this.ensureScalesHaveIDs();
435-
this.buildOrUpdateScales();
436-
437-
const existingEvents = new Set(Object.keys(this._listeners));
438-
const newEvents = new Set(options.events);
439-
440-
if (!setsEqual(existingEvents, newEvents) || !!this._responsiveListeners !== options.responsive) {
441-
// The configured events have changed. Rebind.
442-
this.unbindEvents();
443-
this.bindEvents();
444-
}
445+
this._updateScales();
446+
this._checkEventBindings();
447+
this._updateHiddenIndices();
445448

446449
// plugins options references might have change, let's invalidate the cache
447450
// https://github.com/chartjs/Chart.js/issues/5111#issuecomment-355934167
@@ -493,6 +496,73 @@ class Chart {
493496
this.render();
494497
}
495498

499+
/**
500+
* @private
501+
*/
502+
_updateScales() {
503+
each(this.scales, (scale) => {
504+
layouts.removeBox(this, scale);
505+
});
506+
507+
this.ensureScalesHaveIDs();
508+
this.buildOrUpdateScales();
509+
}
510+
511+
/**
512+
* @private
513+
*/
514+
_checkEventBindings() {
515+
const options = this.options;
516+
const existingEvents = new Set(Object.keys(this._listeners));
517+
const newEvents = new Set(options.events);
518+
519+
if (!setsEqual(existingEvents, newEvents) || !!this._responsiveListeners !== options.responsive) {
520+
// The configured events have changed. Rebind.
521+
this.unbindEvents();
522+
this.bindEvents();
523+
}
524+
}
525+
526+
/**
527+
* @private
528+
*/
529+
_updateHiddenIndices() {
530+
const {_hiddenIndices} = this;
531+
const changes = this._getUniformDataChanges() || [];
532+
for (const {method, start, count} of changes) {
533+
const move = method === '_removeElements' ? -count : count;
534+
moveNumericKeys(_hiddenIndices, start, move);
535+
}
536+
}
537+
538+
/**
539+
* @private
540+
*/
541+
_getUniformDataChanges() {
542+
const _dataChanges = this._dataChanges;
543+
if (!_dataChanges || !_dataChanges.length) {
544+
return;
545+
}
546+
547+
this._dataChanges = [];
548+
const datasetCount = this.data.datasets.length;
549+
const makeSet = (idx) => new Set(
550+
_dataChanges
551+
.filter(c => c[0] === idx)
552+
.map((c, i) => i + ',' + c.splice(1).join(','))
553+
);
554+
555+
const changeSet = makeSet(0);
556+
for (let i = 1; i < datasetCount; i++) {
557+
if (!setsEqual(changeSet, makeSet(i))) {
558+
return;
559+
}
560+
}
561+
return Array.from(changeSet)
562+
.map(c => c.split(','))
563+
.map(a => ({method: a[1], start: +a[2], count: +a[3]}));
564+
}
565+
496566
/**
497567
* Updates the chart layout unless a plugin returns `false` to the `beforeLayout`
498568
* hook, in which case, plugins will not be called on `afterLayout`.

src/core/core.datasetController.js

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -982,16 +982,19 @@ export default class DatasetController {
982982
meta.data.splice(start, count);
983983
}
984984

985+
/**
986+
* @private
987+
*/
985988
_sync(args) {
986989
if (this._parsing) {
987990
this._syncList.push(args);
988991
} else {
989992
const [method, arg1, arg2] = args;
990993
this[method](arg1, arg2);
991994
}
995+
this.chart._dataChanges.push([this.index, ...args]);
992996
}
993997

994-
995998
/**
996999
* @private
9971000
*/
@@ -1018,8 +1021,13 @@ export default class DatasetController {
10181021
* @private
10191022
*/
10201023
_onDataSplice(start, count) {
1021-
this._sync(['_removeElements', start, count]);
1022-
this._sync(['_insertElements', start, arguments.length - 2]);
1024+
if (count) {
1025+
this._sync(['_removeElements', start, count]);
1026+
}
1027+
const newCount = arguments.length - 2;
1028+
if (newCount) {
1029+
this._sync(['_insertElements', start, newCount]);
1030+
}
10231031
}
10241032

10251033
/**

test/specs/core.controller.tests.js

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1966,6 +1966,99 @@ describe('Chart', function() {
19661966
chart.update();
19671967
expect(chart.getDataVisibility(1)).toBe(false);
19681968
});
1969+
1970+
it('should maintain data visibility indices when data changes', function() {
1971+
var chart = acquireChart({
1972+
type: 'pie',
1973+
data: {
1974+
labels: ['0', '1', '2', '3'],
1975+
datasets: [{
1976+
data: [0, 1, 2, 3]
1977+
}, {
1978+
data: [0, 1, 2, 3]
1979+
}]
1980+
}
1981+
});
1982+
1983+
chart.toggleDataVisibility(3);
1984+
1985+
chart.data.labels.splice(1, 1);
1986+
chart.data.datasets[0].data.splice(1, 1);
1987+
chart.data.datasets[1].data.splice(1, 1);
1988+
chart.update();
1989+
1990+
expect(chart.getDataVisibility(0)).toBe(true);
1991+
expect(chart.getDataVisibility(1)).toBe(true);
1992+
expect(chart.getDataVisibility(2)).toBe(false);
1993+
1994+
chart.data.labels.unshift('-1', '-2');
1995+
chart.data.datasets[0].data.unshift(-1, -2);
1996+
chart.data.datasets[1].data.unshift(-1, -2);
1997+
chart.update();
1998+
1999+
expect(chart.getDataVisibility(0)).toBe(true);
2000+
expect(chart.getDataVisibility(1)).toBe(true);
2001+
expect(chart.getDataVisibility(2)).toBe(true);
2002+
expect(chart.getDataVisibility(3)).toBe(true);
2003+
expect(chart.getDataVisibility(4)).toBe(false);
2004+
2005+
chart.data.labels.shift();
2006+
chart.data.datasets[0].data.shift();
2007+
chart.data.datasets[1].data.shift();
2008+
chart.update();
2009+
2010+
expect(chart.getDataVisibility(0)).toBe(true);
2011+
expect(chart.getDataVisibility(1)).toBe(true);
2012+
expect(chart.getDataVisibility(2)).toBe(true);
2013+
expect(chart.getDataVisibility(3)).toBe(false);
2014+
2015+
chart.data.labels.pop();
2016+
chart.data.datasets[0].data.pop();
2017+
chart.data.datasets[1].data.pop();
2018+
chart.update();
2019+
2020+
expect(chart.getDataVisibility(0)).toBe(true);
2021+
expect(chart.getDataVisibility(1)).toBe(true);
2022+
expect(chart.getDataVisibility(2)).toBe(true);
2023+
expect(chart.getDataVisibility(3)).toBe(true);
2024+
2025+
chart.toggleDataVisibility(1);
2026+
chart.data.labels.splice(1, 0, 'b');
2027+
chart.data.datasets[0].data.splice(1, 0, 1);
2028+
chart.data.datasets[1].data.splice(1, 0, 1);
2029+
chart.update();
2030+
2031+
expect(chart.getDataVisibility(0)).toBe(true);
2032+
expect(chart.getDataVisibility(1)).toBe(true);
2033+
expect(chart.getDataVisibility(2)).toBe(false);
2034+
expect(chart.getDataVisibility(3)).toBe(true);
2035+
});
2036+
2037+
it('should leave data visibility indices intact when data changes in non-uniform way', function() {
2038+
var chart = acquireChart({
2039+
type: 'pie',
2040+
data: {
2041+
labels: ['0', '1', '2', '3'],
2042+
datasets: [{
2043+
data: [0, 1, 2, 3]
2044+
}, {
2045+
data: [0, 1, 2, 3]
2046+
}]
2047+
}
2048+
});
2049+
2050+
chart.toggleDataVisibility(0);
2051+
2052+
chart.data.labels.push('a');
2053+
chart.data.datasets[0].data.pop();
2054+
chart.data.datasets[1].data.push(5);
2055+
chart.update();
2056+
2057+
expect(chart.getDataVisibility(0)).toBe(false);
2058+
expect(chart.getDataVisibility(1)).toBe(true);
2059+
expect(chart.getDataVisibility(2)).toBe(true);
2060+
expect(chart.getDataVisibility(3)).toBe(true);
2061+
});
19692062
});
19702063

19712064
describe('isDatasetVisible', function() {

0 commit comments

Comments
 (0)