Animation callback documentation needs clarification

[colmben]

The animation callback doco is as follows:

Animation Callbacks

The onProgress and onComplete callbacks are useful for synchronizing an external draw to the chart animation. The callback is passed an object that implements the following interface. An example usage of these callbacks can be found on Github. This sample displays a progress bar showing how far along the animation is.

{
// Chart object
chartInstance,

// Contains details of the on-going animation
animationObject,

}

Expected Behavior

I would therefore expect to get an object in the callback as described when I set up an animation callback.

Current Behavior

If the duration on the animation (for update) or the responsiveAnimationDuration (for resize) is set at 0 then the animation callback doesn't get the object and instead is called via a .call(this) which sets the this context for the callback function to be the chart instance.

Context

At first blush this seems reasonable (even if it is undocumented), as you still get the chartInstance and presumably you have no need for the animationObject if the duration was 0. The problem is that in an ES6 class you can't now access an outer context in this circumstance. If you do a standard function(){} callback then the function can't see outside of itself. If you do a fat arrow function () => {} then the .call(this) from the callback is lost as the outer this is forced in. So you can't have both the outer context and the chart context at the same time. In that circumstance it becomes crucial to have the chartInstance available as an argument in the callback, as documented.

Possible Solution

Changing animation.duration and responsiveAnimationDuration to. for example, 1 gives the documented behaviour. A line in the doco to that effect would be useful. Beyond that, I would say that doing .call(this) style callbacks is probably going to cause ongoing issues with arrow functions going forward.

Steps to Reproduce (for bugs)

Set up an animation callback (e.g. onComplete) with a duration of 0. Note that there is no object passed in to the callback as per the doco.

[etimberg]

@colmben thanks for the detailed report.

I think the best solution in this case would be to change

animationOptions.onComplete.call(me);

to

animationOptions.onComplete({ chartInstance: me });

and then also updating the docs to include that in the case of no animation, the animationObject will be null.

I'm happy to merge a PR for this if you're up for it.