Conversation
WalkthroughThis PR enhances TypeScript type safety for event handlers in Pie and RadialBar components. It exports Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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: 3
🤖 Fix all issues with AI agents
In `@src/polar/Pie.tsx`:
- Around line 222-224: The public touch event props onTouchStart, onTouchMove,
and onTouchEnd lack JSDoc like the mouse handlers; add JSDoc blocks above each
prop declaration (onTouchStart, onTouchMove, onTouchEnd) describing the callback
signature, parameters (data: PieSectorDataItem, index: number, e:
React.TouchEvent<SVGGraphicsElement>), when it's invoked, and any return
behavior so omnidoc will pick them up—mirror the style and phrasing used for the
corresponding mouse handlers to keep the API docs consistent.
In `@src/polar/RadialBar.tsx`:
- Around line 402-404: Add JSDoc comments for the new public touch event handler
props so they appear in generated docs: document onTouchStart, onTouchMove, and
onTouchEnd (each taking a RadialBarDataItem, index: number, and
React.TouchEvent<SVGGraphicsElement>) with brief descriptions of when they fire,
the meaning of the parameters, and that they are optional callback props on the
RadialBar component; place the JSDoc immediately above each property declaration
to match existing style and ensure omnidoc picks them up.
- Around line 418-421: The prop type uses
PresentationAttributesAdaptChildEvent<any, SVGElement> with any; replace the any
with the existing RadialBarDataItem type so RadialBarProps reads
PresentationAttributesAdaptChildEvent<RadialBarDataItem, SVGElement> (follow the
same pattern used in Bar.tsx which uses BarRectangleItem) and ensure
imports/types remain consistent with RadialBarDataItem.
| onTouchStart?: (data: PieSectorDataItem, index: number, e: React.TouchEvent<SVGGraphicsElement>) => void; | ||
| onTouchMove?: (data: PieSectorDataItem, index: number, e: React.TouchEvent<SVGGraphicsElement>) => void; | ||
| onTouchEnd?: (data: PieSectorDataItem, index: number, e: React.TouchEvent<SVGGraphicsElement>) => void; |
There was a problem hiding this comment.
Add JSDoc for new touch event handlers.
onTouchStart, onTouchMove, and onTouchEnd are public props but don’t have doc blocks like the mouse handlers, so omnidoc won’t surface them.
✍️ Suggested doc additions
interface PieEvents {
/**
* The customized event handler of mouseleave on the sectors in this group.
*/
onMouseLeave?: (data: PieSectorDataItem, index: number, e: React.MouseEvent<SVGGraphicsElement>) => void;
+ /**
+ * The customized event handler of touchstart on the sectors in this group.
+ */
onTouchStart?: (data: PieSectorDataItem, index: number, e: React.TouchEvent<SVGGraphicsElement>) => void;
+ /**
+ * The customized event handler of touchmove on the sectors in this group.
+ */
onTouchMove?: (data: PieSectorDataItem, index: number, e: React.TouchEvent<SVGGraphicsElement>) => void;
+ /**
+ * The customized event handler of touchend on the sectors in this group.
+ */
onTouchEnd?: (data: PieSectorDataItem, index: number, e: React.TouchEvent<SVGGraphicsElement>) => void;
}As per coding guidelines, src/**/*.{ts,tsx}: JSDoc comments and TypeScript definitions in source files must be kept up-to-date as they are used to autogenerate API documentation via 'npm run omnidoc'.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| onTouchStart?: (data: PieSectorDataItem, index: number, e: React.TouchEvent<SVGGraphicsElement>) => void; | |
| onTouchMove?: (data: PieSectorDataItem, index: number, e: React.TouchEvent<SVGGraphicsElement>) => void; | |
| onTouchEnd?: (data: PieSectorDataItem, index: number, e: React.TouchEvent<SVGGraphicsElement>) => void; | |
| /** | |
| * The customized event handler of touchstart on the sectors in this group. | |
| */ | |
| onTouchStart?: (data: PieSectorDataItem, index: number, e: React.TouchEvent<SVGGraphicsElement>) => void; | |
| /** | |
| * The customized event handler of touchmove on the sectors in this group. | |
| */ | |
| onTouchMove?: (data: PieSectorDataItem, index: number, e: React.TouchEvent<SVGGraphicsElement>) => void; | |
| /** | |
| * The customized event handler of touchend on the sectors in this group. | |
| */ | |
| onTouchEnd?: (data: PieSectorDataItem, index: number, e: React.TouchEvent<SVGGraphicsElement>) => void; |
🤖 Prompt for AI Agents
In `@src/polar/Pie.tsx` around lines 222 - 224, The public touch event props
onTouchStart, onTouchMove, and onTouchEnd lack JSDoc like the mouse handlers;
add JSDoc blocks above each prop declaration (onTouchStart, onTouchMove,
onTouchEnd) describing the callback signature, parameters (data:
PieSectorDataItem, index: number, e: React.TouchEvent<SVGGraphicsElement>), when
it's invoked, and any return behavior so omnidoc will pick them up—mirror the
style and phrasing used for the corresponding mouse handlers to keep the API
docs consistent.
| onTouchStart?: (data: RadialBarDataItem, index: number, e: React.TouchEvent<SVGGraphicsElement>) => void; | ||
| onTouchMove?: (data: RadialBarDataItem, index: number, e: React.TouchEvent<SVGGraphicsElement>) => void; | ||
| onTouchEnd?: (data: RadialBarDataItem, index: number, e: React.TouchEvent<SVGGraphicsElement>) => void; |
There was a problem hiding this comment.
Add JSDoc for new touch event handlers.
These new public handlers lack documentation, so omnidoc won’t include them.
✍️ Suggested doc additions
/**
* The customized event handler of mouseleave in this chart.
*/
onMouseLeave?: (data: RadialBarDataItem, index: number, e: React.MouseEvent<SVGGraphicsElement>) => void;
+ /**
+ * The customized event handler of touchstart in this chart.
+ */
onTouchStart?: (data: RadialBarDataItem, index: number, e: React.TouchEvent<SVGGraphicsElement>) => void;
+ /**
+ * The customized event handler of touchmove in this chart.
+ */
onTouchMove?: (data: RadialBarDataItem, index: number, e: React.TouchEvent<SVGGraphicsElement>) => void;
+ /**
+ * The customized event handler of touchend in this chart.
+ */
onTouchEnd?: (data: RadialBarDataItem, index: number, e: React.TouchEvent<SVGGraphicsElement>) => void;As per coding guidelines, src/**/*.{ts,tsx}: JSDoc comments and TypeScript definitions in source files must be kept up-to-date as they are used to autogenerate API documentation via 'npm run omnidoc'.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| onTouchStart?: (data: RadialBarDataItem, index: number, e: React.TouchEvent<SVGGraphicsElement>) => void; | |
| onTouchMove?: (data: RadialBarDataItem, index: number, e: React.TouchEvent<SVGGraphicsElement>) => void; | |
| onTouchEnd?: (data: RadialBarDataItem, index: number, e: React.TouchEvent<SVGGraphicsElement>) => void; | |
| /** | |
| * The customized event handler of touchstart in this chart. | |
| */ | |
| onTouchStart?: (data: RadialBarDataItem, index: number, e: React.TouchEvent<SVGGraphicsElement>) => void; | |
| /** | |
| * The customized event handler of touchmove in this chart. | |
| */ | |
| onTouchMove?: (data: RadialBarDataItem, index: number, e: React.TouchEvent<SVGGraphicsElement>) => void; | |
| /** | |
| * The customized event handler of touchend in this chart. | |
| */ | |
| onTouchEnd?: (data: RadialBarDataItem, index: number, e: React.TouchEvent<SVGGraphicsElement>) => void; |
🤖 Prompt for AI Agents
In `@src/polar/RadialBar.tsx` around lines 402 - 404, Add JSDoc comments for the
new public touch event handler props so they appear in generated docs: document
onTouchStart, onTouchMove, and onTouchEnd (each taking a RadialBarDataItem,
index: number, and React.TouchEvent<SVGGraphicsElement>) with brief descriptions
of when they fire, the meaning of the parameters, and that they are optional
callback props on the RadialBar component; place the JSDoc immediately above
each property declaration to match existing style and ensure omnidoc picks them
up.
| export type RadialBarProps = Omit< | ||
| PresentationAttributesAdaptChildEvent<any, SVGElement>, | ||
| 'ref' | keyof InternalRadialBarProps | ||
| > & |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check the current state of the file at the specified lines
sed -n '415,425p' src/polar/RadialBar.tsxRepository: recharts/recharts
Length of output: 361
🏁 Script executed:
# Find and examine PresentationAttributesAdaptChildEvent definition
rg -A 15 "type PresentationAttributesAdaptChildEvent|interface PresentationAttributesAdaptChildEvent" srcRepository: recharts/recharts
Length of output: 727
🏁 Script executed:
# Search for other uses of PresentationAttributesAdaptChildEvent to see patterns
rg "PresentationAttributesAdaptChildEvent<" src --no-heading -B 2 -A 2Repository: recharts/recharts
Length of output: 4203
🏁 Script executed:
# Look for DOMAttributesAdaptChildEvent definition
rg "type DOMAttributesAdaptChildEvent|interface DOMAttributesAdaptChildEvent" src -A 15Repository: recharts/recharts
Length of output: 909
🏁 Script executed:
# Check if there are any TODOs or tracking issues for typing PresentationAttributesAdaptChildEvent
rg "PresentationAttributesAdaptChildEvent|DOMAttributesAdaptChildEvent" src -B 3 | rg -i "TODO|FIXME|XXX|issue|typing|any" -B 2 -A 2Repository: recharts/recharts
Length of output: 2911
🏁 Script executed:
# Check Bar.tsx to see how it properly types the first parameter
sed -n '1,150p' src/cartesian/Bar.tsx | grep -A 20 "BarSvgProps"Repository: recharts/recharts
Length of output: 43
🏁 Script executed:
# Find the BarRectangleItem type definition and see how Bar.tsx uses it
rg "BarRectangleItem" src -B 5 -A 5Repository: recharts/recharts
Length of output: 19226
🏁 Script executed:
# Check if there are any issues or discussions about typing these attributes
git log --all --grep="PresentationAttributesAdaptChildEvent\|any\|typing" --oneline | head -20Repository: recharts/recharts
Length of output: 1471
🏁 Script executed:
# Search for any type definitions or guidance in CONTRIBUTING.md about this pattern
cat CONTRIBUTING.md | grep -A 10 -B 10 "any\|PresentationAttributesAdaptChildEvent" || echo "No matches in CONTRIBUTING.md"Repository: recharts/recharts
Length of output: 1053
🏁 Script executed:
# Check what RadialBar uses for its data structures - look for sector/point types
rg "sector|RadialBar" src/polar/RadialBar.tsx -B 2 -A 5 | head -80Repository: recharts/recharts
Length of output: 3005
🏁 Script executed:
# Check what type RadialBar uses in its event handlers
rg "onMouseEnter|onMouseLeave|onChange|onClick" src/polar/RadialBar.tsx -B 2 -A 3Repository: recharts/recharts
Length of output: 2348
🏁 Script executed:
# Check Pie.tsx for comparison (uses same pattern)
sed -n '1,100p' src/polar/Pie.tsx | grep -A 20 "type\|interface"Repository: recharts/recharts
Length of output: 2440
Use RadialBarDataItem type instead of any for SVG attributes.
PresentationAttributesAdaptChildEvent<any, SVGElement> violates the "never use any" guideline. Follow the pattern used in Bar.tsx (which uses BarRectangleItem) and use the existing RadialBarDataItem type defined in this file.
🧩 Proposed fix
-export type RadialBarProps = Omit<
- PresentationAttributesAdaptChildEvent<any, SVGElement>,
+export type RadialBarProps = Omit<
+ PresentationAttributesAdaptChildEvent<RadialBarDataItem, SVGElement>,
'ref' | keyof InternalRadialBarProps
> &
Omit<InternalRadialBarProps, 'sectors'>;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export type RadialBarProps = Omit< | |
| PresentationAttributesAdaptChildEvent<any, SVGElement>, | |
| 'ref' | keyof InternalRadialBarProps | |
| > & | |
| export type RadialBarProps = Omit< | |
| PresentationAttributesAdaptChildEvent<RadialBarDataItem, SVGElement>, | |
| 'ref' | keyof InternalRadialBarProps | |
| > & | |
| Omit<InternalRadialBarProps, 'sectors'>; |
🤖 Prompt for AI Agents
In `@src/polar/RadialBar.tsx` around lines 418 - 421, The prop type uses
PresentationAttributesAdaptChildEvent<any, SVGElement> with any; replace the any
with the existing RadialBarDataItem type so RadialBarProps reads
PresentationAttributesAdaptChildEvent<RadialBarDataItem, SVGElement> (follow the
same pattern used in Bar.tsx which uses BarRectangleItem) and ensure
imports/types remain consistent with RadialBarDataItem.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6965 +/- ##
=======================================
Coverage 91.13% 91.13%
=======================================
Files 509 509
Lines 37967 37967
Branches 5287 5287
=======================================
Hits 34601 34601
Misses 3357 3357
Partials 9 9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Bundle ReportBundle size has no change ✅ |
|
Staging Deployment Details
These deployments will remain available for 30 days. To update snapshots: Comment |
Description
Remove any, update the event generics so that it's internally compatible with getRelativeCoordinate
Related Issue
#6645
Summary by CodeRabbit
Release Notes
New Features
onTouchStart,onTouchMove,onTouchEnd) to Pie and RadialBar components for improved touch interaction support.Improvements
RadialBarDataItemtype for improved type support in custom event handlers.Tests