From 73541b40d0edc63f6080a5a78d402646c8df1bf2 Mon Sep 17 00:00:00 2001 From: stockiNail Date: Wed, 1 Mar 2023 15:06:22 +0100 Subject: [PATCH 01/10] Enable animation on inner nodes of the element and options --- src/core/core.animations.js | 119 ++++++++++++++++++++++------ test/specs/core.animations.tests.js | 78 ++++++++++++++++++ 2 files changed, 172 insertions(+), 25 deletions(-) diff --git a/src/core/core.animations.js b/src/core/core.animations.js index 4ee61b84b0d..e40c021bab0 100644 --- a/src/core/core.animations.js +++ b/src/core/core.animations.js @@ -1,12 +1,13 @@ import animator from './core.animator.js'; import Animation from './core.animation.js'; import defaults from './core.defaults.js'; -import {isArray, isObject} from '../helpers/helpers.core.js'; +import {isArray, isObject, _splitKey} from '../helpers/helpers.core.js'; export default class Animations { constructor(chart, config) { this._chart = chart; this._properties = new Map(); + this._pathProperties = []; this.configure(config); } @@ -34,6 +35,14 @@ export default class Animations { } }); }); + const pathAnimatedProps = this._pathProperties; + animatedProps.forEach(function(v, k) { + const value = parserPathOptions(k); + if (value) { + pathAnimatedProps.push(value); + } + }); + } /** @@ -67,23 +76,15 @@ export default class Animations { */ _createAnimations(target, values) { const animatedProps = this._properties; + const pathAnimatedProps = this._pathProperties; const animations = []; const running = target.$animations || (target.$animations = {}); const props = Object.keys(values); const date = Date.now(); - let i; - - for (i = props.length - 1; i >= 0; --i) { - const prop = props[i]; - if (prop.charAt(0) === '$') { - continue; - } - if (prop === 'options') { - animations.push(...this._animateOptions(target, values)); - continue; - } - const value = values[prop]; + const manageItem = function(tgt, vals, prop, subProp) { + const key = subProp || prop; + const value = vals[key]; let animation = running[prop]; const cfg = animatedProps.get(prop); @@ -91,30 +92,52 @@ export default class Animations { if (cfg && animation.active()) { // There is an existing active animation, let's update that animation.update(cfg, value, date); - continue; - } else { - animation.cancel(); + return; } + animation.cancel(); } if (!cfg || !cfg.duration) { // not animated, set directly to new value - target[prop] = value; - continue; + tgt[key] = value; + return; } - - running[prop] = animation = new Animation(cfg, target, prop, value); + running[prop] = animation = new Animation(cfg, tgt, key, value); animations.push(animation); + }; + + let i; + for (i = props.length - 1; i >= 0; --i) { + const prop = props[i]; + if (prop.charAt(0) === '$') { + continue; + } + if (prop === 'options') { + animations.push(...this._animateOptions(target, values)); + continue; + } + const matched = pathAnimatedProps.filter(item => item.prop.startsWith(prop)); + if (matched.length) { + matched.forEach(function(item) { + const newTarget = getInnerObject(target, item); + const newValues = getInnerObject(values, item); + if (newTarget && newValues) { + manageItem(newTarget, newValues, item.prop, item.key); + } + }); + } else { + manageItem(target, values, prop); + } } return animations; } /** - * Update `target` properties to new values, using configured animations - * @param {object} target - object to update - * @param {object} values - new target properties - * @returns {boolean|undefined} - `true` if animations were started - **/ + * Update `target` properties to new values, using configured animations + * @param {object} target - object to update + * @param {object} values - new target properties + * @returns {boolean|undefined} - `true` if animations were started + **/ update(target, values) { if (this._properties.size === 0) { // Nothing is animated, just apply the new values. @@ -160,3 +183,49 @@ function resolveTargetOptions(target, newOptions) { } return options; } + +function parserPathOptions(key) { + if (key.includes('.')) { + const keys = _splitKey(key); + if (keys.length <= 1) { + return; + } + return parseKeys(key, keys); + } +} + +function parseKeys(key, keys) { + const result = { + prop: key, + path: [] + }; + for (let i = 0, n = keys.length; i < n; i++) { + const k = keys[i]; + if (!k.trim().length) { // empty string + return; + } + if (i === (n - 1)) { + result.key = k; + } else { + result.path.push(k); + } + } + return result; +} + +function getInnerObject(target, pathOpts) { + let obj = target; + for (const p of pathOpts.path) { + obj = checkAndGetObject(obj, p); + if (!obj) { + return; + } + } + return obj; +} + +function checkAndGetObject(obj, key) { + if (isObject(obj[key])) { + return obj[key]; + } +} diff --git a/test/specs/core.animations.tests.js b/test/specs/core.animations.tests.js index 6fddb20445f..246cd02325a 100644 --- a/test/specs/core.animations.tests.js +++ b/test/specs/core.animations.tests.js @@ -93,6 +93,84 @@ describe('Chart.animations', function() { }, 300); }); + it('should update path properties to target after animations complete', function(done) { + const chart = { + draw: function() {}, + options: { + } + }; + const anims = new Chart.Animations(chart, {value: {properties: ['level.value'], type: 'number', duration: 500}}); + + const from = 0; + const to = 100; + const target = { + level: { + value: from + } + }; + expect(anims.update(target, { + level: { + value: to + } + })).toBeTrue(); + + const ended = function() { + const value = target.level.value; + expect(value === to).toBeTrue(); + Chart.animator.remove(chart); + done(); + }; + + Chart.animator.listen(chart, 'complete', ended); + Chart.animator.start(chart); + setTimeout(function() { + const value = target.level.value; + expect(value > from).toBeTrue(); + expect(value < to).toBeTrue(); + }, 250); + }); + + it('should update path properties to target options after animations complete', function(done) { + const chart = { + draw: function() {}, + options: { + } + }; + const anims = new Chart.Animations(chart, {value: {properties: ['level.value'], type: 'number', duration: 500}}); + + const from = 0; + const to = 100; + const target = { + options: { + level: { + value: from + } + } + }; + expect(anims.update(target, { + options: { + level: { + value: to + } + } + })).toBeTrue(); + + const ended = function() { + const value = target.options.level.value; + expect(value === to).toBeTrue(); + Chart.animator.remove(chart); + done(); + }; + + Chart.animator.listen(chart, 'complete', ended); + Chart.animator.start(chart); + setTimeout(function() { + const value = target.options.level.value; + expect(value > from).toBeTrue(); + expect(value < to).toBeTrue(); + }, 300); + }); + it('should not assign shared options to target when animations are cancelled', function(done) { const chart = { draw: function() {}, From 78b00c01784db8f4e4166d37047ee63fb51cc5d5 Mon Sep 17 00:00:00 2001 From: stockiNail Date: Wed, 1 Mar 2023 16:06:35 +0100 Subject: [PATCH 02/10] some improvements and test cases --- .size-limit.cjs | 2 +- src/core/core.animations.js | 28 +++++----- test/specs/core.animations.tests.js | 82 ++++++++++++++++++++++++++++- 3 files changed, 94 insertions(+), 18 deletions(-) diff --git a/.size-limit.cjs b/.size-limit.cjs index 2b421ffcb09..245ec51a561 100644 --- a/.size-limit.cjs +++ b/.size-limit.cjs @@ -7,7 +7,7 @@ function modifyWebpackConfig(config) { module.exports = [ { path: 'dist/chart.js', - limit: '79 KB', + limit: '79.5 KB', webpack: false, running: false }, diff --git a/src/core/core.animations.js b/src/core/core.animations.js index e40c021bab0..ba2e6931bd6 100644 --- a/src/core/core.animations.js +++ b/src/core/core.animations.js @@ -35,14 +35,9 @@ export default class Animations { } }); }); - const pathAnimatedProps = this._pathProperties; - animatedProps.forEach(function(v, k) { - const value = parserPathOptions(k); - if (value) { - pathAnimatedProps.push(value); - } - }); + const pathAnimatedProps = this._pathProperties; + loadPathOptions(animatedProps, pathAnimatedProps); } /** @@ -154,6 +149,15 @@ export default class Animations { } } +function loadPathOptions(props, pathProps) { + props.forEach(function(v, k) { + const value = parserPathOptions(k); + if (value) { + pathProps.push(value); + } + }); +} + function awaitAll(animations, properties) { const running = []; const keys = Object.keys(properties); @@ -216,16 +220,10 @@ function parseKeys(key, keys) { function getInnerObject(target, pathOpts) { let obj = target; for (const p of pathOpts.path) { - obj = checkAndGetObject(obj, p); - if (!obj) { + obj = obj[p]; + if (!isObject(obj)) { return; } } return obj; } - -function checkAndGetObject(obj, key) { - if (isObject(obj[key])) { - return obj[key]; - } -} diff --git a/test/specs/core.animations.tests.js b/test/specs/core.animations.tests.js index 246cd02325a..d1264d9c562 100644 --- a/test/specs/core.animations.tests.js +++ b/test/specs/core.animations.tests.js @@ -93,7 +93,7 @@ describe('Chart.animations', function() { }, 300); }); - it('should update path properties to target after animations complete', function(done) { + it('should update path properties to target during animation', function(done) { const chart = { draw: function() {}, options: { @@ -130,7 +130,85 @@ describe('Chart.animations', function() { }, 250); }); - it('should update path properties to target options after animations complete', function(done) { + it('should not update path properties to target during animation because not an object', function() { + const chart = { + draw: function() {}, + options: { + } + }; + const anims = new Chart.Animations(chart, {value: {properties: ['level.value'], type: 'number', duration: 500}}); + + const from = 0; + const to = 100; + const target = { + level: from + }; + expect(anims.update(target, { + level: to + })).toBeUndefined(); + }); + + it('should not update path properties to target during animation because missing target', function() { + const chart = { + draw: function() {}, + options: { + } + }; + const anims = new Chart.Animations(chart, {value: {properties: ['level.value'], type: 'number', duration: 500}}); + + const from = 0; + const to = 100; + const target = { + foo: from + }; + + expect(anims.update(target, { + foo: to + })).toBeUndefined(); + }); + + it('should update path (2 levels) properties to target during animation', function(done) { + const chart = { + draw: function() {}, + options: { + } + }; + const anims = new Chart.Animations(chart, {value: {properties: ['level1.level2.value'], type: 'number', duration: 500}}); + + const from = 0; + const to = 100; + const target = { + level1: { + level2: { + value: from + } + } + }; + expect(anims.update(target, { + level1: { + level2: { + value: to + } + } + })).toBeTrue(); + + const ended = function() { + const value = target.level1.level2.value; + expect(value === to).toBeTrue(); + Chart.animator.remove(chart); + done(); + }; + + Chart.animator.listen(chart, 'complete', ended); + Chart.animator.start(chart); + setTimeout(function() { + const value = target.level1.level2.value; + expect(value > from).toBeTrue(); + expect(value < to).toBeTrue(); + }, 250); + }); + + it('should update path properties to target options during animation', function(done) { const chart = { draw: function() {}, options: { From ed9a07e6aa024477930659c7fdce87f9fb62261c Mon Sep 17 00:00:00 2001 From: stockiNail Date: Wed, 1 Mar 2023 16:12:07 +0100 Subject: [PATCH 03/10] size limit --- .size-limit.cjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.size-limit.cjs b/.size-limit.cjs index 245ec51a561..30cb0a80baf 100644 --- a/.size-limit.cjs +++ b/.size-limit.cjs @@ -20,7 +20,7 @@ module.exports = [ }, { path: 'dist/chart.js', - limit: '19.5 KB', + limit: '19.7 KB', import: '{ BarController, BubbleController, DoughnutController, LineController, PolarAreaController, PieController, RadarController, ScatterController }', running: false, modifyWebpackConfig From ac7bb052505222d43aa10f98275c41c7ebbb1502 Mon Sep 17 00:00:00 2001 From: stockiNail Date: Wed, 1 Mar 2023 16:40:33 +0100 Subject: [PATCH 04/10] add test case on properties consistency --- test/specs/core.animations.tests.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/specs/core.animations.tests.js b/test/specs/core.animations.tests.js index d1264d9c562..d1758092c96 100644 --- a/test/specs/core.animations.tests.js +++ b/test/specs/core.animations.tests.js @@ -167,6 +167,16 @@ describe('Chart.animations', function() { })).toBeUndefined(); }); + it('should not update path properties to target during animation because properties not consistent', function() { + const chart = { + draw: function() {}, + options: { + } + }; + const anims = new Chart.Animations(chart, {value: {properties: ['.value', 'value.', 'value..end'], type: 'number', duration: 500}}); + expect(anims._pathProperties.length === 0).toBeTrue(); + }); + it('should update path (2 levels) properties to target during animation', function(done) { const chart = { draw: function() {}, From f17489ef8a701f7c9e2810bebecec7aab60285a3 Mon Sep 17 00:00:00 2001 From: stockiNail Date: Wed, 1 Mar 2023 16:43:50 +0100 Subject: [PATCH 05/10] improve use cases config --- test/specs/core.animations.tests.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/specs/core.animations.tests.js b/test/specs/core.animations.tests.js index d1758092c96..7120a7166c8 100644 --- a/test/specs/core.animations.tests.js +++ b/test/specs/core.animations.tests.js @@ -136,7 +136,7 @@ describe('Chart.animations', function() { options: { } }; - const anims = new Chart.Animations(chart, {value: {properties: ['level.value'], type: 'number', duration: 500}}); + const anims = new Chart.Animations(chart, {value: {properties: ['level.value'], type: 'number'}}); const from = 0; const to = 100; @@ -154,7 +154,7 @@ describe('Chart.animations', function() { options: { } }; - const anims = new Chart.Animations(chart, {value: {properties: ['level.value'], type: 'number', duration: 500}}); + const anims = new Chart.Animations(chart, {value: {properties: ['level.value'], type: 'number'}}); const from = 0; const to = 100; @@ -173,7 +173,7 @@ describe('Chart.animations', function() { options: { } }; - const anims = new Chart.Animations(chart, {value: {properties: ['.value', 'value.', 'value..end'], type: 'number', duration: 500}}); + const anims = new Chart.Animations(chart, {value: {properties: ['.value', 'value.', 'value..end'], type: 'number'}}); expect(anims._pathProperties.length === 0).toBeTrue(); }); @@ -256,7 +256,7 @@ describe('Chart.animations', function() { const value = target.options.level.value; expect(value > from).toBeTrue(); expect(value < to).toBeTrue(); - }, 300); + }, 250); }); it('should not assign shared options to target when animations are cancelled', function(done) { From 6ec402961b70bbc1306a9d439470b635a1ea0bf4 Mon Sep 17 00:00:00 2001 From: stockiNail Date: Wed, 1 Mar 2023 16:58:35 +0100 Subject: [PATCH 06/10] remove useless check --- src/core/core.animations.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/core/core.animations.js b/src/core/core.animations.js index ba2e6931bd6..8dc11c7ca30 100644 --- a/src/core/core.animations.js +++ b/src/core/core.animations.js @@ -190,11 +190,7 @@ function resolveTargetOptions(target, newOptions) { function parserPathOptions(key) { if (key.includes('.')) { - const keys = _splitKey(key); - if (keys.length <= 1) { - return; - } - return parseKeys(key, keys); + return parseKeys(key, _splitKey(key)); } } From f814f8e2f2294628177be3214bd08eafb86eeca3 Mon Sep 17 00:00:00 2001 From: stockiNail Date: Thu, 2 Mar 2023 10:06:47 +0100 Subject: [PATCH 07/10] improve performances --- src/core/core.animations.js | 12 +++++++----- test/specs/core.animations.tests.js | 2 +- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/core/core.animations.js b/src/core/core.animations.js index 8dc11c7ca30..2c1676bf7e0 100644 --- a/src/core/core.animations.js +++ b/src/core/core.animations.js @@ -7,7 +7,7 @@ export default class Animations { constructor(chart, config) { this._chart = chart; this._properties = new Map(); - this._pathProperties = []; + this._pathProperties = new Map(); this.configure(config); } @@ -110,9 +110,8 @@ export default class Animations { animations.push(...this._animateOptions(target, values)); continue; } - const matched = pathAnimatedProps.filter(item => item.prop.startsWith(prop)); - if (matched.length) { - matched.forEach(function(item) { + if (pathAnimatedProps.has(prop)) { + pathAnimatedProps.forEach(function(item) { const newTarget = getInnerObject(target, item); const newValues = getInnerObject(values, item); if (newTarget && newValues) { @@ -153,7 +152,10 @@ function loadPathOptions(props, pathProps) { props.forEach(function(v, k) { const value = parserPathOptions(k); if (value) { - pathProps.push(value); + const mapKey = value.path[0]; + const mapValue = pathProps.get(mapKey) || []; + mapValue.push(value); + pathProps.set(mapKey, value); } }); } diff --git a/test/specs/core.animations.tests.js b/test/specs/core.animations.tests.js index 7120a7166c8..092ed09159e 100644 --- a/test/specs/core.animations.tests.js +++ b/test/specs/core.animations.tests.js @@ -174,7 +174,7 @@ describe('Chart.animations', function() { } }; const anims = new Chart.Animations(chart, {value: {properties: ['.value', 'value.', 'value..end'], type: 'number'}}); - expect(anims._pathProperties.length === 0).toBeTrue(); + expect(anims._pathProperties.size === 0).toBeTrue(); }); it('should update path (2 levels) properties to target during animation', function(done) { From 87b9297c6ba45c1e2be855fb0ecf76ea411c86d7 Mon Sep 17 00:00:00 2001 From: stockiNail Date: Wed, 5 Jul 2023 11:10:46 +0200 Subject: [PATCH 08/10] Update src/core/core.animations.js Co-authored-by: Jukka Kurkela --- src/core/core.animations.js | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/core/core.animations.js b/src/core/core.animations.js index 2c1676bf7e0..aadf730039b 100644 --- a/src/core/core.animations.js +++ b/src/core/core.animations.js @@ -110,14 +110,12 @@ export default class Animations { animations.push(...this._animateOptions(target, values)); continue; } - if (pathAnimatedProps.has(prop)) { - pathAnimatedProps.forEach(function(item) { - const newTarget = getInnerObject(target, item); - const newValues = getInnerObject(values, item); - if (newTarget && newValues) { - manageItem(newTarget, newValues, item.prop, item.key); - } - }); + if (prop.includes('.')) { + const newTarget = resolveObjectKey(target, prop); + const newValues = newTarget && resolveObjectKey(values, prop); + if (newValues) { + manageItem(newTarget, newValues, prop, prop.split('.').pop()); + } } else { manageItem(target, values, prop); } From 31442e1864dfa10183e957ce8ec8ebc2570fbe79 Mon Sep 17 00:00:00 2001 From: stockiNail Date: Wed, 5 Jul 2023 12:54:05 +0200 Subject: [PATCH 09/10] improve cycle --- src/core/core.animations.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/core/core.animations.js b/src/core/core.animations.js index aadf730039b..e4332dfbe23 100644 --- a/src/core/core.animations.js +++ b/src/core/core.animations.js @@ -110,11 +110,12 @@ export default class Animations { animations.push(...this._animateOptions(target, values)); continue; } - if (prop.includes('.')) { - const newTarget = resolveObjectKey(target, prop); - const newValues = newTarget && resolveObjectKey(values, prop); + const item = pathAnimatedProps.get(prop); + if (item) { + const newTarget = getInnerObject(target, item); + const newValues = newTarget && getInnerObject(values, item); if (newValues) { - manageItem(newTarget, newValues, prop, prop.split('.').pop()); + manageItem(newTarget, newValues, item.prop, item.key); } } else { manageItem(target, values, prop); From 42f31c4f3f55ecc3ecf449ca0be48b5e87230e2e Mon Sep 17 00:00:00 2001 From: stockiNail Date: Wed, 5 Jul 2023 15:53:08 +0200 Subject: [PATCH 10/10] fixed multiple path options with the same root and test case --- src/core/core.animations.js | 18 ++++++------ test/specs/core.animations.tests.js | 44 +++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 8 deletions(-) diff --git a/src/core/core.animations.js b/src/core/core.animations.js index e4332dfbe23..00ca73d4b61 100644 --- a/src/core/core.animations.js +++ b/src/core/core.animations.js @@ -110,13 +110,15 @@ export default class Animations { animations.push(...this._animateOptions(target, values)); continue; } - const item = pathAnimatedProps.get(prop); - if (item) { - const newTarget = getInnerObject(target, item); - const newValues = newTarget && getInnerObject(values, item); - if (newValues) { - manageItem(newTarget, newValues, item.prop, item.key); - } + const propValue = pathAnimatedProps.get(prop); + if (propValue) { + propValue.forEach(function(item) { + const newTarget = getInnerObject(target, item); + const newValues = newTarget && getInnerObject(values, item); + if (newValues) { + manageItem(newTarget, newValues, item.prop, item.key); + } + }); } else { manageItem(target, values, prop); } @@ -154,7 +156,7 @@ function loadPathOptions(props, pathProps) { const mapKey = value.path[0]; const mapValue = pathProps.get(mapKey) || []; mapValue.push(value); - pathProps.set(mapKey, value); + pathProps.set(mapKey, mapValue); } }); } diff --git a/test/specs/core.animations.tests.js b/test/specs/core.animations.tests.js index 092ed09159e..bdd319694bc 100644 --- a/test/specs/core.animations.tests.js +++ b/test/specs/core.animations.tests.js @@ -130,6 +130,50 @@ describe('Chart.animations', function() { }, 250); }); + it('should update multiple path properties with the same root to target during animation', function(done) { + const chart = { + draw: function() {}, + options: { + } + }; + const anims = new Chart.Animations(chart, {value: {properties: ['level.value1', 'level.value2'], type: 'number', duration: 500}}); + + const from = 0; + const to = 100; + const target = { + level: { + value1: from, + value2: from + } + }; + expect(anims.update(target, { + level: { + value1: to, + value2: to + } + })).toBeTrue(); + + const ended = function() { + const value1 = target.level.value1; + expect(value1 === to).toBeTrue(); + const value2 = target.level.value2; + expect(value2 === to).toBeTrue(); + Chart.animator.remove(chart); + done(); + }; + + Chart.animator.listen(chart, 'complete', ended); + Chart.animator.start(chart); + setTimeout(function() { + const value1 = target.level.value1; + const value2 = target.level.value2; + expect(value1 > from).toBeTrue(); + expect(value1 < to).toBeTrue(); + expect(value2 > from).toBeTrue(); + expect(value2 < to).toBeTrue(); + }, 250); + }); + it('should not update path properties to target during animation because not an object', function() { const chart = { draw: function() {},