Add test for ticks spacing#7082
Merged
PavelVanecek merged 2 commits intorecharts:mainfrom Mar 7, 2026
Merged
Conversation
When XAxis had type='number' in a horizontal layout, ticks were placed at data positions instead of being evenly spaced. This happened because the categoricalDomain branch in combineAxisTicks and getTicksOfAxis was used even for number-type axes. Fixed by skipping the categoricalDomain branch when type='number' and niceTicks are available, ensuring ticks fall through to evenly-spaced tick generation via niceTicks. Added regression test with clustered data (1, 2, 3, 100) to verify ticks are evenly spaced at (0, 25, 50, 75, 100) instead of at data positions.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughUpdated comments/guard notes in axis tick selection logic and added a test ensuring XAxis with numeric domain produces evenly spaced ticks; no runtime behavior changes in selectors or public APIs except the new test. Changes
Sequence Diagram(s)(Skipped — changes are comment/test additions and do not introduce new multi-component control flow.) Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/cartesian/XAxis/XAxis.numberdomain.spec.tsx`:
- Around line 1421-1437: The new test rendering clusteredData must use fake
timers to avoid rAF/timer flakiness: before calling render(...) call
vi.useFakeTimers(), and immediately after the render (after the JSX with
<BarChart ...>, <XAxis ...>, and <Customized component={<ExpectAxisDomain .../>}
/>) invoke vi.runOnlyPendingTimers() to flush timers; ensure you restore timers
at the end of the test if needed. This targets the spec surrounding render,
<BarChart>, <XAxis>, and ExpectAxisDomain assertions.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/state/selectors/axisSelectors.tssrc/util/ChartUtils.tstest/cartesian/XAxis/XAxis.numberdomain.spec.tsx
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7082 +/- ##
==========================================
- Coverage 89.80% 89.60% -0.20%
==========================================
Files 529 534 +5
Lines 39911 40250 +339
Branches 5447 5477 +30
==========================================
+ Hits 35843 36068 +225
- Misses 4059 4174 +115
+ Partials 9 8 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Bundle ReportChanges will increase total bundle size by 361 bytes (0.01%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: recharts/bundle-cjsAssets Changed:
|
PavelVanecek
reviewed
Mar 7, 2026
| }); | ||
| }); | ||
|
|
||
| describe('XAxis ticks should be evenly spaced with type=number and clustered data', () => { |
Collaborator
There was a problem hiding this comment.
This test is passing even without the code change.
Collaborator
There was a problem hiding this comment.
I believe #7042 fixed this upstream, before it reached the getAxisTicks function.
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
2 participants
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.
Description
When XAxis has
type='number'in a horizontal layout, ticks were placed at actual data positions instead of being evenly spaced across the axis range. For example, with data values[1, 2, 3, 100], ticks would cluster at positions 1, 2, 3, and 100 instead of distributing evenly at 0, 25, 50, 75, 100.Root cause: In combineAxisTicks (axisSelectors.ts) and getTicksOfAxis (ChartUtils.ts), the
categoricalDomainbranch was always used for axes where isCategoricalAxis() returns true — which includes XAxis in horizontal layout. This branch places ticks at data values rather than using evenly-spacedniceTicks.Fix: Added a guard
!(type === 'number' && niceTicks)to thecategoricalDomainbranch in both functions. Whentype='number'andniceTicksare available (i.e., the scale is linear), the categoricalDomain branch is skipped, allowing tick generation to use the evenly-spacedniceTicksinstead. This preserves correct behavior for:scale="time"(niceTicks is undefined, so categoricalDomain is still used)Related Issue
Fixes #4271
Motivation and Context
Users setting
type='number'on XAxis expect evenly-spaced ticks (like YAxis, which defaults totype='number'). Instead, ticks followed data positions, making charts with clustered data look incorrect. Theinterval='equidistantStartEnd'workaround mentioned in the issue also didn't exist (correct values areequidistantPreserveStart/equidistantPreserveEnd, which control tick filtering, not spacing).How Has This Been Tested?
test/routes/index.spec.tsxalso present on main)[1, 2, 3, 100]verifying ticks render at[0, 25, 50, 75, 100]with even pixel spacingTypes of changes
Checklist:
Summary by CodeRabbit
Bug Fixes
Tests