Reducing storybooks#6681
Conversation
WalkthroughThis PR adds a public Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ 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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (16)
package-lock.jsonis excluded by!**/package-lock.jsontest-vr/__snapshots__/tests/CartesianAxis/CartesianAxis.multi.spec-vr.tsx-snapshots/CartesianAxis-Multiple-Axes-1-chromium-linux.pngis excluded by!**/*.pngtest-vr/__snapshots__/tests/CartesianAxis/CartesianAxis.multi.spec-vr.tsx-snapshots/CartesianAxis-Multiple-Axes-1-firefox-linux.pngis excluded by!**/*.pngtest-vr/__snapshots__/tests/CartesianAxis/CartesianAxis.multi.spec-vr.tsx-snapshots/CartesianAxis-Multiple-Axes-1-webkit-linux.pngis excluded by!**/*.pngtest-vr/__snapshots__/tests/RadarChart/RadarChart.datatypes.spec-vr.tsx-snapshots/category-angle-type-1-chromium-linux.pngis excluded by!**/*.pngtest-vr/__snapshots__/tests/RadarChart/RadarChart.datatypes.spec-vr.tsx-snapshots/category-angle-type-1-firefox-linux.pngis excluded by!**/*.pngtest-vr/__snapshots__/tests/RadarChart/RadarChart.datatypes.spec-vr.tsx-snapshots/category-angle-type-1-webkit-linux.pngis excluded by!**/*.pngtest-vr/__snapshots__/tests/RadarChart/RadarChart.datatypes.spec-vr.tsx-snapshots/number-angle-type-with-default-domain-1-chromium-linux.pngis excluded by!**/*.pngtest-vr/__snapshots__/tests/RadarChart/RadarChart.datatypes.spec-vr.tsx-snapshots/number-angle-type-with-default-domain-1-firefox-linux.pngis excluded by!**/*.pngtest-vr/__snapshots__/tests/RadarChart/RadarChart.datatypes.spec-vr.tsx-snapshots/number-angle-type-with-default-domain-1-webkit-linux.pngis excluded by!**/*.pngtest-vr/__snapshots__/tests/RadarChart/RadarChart.datatypes.spec-vr.tsx-snapshots/number-angle-type-with-explicit-domain-1-chromium-linux.pngis excluded by!**/*.pngtest-vr/__snapshots__/tests/RadarChart/RadarChart.datatypes.spec-vr.tsx-snapshots/number-angle-type-with-explicit-domain-1-firefox-linux.pngis excluded by!**/*.pngtest-vr/__snapshots__/tests/RadarChart/RadarChart.datatypes.spec-vr.tsx-snapshots/number-angle-type-with-explicit-domain-1-webkit-linux.pngis excluded by!**/*.pngtest-vr/__snapshots__/tests/RadarChart/RadarChart.datatypes.spec-vr.tsx-snapshots/should-be-correct-angle-with-odd-number-of-points-1-chromium-linux.pngis excluded by!**/*.pngtest-vr/__snapshots__/tests/RadarChart/RadarChart.datatypes.spec-vr.tsx-snapshots/should-be-correct-angle-with-odd-number-of-points-1-firefox-linux.pngis excluded by!**/*.pngtest-vr/__snapshots__/tests/RadarChart/RadarChart.datatypes.spec-vr.tsx-snapshots/should-be-correct-angle-with-odd-number-of-points-1-webkit-linux.pngis excluded by!**/*.png
📒 Files selected for processing (13)
src/polar/RadialBar.tsx(1 hunks)storybook/stories/API/chart/RadarChart.stories.tsx(2 hunks)storybook/stories/Examples/AreaChart.stories.tsx(0 hunks)storybook/stories/Examples/BarChart/BarChart.stories.tsx(0 hunks)storybook/stories/Examples/LineChart/LineChart.stories.tsx(1 hunks)storybook/stories/Examples/RadarChart.stories.tsx(0 hunks)storybook/stories/Examples/cartesian/Bar/CustomizedShape.stories.tsx(0 hunks)storybook/stories/Examples/cartesian/Bar/StackedAndUnstacked.stories.tsx(0 hunks)storybook/stories/Examples/cartesian/CartesianAxis/MultipleAxes.stories.tsx(0 hunks)storybook/stories/Examples/cartesian/Line/LineWithCustomDot.stories.tsx(0 hunks)storybook/stories/Examples/cartesian/Line/LineWithCustomLabel.stories.tsx(0 hunks)test-vr/tests/CartesianAxis/CartesianAxis.multi.spec-vr.tsx(1 hunks)test-vr/tests/RadarChart/RadarChart.datatypes.spec-vr.tsx(1 hunks)
💤 Files with no reviewable changes (8)
- storybook/stories/Examples/BarChart/BarChart.stories.tsx
- storybook/stories/Examples/AreaChart.stories.tsx
- storybook/stories/Examples/cartesian/CartesianAxis/MultipleAxes.stories.tsx
- storybook/stories/Examples/cartesian/Bar/StackedAndUnstacked.stories.tsx
- storybook/stories/Examples/cartesian/Bar/CustomizedShape.stories.tsx
- storybook/stories/Examples/cartesian/Line/LineWithCustomDot.stories.tsx
- storybook/stories/Examples/cartesian/Line/LineWithCustomLabel.stories.tsx
- storybook/stories/Examples/RadarChart.stories.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
test-vr/tests/CartesianAxis/CartesianAxis.multi.spec-vr.tsx (1)
storybook/stories/data/Tick.ts (1)
ticks(16-16)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build, Test, Pack
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (5)
storybook/stories/API/chart/RadarChart.stories.tsx (1)
4-22: Legend usage in Simple RadarChart story looks correctImporting
Legendfrom the local src and rendering<Legend />inside theRadarChartmatches existing patterns and provides a clear legend example; no issues spotted.storybook/stories/Examples/LineChart/LineChart.stories.tsx (1)
333-377: Confirm newCustomizedDotthreshold (value >= 800) is intentionalChanging the cutoff from a much higher value to
value >= 800will mark many more points with the red icon; if this is just to make the story more visually distinct, it’s fine, but please double‑check this isn’t a leftover tweak from debugging.src/polar/RadialBar.tsx (1)
329-333: RadialBarlabelprop typing now matches usage and defaultsExposing
label?: ImplicitLabelListTypeonInternalRadialBarProps(with@defaultValue false) aligns the public type withdefaultRadialBarProps.labeland the existingLabelListFromLabelPropusage; this is a clean, type‑level fix with no behavior change.test-vr/tests/CartesianAxis/CartesianAxis.multi.spec-vr.tsx (1)
1-77: Multi‑axis CartesianAxis VR test is well‑structuredMounting four differently oriented
CartesianAxisinstances in a singleSurfaceand asserting a screenshot gives solid coverage for axis layout and labeling; implementation looks straightforward and sound.test-vr/tests/RadarChart/RadarChart.datatypes.spec-vr.tsx (1)
1-92: RadarChart datatype VR coverage is comprehensiveThe four Playwright tests nicely exercise numeric vs category angle axes and the odd‑point angle distribution, each with a screenshot assertion; this should catch regressions in RadarChart’s polar coordinate handling.
| <Radar type="number" name="r" dataKey="r" fillOpacity={0} stroke="#000" /> | ||
| <RechartsHookInspector /> | ||
| </RadarChart>, | ||
| ); | ||
| await expect(component).toHaveScreenshot(); | ||
| }); | ||
|
|
||
| test('number angle type with default domain', async ({ mount }) => { | ||
| const component = await mount( | ||
| <RadarChart | ||
| width={360} | ||
| height={360} | ||
| data={[ | ||
| { angle: 0, r: 11 }, | ||
| { angle: 90, r: 22 }, | ||
| { angle: 180, r: 33 }, | ||
| { angle: 270, r: 44 }, | ||
| ]} | ||
| > | ||
| <PolarGrid gridType="circle" /> | ||
| <PolarRadiusAxis type="number" dataKey="r" /> | ||
| <PolarAngleAxis dataKey="angle" axisLineType="circle" type="number" /> | ||
| <Radar type="number" name="r" dataKey="r" fillOpacity={0} stroke="#000" /> | ||
| </RadarChart>, | ||
| ); | ||
| await expect(component).toHaveScreenshot(); | ||
| }); | ||
|
|
||
| test('category angle type', async ({ mount }) => { | ||
| const component = await mount( | ||
| <RadarChart | ||
| width={360} | ||
| height={360} | ||
| data={[ | ||
| { angle: 0, r: 11 }, | ||
| { angle: 90, r: 22 }, | ||
| { angle: 180, r: 33 }, | ||
| { angle: 270, r: 44 }, | ||
| ]} | ||
| > | ||
| <PolarGrid gridType="circle" /> | ||
| <PolarRadiusAxis type="category" dataKey="r" /> | ||
| <PolarAngleAxis dataKey="angle" axisLineType="circle" type="category" /> | ||
| <Radar type="number" name="r" dataKey="r" fillOpacity={0} stroke="#000" /> |
There was a problem hiding this comment.
🧩 Analysis chain
Verify type="number" on <Radar> is a supported and meaningful value
In the first three tests, Radar is rendered with type="number", while the numeric/category typing is already configured on PolarAngleAxis/PolarRadiusAxis. Please confirm that "number" is an accepted type value for Radar and that it affects rendering as intended; if not, consider omitting it to avoid confusion.
🌐 Web query:
What valid values does the `type` prop of the `Radar` component support in Recharts 3.5.0? Is `"number"` a documented or recommended value?
💡 Result:
Short answer: The Radar component in Recharts 3.5.0 does not expose a "type" prop. Its documented props (dataKey, points, shape, dot, legendType, label, animation props, etc.) are listed in the Radar API; there is no "type" option for Radar. [1]
Notes:
- dataKey on Radar accepts String | Number | Function — so a Number is valid as a dataKey, but that is not a Radar "type" value. [1]
- The "type" prop exists on other components (e.g. Line/Area/Curve) and there it accepts specific interpolation strings like "linear", "monotone", "basis", "step", etc.; "number" is not among those documented string values. [2]
References:
- Recharts Radar API (props list, dataKey types). [1]
- Recharts Line / Curve API (documented values for a component that does have a "type" prop). [2]
Remove unsupported type prop from <Radar> components
The type prop is not supported on the Recharts Radar component in version 3.5.0. The Radar component's documented props (dataKey, points, shape, dot, legendType, label, animation props, etc.) do not include a "type" option. Remove type="number" from all three <Radar> instances in this test file.
🤖 Prompt for AI Agents
In test-vr/tests/RadarChart/RadarChart.datatypes.spec-vr.tsx around lines 21 to
64, the Radar components include an unsupported prop type="number"; remove the
type="number" attribute from all three <Radar> instances in this block so the
Radar elements only keep supported props (name, dataKey, fillOpacity, stroke,
etc.), then save the file and re-run the visual tests.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6681 +/- ##
=======================================
Coverage 94.02% 94.02%
=======================================
Files 499 499
Lines 42575 42575
Branches 4873 4873
=======================================
Hits 40032 40032
Misses 2538 2538
Partials 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Bundle ReportBundle size has no change ✅ |
Description
I found and removed some stories that were duplicated (present in both API and Examples, or present in both Line and LineChart sections)
Related Issue
#6668
Summary by CodeRabbit
New Features
labelproperty to RadialBar component for enhanced customization.Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.