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

Convert react spring animations to CSS transitions #662

Merged
merged 3 commits into from
Nov 15, 2023
Merged

Conversation

joglr
Copy link
Contributor

@joglr joglr commented Nov 3, 2023

This PR converts react spring animations to CSS transitions. It should be faster and have less overhead for the Chart component.

I tried recording it, but the video does not do it justice.

Git.Truck.-.Google.Chrome.2023-11-10.02-03-31.mp4

This PR also adds a transition between the tree map and the circle packing!

Git.Truck.-.Google.Chrome.2023-11-10.02-07-25.mp4

The performance is much improved, compared to the old implementation.

You can compare this and current GT like so:

npx git-truck@0.0.0-d21ce66
npx git-truck@latest

My testing has showed that the user experience is much smoother and snappier.

Changes:

  • Remove @react-spring/web dependency
  • Remove useToggleableSpring hook
  • Refactor Node rendering to always use rect (with varying corner radius)
    • Circles have a corner radius of their radius
    • Rectangles have a fixed corner radius of 8 pixels
  • Refactor to single NodeText that handles both labels for circles and rectangles
  • Adjust animation duration and animation function

image

Copy link

codecov bot commented Nov 3, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (cf157b9) 59.52% compared to head (b61a78f) 59.52%.
Report is 7 commits behind head on main.

❗ Current head b61a78f differs from pull request most recent head b9f7d88. Consider uploading reports for the commit b9f7d88 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #662   +/-   ##
=======================================
  Coverage   59.52%   59.52%           
=======================================
  Files           2        2           
  Lines          42       42           
  Branches        7        7           
=======================================
  Hits           25       25           
  Misses         16       16           
  Partials        1        1           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@joglr joglr marked this pull request as draft November 3, 2023 16:10
@joglr joglr self-assigned this Nov 3, 2023
@joglr joglr mentioned this pull request Nov 7, 2023
@joglr joglr marked this pull request as ready for review November 7, 2023 22:15
@joglr joglr requested review from tjomson and removed request for tjomson November 7, 2023 22:22
@joglr joglr marked this pull request as draft November 7, 2023 22:29
@mircealungu
Copy link
Collaborator

when running: npx git-truck@0.0.0-9ac4b13 i see the following in the browser:

image

while this is what I see in the terminal:

image

@joglr
Copy link
Contributor Author

joglr commented Nov 8, 2023

when running: npx git-truck@0.0.0-9ac4b13 i see the following in the browser:

image

while this is what I see in the terminal:

image

Hey Mircea, thank you for taking the time to test this.

Does it work after a refresh of the page?

With Remix >2.0.0, we are no longer able to know when the web server is ready, as they removed an undocunented API that we relied on, so this might be a race condition issue with the web server not being ready yet. We will have to do some migration to a custom web server to fix this issue, or experiment with the new vite integration from Remix.

If you got the page to show, did the styles seem missing? I think I have narrowed the CSS issue down to occurring when publishing the package, which is always fun to debug, I will look into it.

@mircealungu
Copy link
Collaborator

@joglr - indeed, the page loads if I give it enough time!

and you're also right: the stylesheets do not load

image

@joglr
Copy link
Contributor Author

joglr commented Nov 10, 2023

I have provided a fix in another PR, should be incorporated into here as well.

Feel free to try it out with:

npx -y git-truck@0.0.0-d21ce66

@joglr joglr marked this pull request as ready for review November 10, 2023 01:00
@joglr joglr marked this pull request as draft November 10, 2023 01:00
@joglr joglr marked this pull request as ready for review November 10, 2023 01:09
@joglr joglr mentioned this pull request Nov 10, 2023
14 tasks
@joglr joglr added this to the UI milestone Nov 10, 2023
@joglr joglr modified the milestones: UI, Visualization Nov 10, 2023
@mircealungu
Copy link
Collaborator

I have provided a fix in another PR, should be incorporated into here as well.

Feel free to try it out with:

npx -y git-truck@0.0.0-d21ce66

This works!

Also, animation is faster:

  • zooming out from pandas/docs to pandas with the new version ~2.20s and old version ~3.80s

What I realized is that on this branch, the comments panel is visible to the detriment of the legend:
image

Can we make the legend panel always be visible? Either at the top of the right-hand side panel, or maybe as a separate panel, in bottom-right which pushes the details panel above it?

@dawidwoz
Copy link
Member

I have provided a fix in another PR, should be incorporated into here as well.

Feel free to try it out with:

npx -y git-truck@0.0.0-d21ce66

The animations are much smoother now. Good job. I found one bug when you set the depth to 1, you can see in this version the labels of nested folders (left on the picture) where they are not visible in the current main version (right on the picture).
image

@joglr
Copy link
Contributor Author

joglr commented Nov 12, 2023

I have provided a fix in another PR, should be incorporated into here as well.

Feel free to try it out with:

npx -y git-truck@0.0.0-d21ce66

This works!

Also, animation is faster:

  • zooming out from pandas/docs to pandas with the new version ~2.20s and old version ~3.80s

That is great to hear!

What I realized is that on this branch, the comments panel is visible to the detriment of the legend:
image

Can we make the legend panel always be visible? Either at the top of the right-hand side panel, or maybe as a separate panel, in bottom-right which pushes the details panel above it?

As far as I can tell, this is not new and I don't think it is related to this PR? I have created a new issue that address this.

#678

@joglr
Copy link
Contributor Author

joglr commented Nov 12, 2023

I have provided a fix in another PR, should be incorporated into here as well.

Feel free to try it out with:

npx -y git-truck@0.0.0-d21ce66

The animations are much smoother now. Good job. I found one bug when you set the depth to 1, you can see in this version the labels of nested folders (left on the picture) where they are not visible in the current main version (right on the picture).
image

Good catch! Will address this soon and update the PR.

@joglr
Copy link
Contributor Author

joglr commented Nov 14, 2023

@dawidwoz I have pushed a commit that address the bug that you noticed. Feel free to checkout the code and verify it works as intended.

Copy link
Member

@dawidwoz dawidwoz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now, it works great! 🙂

@joglr joglr merged commit ece341d into main Nov 15, 2023
4 checks passed
@joglr joglr deleted the css-transitions branch November 15, 2023 10:14
@mircealungu
Copy link
Collaborator

Nice :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants