perf: memoize investment chart tooltip to fix dropped frames - #533
Conversation
|
@toheebatanda752-stack is attempting to deploy a commit to the David Dada's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
@toheebatanda752-stack Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
sshdopey
left a comment
There was a problem hiding this comment.
Thanks for tackling this performance issue! Memoizing the Sparkline and wrapping Portfolio in memo are solid steps toward reducing re-renders on hover. The changes are focused and align with the issue's goal. I appreciate the effort to isolate tooltip state and stabilize handlers. Keep up the great work! 💖
|
Approved! 🎉 I couldn't auto-merge this just yet (it may need a rebase or have a check still running). A maintainer can merge it whenever it's ready. |
sshdopey
left a comment
There was a problem hiding this comment.
Thanks for tackling this performance issue! Memoizing the chart components and isolating tooltip state is a solid approach to reduce re-renders. I see you've added memo wrappers and stabilized handlers, which should help with frame drops. There are a few minor syntax quirks (like the viewBox template literal and CSS variable syntax) that might need a quick check, but overall this is a genuine effort toward the linked issue. Great work! ❤️
sshdopey
left a comment
There was a problem hiding this comment.
Thanks for tackling this performance issue! Memoizing the chart components and stabilizing handlers is a solid approach to reduce re-renders. I noticed a few typos like var--ink-40 and viewBox={0 0 ...}`` that might cause issues, but they're easy to fix. The overall direction is great and shows real effort. Keep it up! ❤️
|
Approved! 🎉 I couldn't auto-merge this just yet (it may need a rebase or have a check still running). A maintainer can merge it whenever it's ready. |
1 similar comment
|
Approved! 🎉 I couldn't auto-merge this just yet (it may need a rebase or have a check still running). A maintainer can merge it whenever it's ready. |
sshdopey
left a comment
There was a problem hiding this comment.
Thanks for tackling the chart re-render performance issue! Memoizing Sparkline, Portfolio, and ProjectDetail, plus stabilizing handlers, is a solid approach to isolate tooltip updates. I noticed a few minor syntax quirks (like var--ink-40 and the template literal in viewBox) that might cause runtime issues, but the overall direction is great and clearly addresses the linked issue. Keep up the good work! ❤️
|
Approved! 🎉 I couldn't auto-merge this just yet (it may need a rebase or have a check still running). A maintainer can merge it whenever it's ready. |
sshdopey
left a comment
There was a problem hiding this comment.
Thanks for tackling this performance issue! Memoizing the Sparkline and isolating tooltip state is a solid approach to prevent re-renders on hover. I noticed a couple of small typos (like var---ink-40) and an extra brace in the viewBox), but those are easy fixes. The overall direction is great and the verification steps show real effort. Keep up the good work! ❤️
sshdopey
left a comment
There was a problem hiding this comment.
This is a solid effort toward fixing the chart re-render performance issue! Memoizing Sparkline, Portfolio, and ProjectDetail, plus stabilizing handlers, directly addresses the dropped frames. I noticed a couple of small typos (like var---ink-40) and a stray } in the viewBox) that might cause issues, but they're easy to fix. Great work isolating tooltip state and keeping the chart stable. Thanks for tackling this! ❤️
|
Approved! 🎉 I couldn't auto-merge this just yet (it may need a rebase or have a check still running). A maintainer can merge it whenever it's ready. |
sshdopey
left a comment
There was a problem hiding this comment.
This is a solid effort toward fixing the chart re-render performance issue! Memoizing Sparkline, Portfolio, and ProjectDetail, plus stabilizing handlers, directly targets the problem. I noticed a couple of small typos (like var---ink-40) and a stray } in the viewBox) that might cause issues, but they're easy to fix. Great work on isolating tooltip state and keeping the chart stable. Thanks for contributing! ❤️
|
Approved! 🎉 I couldn't auto-merge this just yet (it may need a rebase or have a check still running). A maintainer can merge it whenever it's ready. |
1 similar comment
|
Approved! 🎉 I couldn't auto-merge this just yet (it may need a rebase or have a check still running). A maintainer can merge it whenever it's ready. |
Overview
This PR fixes the performance issue where the investment chart re-renders on every mouse move, causing dropped frames. The chart's tooltip was coupled to the full chart render path: each hover event created new callback/object references, which invalidated memoization boundaries and forced
Sparkline,Portfolio, andProjectDetailto re-render. This PR memoizes the chart components, stabilizes event handlers, and isolates tooltip state so only the tooltip layer updates while the chart itself remains stable.Related Issue
Closes the reported issue: Investment chart re-renders on every mouse move — frames drop.
Changes
⚡ Investment Chart Tooltip Memoization
[MODIFY]
src/components/Sparkline.tsx[MODIFY]
src/screens/Portfolio.tsxuseCallbackhandlers.Sparklinere-renders on mouse move.[MODIFY]
src/screens/ProjectDetail.tsxVerification Results
Sparkline,Portfolio.tsx, andProjectDetail.tsxupdatedCloses #451