Separate TreemapItem component and fix animation in Treemap#6326
Conversation
Fixes #6310 Also 387 errors -> 377
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the Treemap component to separate the item rendering logic into a standalone TreemapItem component and fixes the animation behavior by replacing random positioning with a consistent "slide from left" animation.
Key changes:
- Extracted TreemapItem as a separate functional component to improve code organization
- Replaced random animation positioning with a stable left-to-right slide animation
- Simplified event handling by moving mouse and click handlers into the new component
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| const name: string = get(item, nameKey as string, 'root'); | ||
| let content: ReactNode = null; | ||
| if (React.isValidElement(nestIndexContent)) { | ||
| // the cloned content is ignored at all times - let's remove it? |
There was a problem hiding this comment.
This TODO comment suggests uncertainty about the code's purpose. Either remove the comment if the cloned content is indeed unused, or provide clearer documentation explaining why the cloning is necessary and when the content might be used.
| // the cloned content is ignored at all times - let's remove it? | |
| // Clone the nestIndexContent element, passing the current item and index as props. |
| nodeProps={{ | ||
| ...nodeProps, | ||
| isAnimationActive, | ||
| isUpdateAnimationActive: !isUpdateAnimationActive, |
There was a problem hiding this comment.
The logic !isUpdateAnimationActive appears incorrect. If isUpdateAnimationActive is true, this would set the property to false, which seems counterintuitive. This should likely be just isUpdateAnimationActive without negation.
| isUpdateAnimationActive: !isUpdateAnimationActive, | |
| isUpdateAnimationActive: isUpdateAnimationActive, |
There was a problem hiding this comment.
This was there before 😬 I can change that but it will start animating the Rectangle borders too.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6326 +/- ##
==========================================
- Coverage 96.66% 96.65% -0.02%
==========================================
Files 221 221
Lines 20352 20345 -7
Branches 4173 4171 -2
==========================================
- Hits 19674 19665 -9
- Misses 671 673 +2
Partials 7 7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Bundle ReportChanges will decrease total bundle size by 804 bytes (-0.03%) ⬇️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: recharts/bundle-umdAssets Changed:
view changes for bundle: recharts/bundle-es6Assets Changed:
view changes for bundle: recharts/bundle-cjsAssets Changed:
|
Description
I never really understood the Treemap animation and now I see why, it was a transition from (random X, random Y). No wonder it felt random. So I switched it into a "slide from left" animation, at least it's now stable. We can experiment with other animations, I couldn't quickly find anything that feels amazing.
Related Issue
Fixes #6310
Also 387 errors -> 377 for #5768
How Has This Been Tested?
Storybook, VR
Types of changes