Enable initial animation on annotations

[stockiNail]

This PR is enabling the possibility to the user to set the animation to the annotations when they are drawing at chart initialization.

It adds an options to all annotations: initAnimation.

https://user-images.githubusercontent.com/11741250/171450002-c677e11a-9f77-4d07-9a6e-cb839ff2ee94.mp4

Option	Type	Default
initAnimation	boolean | (chart, properties, options) => void | boolean | AnnotationBoxModel	false

The callback is not invoked passing the element context (because element is not build in that phase) but receives chart, annotation options and the properties of the element (x, y, x2, y2, centerX, centerY, width, height), calculated before callback invocation. The callback can return a boolean or an object, with the initial values of the element properties (x, y, x2, y2, centerX, centerY, width, height), needed to perform the animation.

TODO

• [x] provide 1 default animation (not all modes) and a way to enable the user to customize the default
• [x] provide the initial animation options at plugin level (for all annotations)
• [x] test cases
• [x] types
• [x] documentation
• [x] sample
• [ ] wait for #749 approval and merge

[stockiNail]

@kurkle @LeeLenaleee off topic from this PR. Milestone 2.0.0 is growing but I don't know if there is any community best practice to release new major version. Maybe some PRs in queue could be moved to new milestone, 2.1.0 in order to release 2.0.0 to the users.

[kurkle]

I don't know about best practices, but I'd prefer fast releases. That means trying to include all the breaking changes we think is needed and bug fixes. Non-breaking features can be postponed to next "feature"/minor release.

[stockiNail]

I don't know about best practices, but I'd prefer fast releases. That means trying to include all the breaking changes we think is needed and bug fixes. Non-breaking features can be postponed to next "feature"/minor release.

Great! At the moment I don't see any other breaking change to add. Hopefully I'm right but of course not sure 100%. Therefore some other days are needed, to think about. Do you think we can create new milestone 2.1.0, in order to move some PRs to that?

[stockiNail]

About CC issue. I can reduce the amount of lines (to recover 2 lines needed) but I think it could be better to split some files of types in some specific files to manage specific topic (label, callout, arrowHeads).

• Label: move callout management in a specific file
• Line: move label and arrowHeads management in specific files

This could be addressed by a specific PR.

[kurkle]

I don't know about best practices, but I'd prefer fast releases. That means trying to include all the breaking changes we think is needed and bug fixes. Non-breaking features can be postponed to next "feature"/minor release.

Great! At the moment I don't see any other breaking change to add. Hopefully I'm right but of course not sure 100%. Therefore some other days are needed, to think about. Do you think we can create new milestone 2.1.0, in order to move some PRs to that?

Sure we can :)

[kurkle]

About CC issue. I can reduce the amount of lines (to recover 2 lines needed) but I think it could be better to split some files of types in some specific files to manage specific topic (label, callout, arrowHeads).

• Label: move callout management in a specific file
• Line: move label and arrowHeads management in specific files

This could be addressed by a specific PR.

I think we should switch over to sonar or something else (I don't know anything better), because some of the CC "issues" are nonsense IMO.

[kurkle]

I'm not sure we should be providing a bunch of builtin initial named animations. I think it would be better to stick what Chart.js does (one default animation), and extend the animation configuration if needed.

[stockiNail]

I'm not sure we should be providing a bunch of builtin initial named animations. I think it would be better to stick what Chart.js does (one default animation), and extend the animation configuration if needed.

Let me recap in order to understand better. You are proposing 2 things:

1. to have only 1 animation configuration for the whole plugin (and not at annotation level) and all annotations are acting at the same way in a chart instance
2. remove all animations modes and provide a default (maybe fade that sounds better to me) but find a way in order to enable the user to enable his/her custom modes.

Have I understood well?

[kurkle]

Let me recap in order to understand better. You are proposing 2 things:

1. to have only 1 animation configuration for the whole plugin (and not at annotation level) and all annotations are acting at the same way in a chart instance

Hmm, no, I don't really have any objection for annotation level animation configuration. Though it should be doable at plugin level with scriptable options too.

2. remove all animations modes and provide a default (maybe fade that sounds better to me) but find a way in order to enable the user to enable his/her custom modes.

This is what I'm suggesting. I'd rather have samples for some custom animations instead of built-in modes.

[stockiNail]

Let me recap in order to understand better. You are proposing 2 things:

1. to have only 1 animation configuration for the whole plugin (and not at annotation level) and all annotations are acting at the same way in a chart instance

Hmm, no, I don't really have any objection for annotation level animation configuration. Though it should be doable at plugin level with scriptable options too.

2. remove all animations modes and provide a default (maybe fade that sounds better to me) but find a way in order to enable the user to enable his/her custom modes.

This is what I'm suggesting. I'd rather have samples for some custom animations instead of built-in modes.

Clear!!! Let me code a little bit, going to this direction. I have to think a clever way to provide a customization method to the users. I'm going to leave this PR in draft anyway. ;)

[stockiNail]

I think we should switch over to sonar or something else (I don't know anything better), because some of the CC "issues" are nonsense IMO.

I have experience on Sonar but for JAVA code (my project is there). But I don't have enough experience to say if Sonar is better than CC for JS. Maybe there some good things in CC comparing with Sonar that I don't know

[stockiNail]

@kurkle I think I have completed the "first" imp, following your hints. I don't release this PR because it needs to be changed when other PRs will be merged (mainly the label on ellipse). Anyway, feel free to comment and review it, I'll change the impl accordingly.

[stockiNail]

Sure we can :)

done!

[stockiNail]

Sure we can :)

done!

@kurkle I have changed milestone on the pending PRs (apart for #757) which can be merged before releasing version 2.0.0.

Nevertheless, before bumping to 2.0.0, there are some items to do or take a decision:

• ~~fixes the properties in animations config (label* properties still there) in the doc. I'm going to submit a PR soon.~~
• ~~review types because I see some inconsistent definitions (like core annotations). I'm going to submit a PR soon.~~
• #749 raises a doubt about the clip on box for label. If the proposal will be accepted, we have a breaking change and therefore another PR to add it in the migration guide is required
• currently events options, commons for all annotation, are managed at root level of plugin options. Now with common node, maybe it makes sense to move over there, like all other annotation options. This would be a breaking change.
• ~~add element model diagrams in the annotation type documentation page, not only in the migration guide. I'm going to submit a PR soon.~~

Maybe, thinking more, during weekend, other items will popup!

[stockiNail]

@kurkle I think I have completed the "first" imp, following your hints. I don't release this PR because it needs to be changed when other PRs will be merged (mainly the label on ellipse). Anyway, feel free to comment and review it, I'll change the impl accordingly.

Still working on animation test case...

[stockiNail]

I think we need a common way to define the initial properties for an element. My hunch would be to place those in the element defaults (scriptable, with the resolved target element properties and chart/dataset area coordinates in context)

I'd need more time with this (and I don't have it currently)

OK. I don't know if I can help you :( . I need to understand better "with the resolved target element properties and chart/dataset area coordinates in context". I'm going to try to understand, maybe with some tests.

Do you want that I close this PR or do you want to go on anyway and then, when common method will be ready, to change the plugin accordingly?

[stockiNail]

@kurkle feel free to take the decision about this PR as you prefer, nop for me. The PR is now rebased and I have applied your suggestion and ready for review.

[kurkle]

Correct me if I'm wrong, but does initAnimations provide the starting properties for annotation animations? If that is it, I think it could be named better. Maybe initialState would describe it better? Or maybe it could just initialize or even init.

[stockiNail]

Correct me if I'm wrong, but does initAnimations provide the starting properties for annotation animations?

Yes , it does.

If that is it, I think it could be named better. Maybe initialState would describe it better? Or maybe it could just initialize or even init.

OK, I'll do. I have named it adding Animations because I wanted to highlight we are talking about init of animation.

[stockiNail]

If that is it, I think it could be named better. Maybe initialState would describe it better? Or maybe it could just initialize or even init.

Renamed in init.

[kurkle]

Needs a rebase

[stockiNail]

Merging, helpers.canvas has got 254 rows, more than 250 set by CC... :(