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

feat: portfolio performance improvements #1746

Merged
merged 18 commits into from
Dec 16, 2024
Merged

Conversation

0xKheops
Copy link
Contributor

@0xKheops 0xKheops commented Dec 12, 2024

  • improves allBalances$ observable (one less pipe stage, and added a shareReplay as it s often read)
  • removed the shareReplay on authorisedSites$ observable, as it s wrapped in a bind (has a built-in shareReplay)
  • optimised few useMemo/useCallback that had a dependency on balances.each (which changes on each read, nullifying the memo effect) to have a dependency on balances instead.
  • virtualized both popup and dashboard asset tables using @tanstack/react-virtual
  • removed countUps on virtualized rows, as the count up animation would trigger too often
  • virtualized rows in token picker
  • virtualized rows in tx history

@0xKheops 0xKheops marked this pull request as draft December 12, 2024 15:26
Copy link

socket-security bot commented Dec 13, 2024

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/@tanstack/[email protected] None +1 196 kB
npm/@zondax/[email protected] None +6 1.15 MB jleni
npm/@zxing/[email protected] None 0 5.4 MB werthd
npm/@zxing/[email protected] None 0 9.46 MB werthd

View full report↗︎

@0xKheops 0xKheops marked this pull request as ready for review December 13, 2024 08:37
@0xKheops 0xKheops requested a review from chidg December 13, 2024 08:39
@UrbanWill UrbanWill self-requested a review December 16, 2024 02:54
UrbanWill
UrbanWill previously approved these changes Dec 16, 2024
Copy link
Contributor

@UrbanWill UrbanWill left a comment

Choose a reason for hiding this comment

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

LGTM!

However I am kinda sad seeing the count up go, I've create an example where we can keep it while virtualizing the rows without too much change in the current virtualization.

Have a look and let us know if you like it: #1750

Copy link
Contributor

@UrbanWill UrbanWill left a comment

Choose a reason for hiding this comment

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

🔥

@0xKheops 0xKheops merged commit eb959d4 into dev Dec 16, 2024
6 checks passed
@0xKheops 0xKheops deleted the fix/perf-improvements-202412 branch December 16, 2024 06:28
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.

2 participants