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.

Sign up for GitHub

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

Expose animation default interpolators to use in custom implementation #11129

Closed
wants to merge 7 commits into from
Closed

Expose animation default interpolators to use in custom implementation #11129

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
80736c3
Expose animation interpolators to use in custom ones
stockiNail Feb 9, 2023
fb64719
Update src/types/animation.d.ts
stockiNail Feb 10, 2023
a2e82a6
apply review
stockiNail Feb 10, 2023
16d52a0
remove useless comma
stockiNail Feb 10, 2023
1fba89b
remove useless empty line
stockiNail Feb 10, 2023
e3a0b0e
add new empty line
stockiNail Feb 10, 2023
3beac4b
Merge remote-tracking branch 'chartjs/master' into exposeInterpolators
stockiNail Feb 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/core/core.animation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ const interpolators = {
};

export default class Animation {

static interpolators = Object.freeze(interpolators);

constructor(cfg, target, prop, to) {
const currentValue = target[prop];

Expand Down
13 changes: 13 additions & 0 deletions src/types/animation.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,16 @@
import {Chart} from './index.js';
import {AnyObject} from './basic.js';
import {Color} from './color.js';

export type PretypedInterpolator<T> = (from: T, to: T, factor: number) => T;

export type Interpolator = <T>(from: T, to: T, factor: number) => T;

export interface InterpolatorsMap {
boolean: PretypedInterpolator<boolean>;
color: PretypedInterpolator<Color>;
number: PretypedInterpolator<number>;
}

export declare class Animation {
constructor(cfg: AnyObject, target: AnyObject, prop: string, to?: unknown);
Expand All @@ -8,6 +19,8 @@ export declare class Animation {
cancel(): void;
tick(date: number): void;
readonly _to: unknown;

static readonly interpolators: InterpolatorsMap;
}

export interface AnimationEvent {
Expand Down
4 changes: 2 additions & 2 deletions src/types/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {DeepPartial, DistributiveArray, UnionToIntersection} from './utils.js';
import {TimeUnit} from '../core/core.adapters.js';
import PointElement from '../elements/element.point.js';
import {EasingFunction} from '../helpers/helpers.easing.js';
import {AnimationEvent} from './animation.js';
import {AnimationEvent, Interpolator} from './animation.js';
import {AnyObject, EmptyObject} from './basic.js';
import {Color} from './color.js';
import Element from '../core/core.element.js';
Expand Down Expand Up @@ -1594,7 +1594,7 @@ export type AnimationsSpec<TType extends ChartType> = {
*/
type: 'color' | 'number' | 'boolean';

fn: <T>(from: T, to: T, factor: number) => T;
fn: Interpolator;

/**
* Start value for the animation. Current value is used when undefined
Expand Down
15 changes: 15 additions & 0 deletions test/specs/core.animation.tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,4 +83,19 @@ describe('Chart.Animation', function() {
expect(anim._duration).toEqual(100);
expect(anim._to).toEqual('red');
});

it('should invoke out-of-the-box interpolator', function() {
const fn = function(from, to, factor) {
const val = Chart.Animation.interpolators.color(from, to, factor);
expect(val).toEqual('#00F3');
return 'green';
};
const target = {testColor: 'transparent'};
const anim = new Chart.Animation({duration: 100, type: 'color', fn}, target, 'testColor', 'red');

anim.update({duration: 500}, 'blue', Date.now());
anim.tick(anim._start + 100);
expect(target.testColor).toEqual('green');
});

});
5 changes: 3 additions & 2 deletions test/types/animation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Chart } from '../../src/types.js';
import { Chart, Animation } from '../../src/types.js';

const chart = new Chart('id', {
type: 'bar',
Expand All @@ -20,7 +20,8 @@ const chart = new Chart('id', {
delay: (ctx) => ctx.dataIndex * 100,
duration: (ctx) => ctx.datasetIndex * 1000,
loop: true,
easing: 'linear'
easing: 'linear',
fn: (from: any, to: any, factor: number) => ({size: Animation.interpolators.number(0, 32, factor)} as any)
Copy link
Member

Choose a reason for hiding this comment

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

this is a really bad test, animating numbers a, and b and resulting to an object value.

I would argue it would be better to fix the options resolution for font in the use case and just animate the size as a number.. and not support animation objects, because its going to open deep can of worms.

Copy link
Member

Choose a reason for hiding this comment

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

That said, I'm not against this PR itself. I'd even maybe make the interpolators writable, if that can be supported.

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 when you wrote ‘writable’ do you mean custom interpolator for custom type?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is a really bad test, animating numbers a, and b and resulting to an object value.

@kurkle this is just a test for the OOTB interpolators. I can add a specific test for objects but I didn't want to add another test case because fn was used here.

I would argue it would be better to fix the options resolution for font in the use case and just animate the size as a number.. and not support animation objects, because its going to open deep can of worms.

@kurkle I could agree with you but, afai understood, fn option was born for this reason, to give to the user the possibility to animate items not manageable OOTB (like objects). If there is this option for other purposes, it's not clear to me.
About font, this is not really my use case. The use case is coming from annotation plugin where, having color option aa an array, the animation doesn't recognize the array, therefore I was thinking to have a specific animation for color where, when configured as an array, the custom interpolator will return an array of intepolated colors.

The best solution could be to enable animation on inner nodes of the options (like font, padding, borders and other), where the property is defined by dot separator. Something like:

properties: ['font.size', 'padding.top'],

But I think this deserve a specific PR, this one is just to expose the OOTB interpolator.
Let me know how to proceed.

}
},
transitions: {
Expand Down