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

@@ -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`.

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]);
+    }
   }
 
   /**

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() {