Enable label annotation animations

[kurkle]

Fix: #539

Partially address: #535

[kurkle]

Filename	Size	Change
dist/chartjs-plugin-annotation.esm.js	11.2 kB	-155 B (-1%)
dist/chartjs-plugin-annotation.js	11.3 kB	-156 B (-1%)
dist/chartjs-plugin-annotation.min.js	7.64 kB	+38 B (0%)

[stockiNail]

@kurkle first of all, apologize... yesterday I haven't understood what you meant but now yes.

You moved the properties about stored value as properties of the element and then you configured them as animated.... Really sorry but I didn't think to implement as you wrote... Probably this approach could be also used in other annotations.

Let me review better your updates..

[stockiNail]

@kurkle apoligize again for I'm poor knowledge on the animation engine...

But I was looking for animation activation since the first drawing of the chart. It seems that annotations, at the first drawing, are located to the final position. Maybe it could be good to animate them during the first drawing as well.

Having a look to the code, I don't seen how to enable it at first drawing (it seems already implemented):

https://github.com/chartjs/chartjs-plugin-annotation/blob/ef7bf770652f21fd151ea3043156b4dbadd59064/src/annotation.js#L122-L127

[kurkle]

It's the reset state that should be the animation starting point for first draw, as seen on Chart.js controllers. It's just not handled for annotations, but should be quite easy to handle. But the problem is, I'm not sure what annotations should do by default. Appear from some edge? Or grow from zero size? Maybe fade the colors in? It's not going to please everyone.

[stockiNail]

ok, let's stay as is. Anyway I'm going to try it, to see how it will look like.

[kurkle]

ok, let's stay as is. Anyway I'm going to try it, to see how it will look like.

Actually it looks like the initial reset is only applied to updating new elements on controllers. Plugins do not receive it.

I was thinking we should make the initial element state configurable in Chart.js, but maybe it could be done in this plugin only (to start with)

Anyhow, you can try for example this:

export default class LineAnnotation extends Element {
  constructor() {
    super();
    this.options = {
      borderColor: 'transparent',
    };
  }
...

The lines are transparent to begin with. Chart.js does not seem to animate the sub-objects in options (eg. label.backgroundColor), that should be fixed.

[stockiNail]

@kurkle uhm... by default, if I have seen correctly, 'color', 'borderColor', 'backgroundColor' are the default properties of animations, type color.

We need to do the same think that you have done here, I guess, setting specific properties on the element for all subnodes. Correct?

[kurkle]

@kurkle uhm... by default, if I have seen correctly, 'color', 'borderColor', 'backgroundColor' are the default properties of animations, type color.

We need to do the same think that you have done here, I guess, setting specific properties on the element for all subnodes. Correct?

Yes, but it turns out that

this.options = {
  borderColor: 'transparent',
  label: {
    backgroundColor: 'transparent'
  }
}

does not animate the label backgroundcolor. I assume this is an limitation of the animator in Chart.js (did not take a look, and don't remember anymore the specifics)

[stockiNail]

I think the engine is currently using ONLY properties of the element (but I didn't check) therefore all properties of subnodes should be ignored.

Nevertheless I still have some doubts about also width and height, because, even if they are numbers to animated, it doesn't seem it's happening.

EDIT: no, width works fine!

[stockiNail]

I assume this is an limitation of the animator in Chart.js (did not take a look, and don't remember anymore the specifics)

It seems so. it could be great to configure the property with its path, something like:

animations: {
  colors: {
    type: 'color',
    properties: ['color', 'borderColor', 'backgroundColor', 'label.borderColor', 'label.backgroundColor' ]
  },
},

[stockiNail]

@kurkle I think we could merge this and then enable other options to be animated with another PR. What do you think?

[kurkle]

Sure, I will fix the merge conflicts soon, when I'm free from other work.

[stockiNail]

Sure, I will fix the merge conflicts soon, when I'm free from other work.

Sorry, I didn't want to push you. I didn't know if you wanted to add this additional options... I'm doing other stuff too.

[kurkle]

Sure, I will fix the merge conflicts soon, when I'm free from other work.

Sorry, I didn't want to push you. I didn't know if you wanted to add this additional options... I'm doing other stuff too.

No problem, was just busy and wanted to let you know. Will do the rebase now.

[kurkle]

Hmm, I'll investigate these random failures a bit still, found a way to reproduce consistently locally.

[kurkle]

Did not find the real cause. But with random seed 94637 I get a consistent failure in fixtures/label/stylingPoint.js

The failure is because measureText returns different values than normally: key: This is my text,and this is the second row of my textnormal 12px 'Helvetica Neue', 'Helvetica', 'Arial', sans-serif w/h: 194.091796875 28.799999999999997

No fail: key: This is my text,and this is the second row of my textnormal 12px 'Helvetica Neue', 'Helvetica', 'Arial', sans-serif w/h: 232 28.799999999999997

[stockiNail]

Sometimes it happens to me as well, but when I relaunch it works... maybe I'm wrong because I'm not an expert of karma, but it sounds related to it. Every time that I'm testing locally, with the browser, the rendering it's always fine. Maybe some config in karma?

[stockiNail]

it happens to clip-false test case too. But it's really random.

[kurkle]

Can repeat by passing the seed: http://localhost:9876/debug.html?seed=94637 (makes it run the tests in same order)

[stockiNail]

@kurkle FYI

> chartjs-plugin-annotation@1.1.0 build
> rollup -c

src/index.js → dist/chartjs-plugin-annotation.js...
(!) Circular dependencies
src\helpers\index.js -> src\helpers\helpers.canvas.js -> src\helpers\index.js
src\helpers\index.js -> src\helpers\helpers.chart.js -> src\helpers\index.js
created dist/chartjs-plugin-annotation.js in 219ms

src/index.js → dist/chartjs-plugin-annotation.min.js...
(!) Circular dependencies
src\helpers\index.js -> src\helpers\helpers.canvas.js -> src\helpers\index.js
src\helpers\index.js -> src\helpers\helpers.chart.js -> src\helpers\index.js
created dist/chartjs-plugin-annotation.min.js in 768ms

src/index.esm.js → dist/chartjs-plugin-annotation.esm.js...
(!) Circular dependencies
src\helpers\index.js -> src\helpers\helpers.canvas.js -> src\helpers\index.js
src\helpers\index.js -> src\helpers\helpers.chart.js -> src\helpers\index.js
created dist/chartjs-plugin-annotation.esm.js in 178ms