victory icon indicating copy to clipboard operation
victory copied to clipboard

Refactor VictoryTransition and animations

Open becca-bailey opened this issue 4 years ago • 5 comments

This is a larger issue designed to cover a few individual bugs and issues that have been raised around Victory transitions. I'm just linking to all of the issues here since there have been a lot of issues around this recently, and I'm trying to prioritize our issues and cut down on some of the duplicates. Before these can be addressed, we should consider refactoring victory-transition.js and overhauling how transitions and animations are handled.

I am currently marking this as on hold until we can have more discussions about how we want to break up this work.

Issues to address:

  • Slowness introduced by re-rendering when data interpolates (#847)
  • Enter and exit transitions (#246, #531)
  • Unexpected data mutation (#770)
  • Configuring delays (#848)
  • Support for custom data components (#973)

Technical notes:

Because of the performance concerns around re-rendering every React component many times during each transition state, it might be a good idea to consider a d3-based approach that mutates the DOM without re-rendering every React component. In her d3 + React course, Shirley Wu discusses the division of responsibilities between React and d3, and the advantages of allowing d3 to control DOM rendering in some specific circumstances, such as during transitions. use-d3-transition is an example of how we might cede this responsibility to d3 instead of React to simplify our code and reduce React re-rendering cycles.

becca-bailey avatar Feb 15 '22 00:02 becca-bailey

Since I also had chart animation / flickering issues I did some tests and I found out how to reproduce the flickering issues.

TL;DR: React 18 ReactDOM.createRoot causes animations to break

Steps to Reproduce:

  1. Set up a Victory Chart with animations in a React 18 project.
  2. Render the application using ReactDOM.createRoot (code snippet provided below).
  3. Notice that the chart's animation flickers and is not smooth.
  4. When rendering the same application using the older ReactDOM.render method, the animation is smooth and there are no flickering issues.

Example with broken animations:

index.js

import React from "react";
import ReactDOM from "react-dom/client";
import App from "./App";

const root = ReactDOM.createRoot(document.getElementById("root"));
root.render(
  <React.StrictMode>
    <App />
  </React.StrictMode>,
);

Example with working animations:

index.js

import React from "react";
import ReactDOM from "react-dom";

import App from "./App";

const rootElement = document.getElementById("root");
ReactDOM.render(
  <React.StrictMode>
    <App />
  </React.StrictMode>,
  rootElement,
);

I have also created a sandbox to demonstrate the issue here

clemwo avatar Jan 08 '24 09:01 clemwo