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

perf: tweaks to speed up sidebar transition on desktop #3120

Merged
merged 3 commits into from
Dec 13, 2023

Conversation

patosullivan
Copy link
Member

@patosullivan patosullivan commented Dec 11, 2023

(partially) Fixes LAND-1360 by reducing re-rendering throughout the Sidebar and GroupSidebar components and their children.

One major source of re-rendering was the data from useGroup() getting wiped on navigating away from a group, which caused the GroupSidebar itself to re-render and show loading state/skeleton on the way out. I added keepPreviousData to the useGroup() query to keep current data until we want to load in a new group.

I also memoized as much as seemed appropriate throughout.

I played around with animationConfig settings in GroupsNav, but nothing stood out as actually improving the perceived speed.

I also attempted to use framer-motion's useIsPresent hook to prevent certain components from rendering (including preventing group images from loading in the sidebar), but it usually either didn't make an appreciable difference or it added too much overhead for it to be worth it.

I also realized that the number of notifications in the Activity list in the home screen has an effect on how quickly we transition from viewing a group to returning to the home screen, and attempted to mitigate this by using Virtuoso to turn the notifications list into a virtualized list. This didn't make any appreciable difference. I think we ought to think of other ways to optimize here (maybe we ought to page the notifications?).

To get a good feel of how these changes will actually feel in production, I recommend running a build, running serve, and pointing the frontend at your personal ship. If you want to see what I'm talking about, re: the notifications themselves causing it to feel slow, make sure you have a lot of notifications. You ought to also test in Safari, where the issue was most pronounced.

Here is how it looks now in that scenario:

Screen.Recording.2023-12-11.at.2.54.32.PM.mov

Copy link

linear bot commented Dec 11, 2023

Copy link
Member

@latter-bolden latter-bolden left a comment

Choose a reason for hiding this comment

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

Code looks good, does appear to be sped up on Safari.

@mrozanski mrozanski merged commit c60fdf5 into develop Dec 13, 2023
1 check passed
@mrozanski mrozanski deleted the po/perf/land-1360-sidebar-transition-slow branch December 13, 2023 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants