Components flashing and re-rendering when `animate` is set to true

[becca-bailey]

Setting the animation prop to true impacts the performance of a component even when it is not actively animating.

If I enable paint flashing in the Chrome dev tools to show which parts of the chart are re-painting, there is a big difference between a chart where animate is set to false vs. a chart where animate is set to true.

With animation:

Without animation:

In the second example, only the tooltip is re-rendering, as expected. In the first, there is some visible flashing in the axis labels, as they are re-rendering along with a lot of the data as the tooltip moves.

Steps to reproduce: 1.Enable Paint flashing in the chrome dev tools

2. Compare the stacked bar chart performance of https://victory-testing-dashboard.netlify.app/?animate=true vs https://victory-testing-dashboard.netlify.app/

Related issues: #1993

[becca-bailey]

Investigating a little more - there appears to be some weirdness in the component tree. Notice how we are rendering VictoryBar -> VictoryTransition -> VictoryAnimation -> VictoryBar. This is causing VictoryBar to render ~20x on each user interaction.

[becca-bailey]

Other observations — VictoryBar is re-rendering ~20x on each interaction, and calling getBaseProps to re-calculate props each time. There is probably some caching here that is not working as expected.

Also, VictoryAnimation is re-rendering hundreds of times on each interaction. This presumably has something to do with the d3.timer and data interpolation, but this shouldn't be happening when the data isn't animating.

[becca-bailey]

So, here's what's happening. When animate is set to true, every time the sharedEvents state changes, VictoryAnimation re-renders. Every time VictoryAnimation renders, it kicks off an animation, which uses d3.timer and d3.interpolate to interpolate and re-render the data for 1000 ms. In this case, the 1000 ms is the default value, and is not currently configurable.

I do think that we should probably re-evaluate the way Victory handles shared events so it doesn't pass the sharedEvents prop down through the entire component tree, causing every component to re-renders on each event.

Short term, I am working on a PR to add an onMove property to the animate configuration. This property exists within the transition state, but is not currently configurable. https://github.com/FormidableLabs/victory/blob/2726d05d2950769e24c5f8709561f53cd2d801af/packages/victory-core/src/victory-util/transitions.js#L273

This way, we can at least configure a chart not to kick off an animation cycle on every state change.

[becca-bailey]

On second thought, configuring the default move animation to be zero broke the loading animation. I thought that the onLoad configuration would take care of this, but I'm not sure I can fix it without a deep dive into the transition state and a bigger risk of breaking other expected animation behavior.

[becca-bailey]

I'm going to keep this open and add it to the events state milestone so we can include some refactoring in the state management work. Hopefully some initial refactoring will make this behavior easier to change.

[chriszrc]

@becca-bailey basic animations (value, color, etc) on area and bar charts are still intensely jittery/janky with lots of flashing. Is there a way to fix this? A branch to try?

[joseForth]

I'm having this issue as well on the area and doughnut charts. Whenever the component is mounting or re-render, the colours flash and the animation doesn't work smoothly. Any solution??

[chriszrc]

@joseForth one thing I've noticed is that it seems to currently matter where you put the animation attribute, on the chart container, or on the specific chart. Look at this example:

https://codesandbox.io/s/reverent-jasper-2fwqib?file=/src/App.js:164-210

If you use the animate on the container, you get flickering and no real animation. On the chart it works fine. Seems there are differences here between different chart type too, for instance, the bar chart doesn't seem to have the same problem-

[RoccoDocco]

If you use the animate on the container, you get flickering and no real animation. On the chart it works fine. Seems there are differences here between different chart type too, for instance, the bar chart doesn't seem to have the same problem-

I have the same problem and was able to reproduce this too. The only problem with this solution is that the Axis labels do not animate anymore(which makes sense).

[chriszrc]

@RoccoDocco yes, the axis labels won't animate, but honestly, most charts don't have the axes animate, just the data, so I actually liked that better-

[darimuhittinhey]

@becca-bailey which ide are u using ? (it has nothing to do with the subject, I just asked out of curiosity)

[carbonrobot]

Additional example with much simpler setup: https://codesandbox.io/s/single-point-flickering-076ox?file=/src/Components/VictoryLineDemo.tsx