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

Expose animation default interpolators to use in custom implementation #11129

Closed
wants to merge 7 commits into from

Conversation

stockiNail
Copy link
Contributor

This PR is exposing the out-of-the-box animation interpolators in order to enable the user to use them in a custom interpolator.

See #11118 (comment)

@stockiNail
Copy link
Contributor Author

I'm not TS expert (very low skill, apologize) therefore not sore if the TS is 100% correct

src/types/animation.d.ts Outdated Show resolved Hide resolved
@stockiNail stockiNail requested review from dangreen and kurkle and removed request for dangreen and kurkle February 10, 2023 19:33
@@ -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.

@stockiNail
Copy link
Contributor Author

I think I have found another way. I close it in the meantime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants