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

Enable animation on inner nodes of the element and options #11173

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
115 changes: 90 additions & 25 deletions src/core/core.animations.js
Original file line number Diff line number Diff line change
@@ -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 = new Map();
this.configure(config);
}

Expand Down Expand Up @@ -34,6 +35,9 @@ export default class Animations {
}
});
});

const pathAnimatedProps = this._pathProperties;
loadPathOptions(animatedProps, pathAnimatedProps);
}

/**
Expand Down Expand Up @@ -67,54 +71,67 @@ 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);

if (animation) {
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;
}
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);
}
});
} else {
manageItem(target, values, prop);
}
Copy link
Member

@kurkle kurkle Jul 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like each "path" prop would be managed multiple times if there are multiple "path" props?

I think most of the new helpers could be omitted(?):

Suggested change
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);
}
});
} else {
manageItem(target, values, prop);
}
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);
}

Did I miss something?

Copy link
Contributor Author

@stockiNail stockiNail Jul 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kurkle I think the IF statement, you suggested, is wrong because the "prop" is not an item of "properties" animation configuration but one of the keys of the config object, therefore it will never have a dot.
The for cycle is on the config properties and not on the animation config one. That's why I used a map, reading the anim config, where the K is the fist part of the path (to check with the prop) and the V the whole path (an object with prop and whole path as array).

EDIT: I have merged your suggestion anyway because I didn't recall all logic of this PR... 4 months is to big period for my memory, getting old. 😃

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Furthermore, I cannot use resolveObjectKey helper because this function is returning the value of the key and not the object where the key belongs to and therefore I cannot reuse the manageItem function for both normal and path anim options, where the function needs the object with key.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kurkle I have improved a bit removing the for Each statemenet. Anyway forEach should be needed. Need to time to review. Apologize

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kurkle now I recall why we need to scan the map but was wrong. Now I fixed.

The map has:

K = string = the first item of the path (i.e. 'font' if animation properties config has 'font.size')
V = array = the array of objects with the animation properties config

The V is an array because you can configure more than 1 anim prop as path (i.e. ['font.size', 'font.weight'])

}
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.
Expand All @@ -131,6 +148,18 @@ export default class Animations {
}
}

function loadPathOptions(props, pathProps) {
props.forEach(function(v, k) {
const value = parserPathOptions(k);
if (value) {
const mapKey = value.path[0];
const mapValue = pathProps.get(mapKey) || [];
mapValue.push(value);
pathProps.set(mapKey, value);
}
});
}

function awaitAll(animations, properties) {
const running = [];
const keys = Object.keys(properties);
Expand Down Expand Up @@ -160,3 +189,39 @@ function resolveTargetOptions(target, newOptions) {
}
return options;
}

function parserPathOptions(key) {
if (key.includes('.')) {
return parseKeys(key, _splitKey(key));
}
}

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 = obj[p];
if (!isObject(obj)) {
return;
}
}
return obj;
}
166 changes: 166 additions & 0 deletions test/specs/core.animations.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,172 @@ describe('Chart.animations', function() {
}, 300);
});

it('should update path properties to target during animation', 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 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'}});

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'}});

const from = 0;
const to = 100;
const target = {
foo: from
};

expect(anims.update(target, {
foo: to
})).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'}});
expect(anims._pathProperties.size === 0).toBeTrue();
});

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: {
}
};
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();
}, 250);
});

it('should not assign shared options to target when animations are cancelled', function(done) {
const chart = {
draw: function() {},
Expand Down