Add chart animation timing and donut hover callback - #2440
Merged
Conversation
Shares one animation duration/easing across BarChart and DonutChart (recharts defaults differ per element, so side-by-side charts finished transitioning at different times), fixes BarChart XAxis label clipping, and adds an optional onSliceHover callback to DonutChart. Ported from openops-cloud/openops-internal#992.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the ui-components chart primitives to align Recharts animation timing between BarChart and DonutChart, fixes X-axis label clipping in BarChart, and introduces an optional onSliceHover callback in DonutChart for hover-driven UI.
Changes:
- Introduce shared animation constants (
CHART_ANIMATION_DURATION_MS,CHART_ANIMATION_EASING) and apply them toBarChartandDonutChart. - Reserve additional
XAxisheight inBarChartto prevent tick/label clipping. - Add
DonutChart’sonSliceHoverprop and wire it to Recharts hover events.
Blocking
DonutCharthover callback currently clears hover state on per-sliceonMouseLeave, which can emitnullwhen moving between slices and cause flicker for hover-driven UI. (See PR comment.)
Non-blocking
- None
Merge recommendation
- Do not merge
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/ui-components/src/ui/chart/chart.tsx | Adds shared animation duration/easing constants for consistent cross-chart transitions. |
| packages/ui-components/src/ui/chart/bar-chart.tsx | Applies shared animation settings to bars and increases XAxis height to avoid label clipping. |
| packages/ui-components/src/ui/chart/donut-chart.tsx | Applies shared animation settings to pie and adds onSliceHover hover callback wiring. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
onMouseLeave on Pie fires per-sector, so crossing directly from one slice into an adjacent one briefly clears the hover state before the next slice's onMouseEnter sets it again. Move the clear to the PieChart container's onMouseLeave instead.
|
ravikiranvm
changed the title
MarceloRGonc
approved these changes
Aug 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes OPS-4728.
Summary
CHART_ANIMATION_DURATION_MS,CHART_ANIMATION_EASING) acrossBarChartandDonutChartso side-by-side charts finish transitions at the same pace — recharts defaults differ per element (Bar 400ms, Pie 800ms, Line 1500ms)BarChartXAxis label clipping by reservingheight={40}for the axis (default 30px isn't enough for the 20px tickMargin plus label text)onSliceHovercallback toDonutChartfor hover-driven UI (e.g. swapping a center label on hover)