Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delay data to elements synchronization to update #9105

Merged
merged 1 commit into from May 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
56 changes: 39 additions & 17 deletions src/core/core.datasetController.js
Expand Up @@ -228,6 +228,7 @@ export default class DatasetController {
this._drawCount = undefined;
this.enableOptionSharing = false;
this.$context = undefined;
this._syncList = [];

this.initialize();
}
Expand All @@ -242,6 +243,9 @@ export default class DatasetController {
}

updateIndex(datasetIndex) {
if (this.index !== datasetIndex) {
clearStacks(this._cachedMeta);
}
this.index = datasetIndex;
}

Expand Down Expand Up @@ -316,21 +320,27 @@ export default class DatasetController {
const me = this;
const dataset = me.getDataset();
const data = dataset.data || (dataset.data = []);
const _data = me._data;

// In order to correctly handle data addition/deletion animation (an thus simulate
// real-time charts), we need to monitor these data modifications and synchronize
// the internal meta data accordingly.

if (isObject(data)) {
me._data = convertObjectDataToArray(data);
} else if (me._data !== data) {
if (me._data) {
} else if (_data !== data) {
if (_data) {
// This case happens when the user replaced the data array instance.
unlistenArrayEvents(me._data, me);
clearStacks(me._cachedMeta);
unlistenArrayEvents(_data, me);
// Discard old elements, parsed data and stacks
const meta = me._cachedMeta;
clearStacks(meta);
meta._parsed = [];
meta.data = [];
}
if (data && Object.isExtensible(data)) {
listenArrayEvents(data, me);
me._syncList = [];
}
me._data = data;
}
Expand All @@ -356,6 +366,7 @@ export default class DatasetController {
me._dataCheck();

// make sure cached _stacked status is current
const oldStacked = meta._stacked;
meta._stacked = isStacked(meta.vScale, meta);

// detect change in stack option
Expand All @@ -371,7 +382,7 @@ export default class DatasetController {
me._resyncElements(resetNewElements);

// if stack changed, update stack values for the whole dataset
if (stackChanged) {
if (stackChanged || oldStacked !== meta._stacked) {
updateStacks(me, meta._parsed);
}
}
Expand Down Expand Up @@ -905,17 +916,28 @@ export default class DatasetController {
*/
_resyncElements(resetNewElements) {
const me = this;
const numMeta = me._cachedMeta.data.length;
const numData = me._data.length;
const data = me._data;
const elements = me._cachedMeta.data;

// Apply changes detected through array listeners
for (const [method, arg1, arg2] of me._syncList) {
me[method](arg1, arg2);
}
me._syncList = [];

const numMeta = elements.length;
const numData = data.length;
const count = Math.min(numData, numMeta);

if (numData > numMeta) {
me._insertElements(numMeta, numData - numMeta, resetNewElements);
} else if (numData < numMeta) {
me._removeElements(numData, numMeta - numData);
}
// Re-parse the old elements (new elements are parsed in _insertElements)
const count = Math.min(numData, numMeta);
if (count) {
} else if (count) {
// TODO: It is not optimal to always parse the old data
// This is done because we are not detecting direct assignments:
// chart.data.datasets[0].data[5] = 10;
// chart.data.datasets[0].data[5].y = 10;
me.parse(0, count);
}
}
Expand Down Expand Up @@ -975,36 +997,36 @@ export default class DatasetController {
*/
_onDataPush() {
const count = arguments.length;
this._insertElements(this.getDataset().data.length - count, count);
this._syncList.push(['_insertElements', this.getDataset().data.length - count, count]);
}

/**
* @private
*/
_onDataPop() {
this._removeElements(this._cachedMeta.data.length - 1, 1);
this._syncList.push(['_removeElements', this._cachedMeta.data.length - 1, 1]);
}

/**
* @private
*/
_onDataShift() {
this._removeElements(0, 1);
this._syncList.push(['_removeElements', 0, 1]);
}

/**
* @private
*/
_onDataSplice(start, count) {
this._removeElements(start, count);
this._insertElements(start, arguments.length - 2);
this._syncList.push(['_removeElements', start, count]);
this._syncList.push(['_insertElements', start, arguments.length - 2]);
}

/**
* @private
*/
_onDataUnshift() {
this._insertElements(0, arguments.length);
this._syncList.push(['_insertElements', 0, arguments.length]);
}
}

Expand Down
11 changes: 11 additions & 0 deletions test/specs/core.datasetController.tests.js
Expand Up @@ -268,13 +268,15 @@ describe('Chart.DatasetController', function() {
last = meta.data[5];
data.push(6, 7, 8);
data.push(9);
chart.update();
expect(meta.data.length).toBe(10);
expect(meta.data[0]).toBe(first);
expect(meta.data[5]).toBe(last);
expect(parsedYValues()).toEqual(data);

last = meta.data[9];
data.pop();
chart.update();
expect(meta.data.length).toBe(9);
expect(meta.data[0]).toBe(first);
expect(meta.data.indexOf(last)).toBe(-1);
Expand All @@ -284,6 +286,7 @@ describe('Chart.DatasetController', function() {
data.shift();
data.shift();
data.shift();
chart.update();
expect(meta.data.length).toBe(6);
expect(meta.data.indexOf(first)).toBe(-1);
expect(meta.data[5]).toBe(last);
Expand All @@ -293,6 +296,7 @@ describe('Chart.DatasetController', function() {
second = meta.data[1];
last = meta.data[5];
data.splice(1, 4, 10, 11);
chart.update();
expect(meta.data.length).toBe(4);
expect(meta.data[0]).toBe(first);
expect(meta.data[3]).toBe(last);
Expand All @@ -301,6 +305,7 @@ describe('Chart.DatasetController', function() {

data.unshift(12, 13, 14, 15);
data.unshift(16, 17);
chart.update();
expect(meta.data.length).toBe(10);
expect(meta.data[6]).toBe(first);
expect(meta.data[9]).toBe(last);
Expand Down Expand Up @@ -333,12 +338,14 @@ describe('Chart.DatasetController', function() {
last = controller.getParsed(5);
data.push({x: 6, y: 6}, {x: 7, y: 7}, {x: 8, y: 8});
data.push({x: 9, y: 9});
chart.update();
expect(meta.data.length).toBe(10);
expect(controller.getParsed(0)).toBe(first);
expect(controller.getParsed(5)).toBe(last);

last = controller.getParsed(9);
data.pop();
chart.update();
expect(meta.data.length).toBe(9);
expect(controller.getParsed(0)).toBe(first);
expect(controller.getParsed(9)).toBe(undefined);
Expand All @@ -348,19 +355,22 @@ describe('Chart.DatasetController', function() {
data.shift();
data.shift();
data.shift();
chart.update();
expect(meta.data.length).toBe(6);
expect(controller.getParsed(5)).toBe(last);

first = controller.getParsed(0);
last = controller.getParsed(5);
data.splice(1, 4, {x: 10, y: 10}, {x: 11, y: 11});
chart.update();
expect(meta.data.length).toBe(4);
expect(controller.getParsed(0)).toBe(first);
expect(controller.getParsed(3)).toBe(last);
expect(controller.getParsed(1)).toEqual({x: 10, y: 10});

data.unshift({x: 12, y: 12}, {x: 13, y: 13}, {x: 14, y: 14}, {x: 15, y: 15});
data.unshift({x: 16, y: 16}, {x: 17, y: 17});
chart.update();
expect(meta.data.length).toBe(10);
expect(controller.getParsed(6)).toBe(first);
expect(controller.getParsed(9)).toBe(last);
Expand Down Expand Up @@ -390,6 +400,7 @@ describe('Chart.DatasetController', function() {
expect(meta._parsed.map(p => p.y)).toEqual(data1);

data1.push(9);
chart.update();
expect(meta.data.length).toBe(4);

chart.data.datasets[0].data = data0;
Expand Down