Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor VictoryTransition and animations #2104

Open
becca-bailey opened this issue Feb 15, 2022 · 1 comment
Open

Refactor VictoryTransition and animations #2104

becca-bailey opened this issue Feb 15, 2022 · 1 comment
Labels
Issue: Accepted The submitted issue has been confirmed by the Victory core team Status: On Hold ⚠️ This issue has prerequisites before it can be addressed Type: Enhancement ✏️ An enhancement or feature proposal that will be addressed after the next release
Milestone

Comments

@becca-bailey
Copy link
Contributor

becca-bailey commented Feb 15, 2022

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:

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 becca-bailey added Type: Enhancement ✏️ An enhancement or feature proposal that will be addressed after the next release Status: On Hold ⚠️ This issue has prerequisites before it can be addressed labels Feb 15, 2022
@becca-bailey becca-bailey added this to the Refactor transitions milestone Feb 15, 2022
@becca-bailey becca-bailey changed the title Refactor VictoryTransition Refactor VictoryTransition and animations Feb 15, 2022
@clemwo
Copy link

clemwo commented Jan 8, 2024

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

@carbonrobot carbonrobot added the Issue: Accepted The submitted issue has been confirmed by the Victory core team label Feb 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue: Accepted The submitted issue has been confirmed by the Victory core team Status: On Hold ⚠️ This issue has prerequisites before it can be addressed Type: Enhancement ✏️ An enhancement or feature proposal that will be addressed after the next release
Projects
None yet
Development

No branches or pull requests

3 participants