Conversation
WalkthroughThis PR addresses hover transition issues in bar charts by introducing a layered rendering system for active bars with staged CSS transitions. Changes include new enter/exit transition states in Bar rendering, a public Changes
Sequence DiagramsequenceDiagram
participant User
participant Bar as BarRectangleWithActiveState
participant RAF as requestAnimationFrame
participant Layer as ZIndexLayer
participant CSS as CSS Transition
participant Handler as transitionEnd
User->>Bar: Hover (activate)
activate Bar
Bar->>Bar: Set stayInLayer=true, hasMountedActive=false
Bar->>RAF: Schedule delay callback
deactivate Bar
RAF->>Bar: Trigger callback
activate Bar
Bar->>Bar: Set hasMountedActive=true
Bar->>Layer: Render inactive shape in layer
deactivate Bar
Note over Layer,CSS: Re-render triggered by state change
Bar->>Layer: Render with isVisuallyActive=true
Layer->>CSS: Apply width transition
activate CSS
CSS->>CSS: Animate width grow
deactivate CSS
CSS->>Handler: Fire transitionEnd event
activate Handler
Handler->>Bar: Call onTransitionEnd
Bar->>Bar: Set stayInLayer=false
deactivate Handler
Bar->>Layer: Remove conditional layer rendering
User->>Bar: Leave (deactivate)
activate Bar
Bar->>Bar: Set stayInLayer=true, trigger exit sequence
Bar->>Layer: Render inactive in layer for reversal
deactivate Bar
CSS->>Handler: Fire transitionEnd event
Handler->>Bar: Remove layer, render in place
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/cartesian/Bar.tsx`:
- Around line 10-12: The effect that listens for onTransitionEnd and toggles the
stayInLayer state can hang if no CSS transition exists; update the useEffect
that uses useRef/useState to add a fallback timeout: when entering the active
state start a timeout (store its id in a ref) that sets stayInLayer = false
after a short delay (e.g. 50–100ms), and in the onTransitionEnd handler clear
that timeout and set stayInLayer = false immediately; also ensure the cleanup of
the effect clears the timeout and removes the transition listener to avoid
leaks; reference the stayInLayer state, the effect that attaches
onTransitionEnd, and the timeout ref used to store the timer, and ensure class
selectors '.recharts-active-bar' and '.recharts-inactive-bar' behavior remains
unchanged.
In `@src/util/types.ts`:
- Around line 37-40: The import for TextProps is pulled in as a runtime import
but it's only used as a type; change the import to a type-only import by
updating the import statement to use the type modifier (e.g., import type {
TextProps } from '../index') so TextProps is erased at runtime and no
unnecessary module side-effects occur; locate the existing import block that
currently reads "import { TextProps } from '../index';" and replace it with the
type-only form while leaving other imports (TextAnchor, TextVerticalAnchor,
TickFormatter) unchanged.
| useEffect, | ||
| useRef, | ||
| useState, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find and examine src/cartesian/Bar.tsx
wc -l src/cartesian/Bar.tsxRepository: recharts/recharts
Length of output: 87
🏁 Script executed:
# Read the file around lines 602-672 to understand the transition handling
sed -n '590,680p' src/cartesian/Bar.tsx | cat -nRepository: recharts/recharts
Length of output: 3469
🏁 Script executed:
# Search for stayInLayer, hasMountedActive, and onTransitionEnd references
rg -n "stayInLayer|hasMountedActive|onTransitionEnd" src/cartesian/Bar.tsxRepository: recharts/recharts
Length of output: 482
🏁 Script executed:
# Look at the BarRectangle component to see how onTransitionEnd is handled
rg -n "BarRectangle" src/cartesian/Bar.tsx -A 3 -B 3 | head -60Repository: recharts/recharts
Length of output: 1990
🏁 Script executed:
# Search for the BarRectangle component definition
fd -t f "BarRectangle" src/Repository: recharts/recharts
Length of output: 43
🏁 Script executed:
# Check if there are any CSS or style definitions for transitions in Bar or BarRectangle
rg -n "transition|onTransitionEnd" src/cartesian/Bar.tsx -B 2 -A 2Repository: recharts/recharts
Length of output: 843
🏁 Script executed:
# Look at related files for CSS definitions or transition handling
find src -name "*.css" -o -name "*.scss" | xargs grep -l "transition" 2>/dev/null | head -10Repository: recharts/recharts
Length of output: 43
🏁 Script executed:
# Find BarUtils and examine the BarRectangle component
cat -n src/util/BarUtils.tsx | head -200Repository: recharts/recharts
Length of output: 2372
🏁 Script executed:
# Search for CSS or className with transition related content in BarUtils
rg -n "className|style" src/util/BarUtils.tsx | head -30Repository: recharts/recharts
Length of output: 43
🏁 Script executed:
# Look for the actual SVG rectangle element and how onTransitionEnd is attached
rg -n "onTransitionEnd|onAnimationEnd" src/util/BarUtils.tsx -B 3 -A 3Repository: recharts/recharts
Length of output: 288
🏁 Script executed:
# Check if there's a CSS file or style definitions for Bar component
find src -type f \( -name "*.css" -o -name "*.scss" -o -name "*.less" \) | head -20Repository: recharts/recharts
Length of output: 43
🏁 Script executed:
# Find the Shape component and examine how it handles onTransitionEnd
fd -type f "ActiveShapeUtils" src/Repository: recharts/recharts
Length of output: 231
🏁 Script executed:
# Read the ActiveShapeUtils to see how Shape handles transitions
cat -n src/util/ActiveShapeUtils.tsxRepository: recharts/recharts
Length of output: 4313
🏁 Script executed:
# Search for CSS files that might define recharts-active-bar and recharts-inactive-bar
rg -n "recharts-active-bar|recharts-inactive-bar" src/Repository: recharts/recharts
Length of output: 202
🏁 Script executed:
# Check if there are any CSS or SCSS files with transition definitions
find src -type f \( -name "*.css" -o -name "*.scss" \) -exec grep -l "recharts-active-bar\|recharts-inactive-bar\|transition" {} \;Repository: recharts/recharts
Length of output: 43
🏁 Script executed:
# Search for CSS files in the entire repo
find src -name "*.css" -o -name "*.scss" -o -name "*.less" 2>/dev/nullRepository: recharts/recharts
Length of output: 43
🏁 Script executed:
# Search for any transition or animation definitions in the codebase
rg -n "transition|animation" src/ -t css -t scssRepository: recharts/recharts
Length of output: 89
🏁 Script executed:
# Check if styles are defined inline or in any index files
rg -n "recharts-active-bar\|recharts-inactive-bar" . --include="*.ts" --include="*.tsx" --include="*.js" --include="*.jsx" --include="*.css"Repository: recharts/recharts
Length of output: 502
🏁 Script executed:
# Check the Rectangle component to see how it handles the className and onTransitionEnd
cat -n src/shape/Rectangle.tsx | head -150Repository: recharts/recharts
Length of output: 6229
🏁 Script executed:
# Continue reading Rectangle component to find onTransitionEnd handling
sed -n '150,350p' src/shape/Rectangle.tsx | cat -nRepository: recharts/recharts
Length of output: 6266
🏁 Script executed:
# Search for onTransitionEnd in Rectangle component
rg -n "onTransitionEnd" src/shape/Rectangle.tsx -B 5 -A 5Repository: recharts/recharts
Length of output: 43
🏁 Script executed:
# Check if there's a Layer component that might handle classes or transitions
cat -n src/container/Layer.tsxRepository: recharts/recharts
Length of output: 1183
🏁 Script executed:
# Check what svgPropertiesAndEvents includes
cat -n src/util/svgPropertiesAndEvents.tsRepository: recharts/recharts
Length of output: 2998
🏁 Script executed:
# Look for onTransitionEnd in the entire codebase
rg -n "onTransitionEnd" src/Repository: recharts/recharts
Length of output: 569
🏁 Script executed:
# Check if there's any CSS file that might define transitions for recharts-active-bar
find . -name "*.css" -o -name "*.scss" 2>/dev/null | head -20Repository: recharts/recharts
Length of output: 856
🏁 Script executed:
# Look at how the BarRectangle spreads props to Shape
sed -n '1,60p' src/util/BarUtils.tsx | cat -nRepository: recharts/recharts
Length of output: 2372
🏁 Script executed:
# Check if onTransitionEnd is properly propagated through svgPropertiesAndEvents
cat -n src/util/excludeEventProps.ts | grep -A 5 -B 5 "onTransitionEnd"Repository: recharts/recharts
Length of output: 467
🏁 Script executed:
# Look for any CSS defining transitions in the storybook examples or global styles
grep -r "recharts-active-bar\|recharts-inactive-bar" storybook/ www/ 2>/dev/null | head -20Repository: recharts/recharts
Length of output: 43
🏁 Script executed:
# Check if there are any default transitions or styles in the main library
find src -type f \( -name "*.ts" -o -name "*.tsx" \) -exec grep -l "transition\|animation" {} \; | head -10Repository: recharts/recharts
Length of output: 942
🏁 Script executed:
# Examine the actual Bar component integration test or examples to see if CSS transitions are expected
rg -n "recharts-active-bar|recharts-inactive-bar" . -l | head -10Repository: recharts/recharts
Length of output: 191
🏁 Script executed:
# Read the CSS transition test to understand the expected behavior
cat -n test/cartesian/Bar/Bar.csstransition.spec.tsxRepository: recharts/recharts
Length of output: 13201
🏁 Script executed:
# Check the expectBars helper to see what it verifies
cat -n test/helper/expectBars.tsRepository: recharts/recharts
Length of output: 2018
🏁 Script executed:
# Look for any storybook stories that demonstrate the active bar behavior
find . -name "*.stories.tsx" -o -name "*.stories.ts" | xargs grep -l "recharts-active-bar\|isActive" 2>/dev/null | head -5Repository: recharts/recharts
Length of output: 240
Add fallback timeout when CSS transitions are absent.
The code relies entirely on the onTransitionEnd event to restore bars to the base layer when exiting the active state. If CSS transitions aren't defined on the .recharts-active-bar and .recharts-inactive-bar classes, onTransitionEnd will never fire and stayInLayer will remain true, leaving inactive bars stuck in the higher z-index active layer.
Recommend adding a timeout fallback to detect zero-duration transitions or to ensure cleanup even without CSS transitions defined.
🤖 Prompt for AI Agents
In `@src/cartesian/Bar.tsx` around lines 10 - 12, The effect that listens for
onTransitionEnd and toggles the stayInLayer state can hang if no CSS transition
exists; update the useEffect that uses useRef/useState to add a fallback
timeout: when entering the active state start a timeout (store its id in a ref)
that sets stayInLayer = false after a short delay (e.g. 50–100ms), and in the
onTransitionEnd handler clear that timeout and set stayInLayer = false
immediately; also ensure the cleanup of the effect clears the timeout and
removes the transition listener to avoid leaks; reference the stayInLayer state,
the effect that attaches onTransitionEnd, and the timeout ref used to store the
timer, and ensure class selectors '.recharts-active-bar' and
'.recharts-inactive-bar' behavior remains unchanged.
| import { TextAnchor, TextVerticalAnchor } from '../component/Text'; | ||
| import type { TickFormatter } from '../cartesian/CartesianAxis'; | ||
| import { TextProps } from '../index'; | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check where TextProps is defined and exported
rg -n "export (type |interface )?TextProps" src/component
# Check current imports and usage in types.ts
head -50 src/util/types.ts | cat -n
# Check tsconfig settings for module syntax
rg -n "verbatimModuleSyntax|importsNotUsedAsValues|preserveValueImports" -g "tsconfig*.json"
# Look for potential circular dependencies
rg -n "from.*types" src/component/Text.tsx | head -20Repository: recharts/recharts
Length of output: 2253
🏁 Script executed:
# Find where TextProps is exported
rg -n "export.*TextProps" src/
# Check TypeScript config settings
cat tsconfig.json | grep -E "verbatimModuleSyntax|importsNotUsedAsValues|preserveValueImports"
# Check what index.ts exports regarding TextProps
rg -n "TextProps" src/index.ts
# Verify if TextProps is in Text component
cat src/component/Text.tsx | head -100Repository: recharts/recharts
Length of output: 3503
🏁 Script executed:
# Verify TextProps is actually accessible from ../component/Text and check for any type-only constraints
rg -A 5 "^interface TextProps" src/component/Text.tsx
# Confirm that using type keyword is supported (check if other type-only imports exist)
rg "import.*type {.*}" src/util/types.ts | head -5
# Double-check line 795 context to see the TickProp usage
sed -n '790,800p' src/util/types.ts | cat -nRepository: recharts/recharts
Length of output: 771
🏁 Script executed:
# Check if type-only imports are used elsewhere in the file
grep -n "import type" src/util/types.ts
# Verify line 795 exactly
sed -n '795p' src/util/types.tsRepository: recharts/recharts
Length of output: 276
Use type-only import for TextProps to avoid unnecessary runtime import.
TextProps is only used in the type definition on line 795, so import it with the type modifier directly from the source to eliminate a runtime import.
♻️ Proposed fix
-import { TextAnchor, TextVerticalAnchor } from '../component/Text';
-import type { TickFormatter } from '../cartesian/CartesianAxis';
-import { TextProps } from '../index';
+import { TextAnchor, TextVerticalAnchor, type TextProps } from '../component/Text';
+import type { TickFormatter } from '../cartesian/CartesianAxis';🤖 Prompt for AI Agents
In `@src/util/types.ts` around lines 37 - 40, The import for TextProps is pulled
in as a runtime import but it's only used as a type; change the import to a
type-only import by updating the import statement to use the type modifier
(e.g., import type { TextProps } from '../index') so TextProps is erased at
runtime and no unnecessary module side-effects occur; locate the existing import
block that currently reads "import { TextProps } from '../index';" and replace
it with the type-only form while leaving other imports (TextAnchor,
TextVerticalAnchor, TickFormatter) unchanged.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6920 +/- ##
========================================
Coverage 94.26% 94.27%
========================================
Files 569 570 +1
Lines 55728 55855 +127
Branches 5185 5202 +17
========================================
+ Hits 52532 52657 +125
- Misses 3187 3189 +2
Partials 9 9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Staging Deployment Details
These deployments will remain available for 30 days. To update snapshots: Comment |
|
legend 🌹 |
Description
Bar transitions were broken since the ZIndex layers but indeed they are useful! So I put them back.
The process is rather convoluted so what was before "just render" is now a multi step adventure:
When a Bar becomes inactive, the process is even more complicated because we need to wait until the transition completes before we can move it back:
onTransitionEndIt's ... complicated. But it works. As far as I can tell. If you would like to test some more please do. Or perhaps if you have ideas how to solve this better.
Related Issue
Fixes #6891
Fixes #6856
Motivation and Context
CSS transitions are very nice to have. Without those we may just as well render to canvas!
Types of changes
Checklist:
Summary by CodeRabbit
Release Notes
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.