Conversation
|
Caution Review failedFailed to post review comments WalkthroughThis PR introduces new API examples for axis inverse snap scale and tick hooks, adds corresponding example components demonstrating data and tick snapping with pixel-to-data conversions, refactors the coordinate systems guide, and enhances example retrieval logic with component name normalization and API page filtering. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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: 1
🤖 Fix all issues with AI agents
In `@www/src/docs/apiExamples/useAxisInverseDataSnapScale/DataSnapExample.tsx`:
- Line 139: The YAxis in DataSnapExample (YAxis domain={[0, 7000]}) is set far
below the mock data (sales up to ~50,000), causing the line to render off-chart;
update the YAxis domain in the DataSnapExample component to cover the data range
(e.g., domain={[0, 50000]} or a computed max) or reduce/mock the sales values to
match 0–7000, or add an inline comment explaining deliberate clipping if that
behavior is intended.
🧹 Nitpick comments (5)
omnidoc/readExamples.ts (1)
29-31: Minor inconsistency:includesvs strict equality for URL matching.Line 30 uses
ex.url.includes(...)to determinehasOwnApiPage, while line 38 usesex.url === ...for the explicit match. Since URLs follow a consistent/api/{name}/pattern,includeswon't cause false positives today, but switching to strict equality here would be more consistent and future-proof.Suggested change
- const hasOwnApiPage = Array.from(this.fileToExamples.values()).some(exList => - exList.some(ex => ex.url.includes(`/api/${cleanName}/`)), - ); + const hasOwnApiPage = Array.from(this.fileToExamples.values()).some(exList => + exList.some(ex => ex.url === `/api/${cleanName}/`), + );www/src/components/GuideView/CoordinateSystems/index.tsx (1)
235-245: Both examples share the samestackBlitzTitle— consider differentiating them.Lines 238 and 244 both use
"Recharts Scale Conversion Example". If a user opens both in StackBlitz, they'll have identical titles. Consider giving the second one a distinct title, e.g.,"Recharts Axis Tick Snap Example".Proposed diff
<CodeEditorWithPreview Component={AxisTickSnapExample} sourceCode={AxisTickSnapExampleSource} - stackBlitzTitle="Recharts Scale Conversion Example" + stackBlitzTitle="Recharts Axis Tick Snap Example" />www/src/docs/apiExamples/useYAxisTicks/index.ts (1)
2-3: Inconsistent file extension on raw import.Line 2 imports the component with
.tsxextension, but line 3 omits it for the?rawimport. Other files in this PR (e.g.,CoordinateSystems/index.tsxlines 11-12) consistently include.tsxbefore?raw. This may work depending on bundler config, but it's inconsistent.Proposed fix
-import AxisTickSnapExampleSource from '../useAxisInverseTickSnapScale/AxisTicksSnapExample?raw'; +import AxisTickSnapExampleSource from '../useAxisInverseTickSnapScale/AxisTicksSnapExample.tsx?raw';www/src/docs/apiExamples/useXAxisTicks/index.ts (1)
2-3: Same raw import extension inconsistency asuseYAxisTicks/index.ts.Line 2 uses
.tsx, line 3 omits it for the?rawimport.Proposed fix
-import AxisTickSnapExampleSource from '../useAxisInverseTickSnapScale/AxisTicksSnapExample?raw'; +import AxisTickSnapExampleSource from '../useAxisInverseTickSnapScale/AxisTicksSnapExample.tsx?raw';www/src/docs/apiExamples/useAxisInverseTickSnapScale/AxisTicksSnapExample.tsx (1)
1-149: Heavy duplication withDataSnapExample.tsx— consider extracting shared logic.This file is nearly identical to
DataSnapExample.tsx, differing only in the inverse scale hooks used (useXAxisInverseTickSnapScalevsuseXAxisInverseDataSnapScale) and component names. The mock data, event handlers, chart layout, and crosshair rendering pattern are all duplicated.Since these are documentation examples meant to be viewed as standalone source (via
?raw), full deduplication might hurt readability for the end user. But extracting the shared event handling and chart wrapper into a small shared module (leaving only the crosshair component unique) could reduce maintenance burden without hurting example clarity.
| > | ||
| <CartesianGrid strokeDasharray="3 3" /> | ||
| <XAxis dataKey="name" /> | ||
| <YAxis domain={[0, 7000]} /> |
There was a problem hiding this comment.
Y-axis domain [0, 7000] is far below the data range (max 50,000).
The mock data has sales values up to 50,000, but the Y-axis domain caps at 7,000. This will cause the line to extend well beyond the visible chart area. If this is intentional to demonstrate clipping behavior, consider adding a comment. Otherwise, adjust the domain or the data to keep the example visually coherent.
🤖 Prompt for AI Agents
In `@www/src/docs/apiExamples/useAxisInverseDataSnapScale/DataSnapExample.tsx` at
line 139, The YAxis in DataSnapExample (YAxis domain={[0, 7000]}) is set far
below the mock data (sales up to ~50,000), causing the line to render off-chart;
update the YAxis domain in the DataSnapExample component to cover the data range
(e.g., domain={[0, 50000]} or a computed max) or reduce/mock the sales values to
match 0–7000, or add an inline comment explaining deliberate clipping if that
behavior is intended.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #6979 +/- ##
==========================================
- Coverage 90.45% 90.10% -0.36%
==========================================
Files 517 522 +5
Lines 38618 38844 +226
Branches 5347 5347
==========================================
+ Hits 34933 35000 +67
- Misses 3676 3835 +159
Partials 9 9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Bundle ReportChanges will increase total bundle size by 280 bytes (0.01%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: recharts/bundle-es6Assets Changed:
view changes for bundle: recharts/bundle-cjsAssets Changed:
|
|
Staging Deployment Details
These deployments will remain available for 30 days. To update snapshots: Comment |
Summary by CodeRabbit
Release Notes
New Features
Documentation
Tests