fix(getNiceTickValues): remove trailing duplicate tick when allowDecimals=false#7482
Conversation
…mals=false getTickValuesFixedDomain appends cormax after the rangeStep values, then rounds all ticks to integers when allowDecimals is false. When cormax rounds down to the same integer as the last rangeStep value, the result contains a duplicate trailing tick (e.g. getTickValuesFixedDomain([0, 4.4], 5, false) returned [0, 2, 4, 4]). Fix: after rounding, check whether the last two values are equal and remove the trailing duplicate. The duplicate can only appear at the end because the step is an integer, so all interior rangeStep values differ by at least 1 after rounding. Also rename getTickValuesFixedDomain.test.ts → getTickValuesFixedDomain.spec.ts so vitest (configured to include test/**/*.spec.ts) actually picks up those tests.
|
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)
WalkthroughIn ChangesDuplicate tick deduplication in getTickValuesFixedDomain
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 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 |
Bundle ReportChanges will increase total bundle size by 2.51kB (0.05%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: recharts/bundle-treeshaking-sankeyAssets Changed:
view changes for bundle: recharts/bundle-treeshaking-polarAssets Changed:
view changes for bundle: recharts/bundle-umdAssets Changed:
view changes for bundle: recharts/bundle-treeshaking-cartesianAssets Changed:
view changes for bundle: recharts/bundle-es6Assets Changed:
view changes for bundle: recharts/bundle-treeshaking-sunburstAssets Changed:
view changes for bundle: recharts/bundle-cjsAssets Changed:
view changes for bundle: recharts/bundle-treeshaking-treemapAssets Changed:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7482 +/- ##
=======================================
Coverage 88.39% 88.40%
=======================================
Files 602 602
Lines 13911 13914 +3
Branches 3514 3515 +1
=======================================
+ Hits 12296 12300 +4
+ Misses 1432 1431 -1
Partials 183 183 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Problem
getTickValuesFixedDomainbuilds its tick array by callingrangeStepand then appendingcormaxat the end. WhenallowDecimalsisfalse, all values are rounded to integers. Ifcormaxrounds down to the same integer as the last value produced byrangeStep, the result contains a duplicate trailing tick.Example:
Root cause
rangeStep(0, 4.4, 2)produces[0, 2, 4]. Thencormax = 4.4is appended:[0, 2, 4, 4.4]. AfterMath.round:[0, 2, 4, 4]. The duplicate4is a result ofMath.round(4.4) === 4, matching the last step value.The duplicate can only occur at the trailing position because the step is always an integer when
allowDecimals = false, so all interiorrangeStepvalues differ by at least 1 after rounding.Fix
After rounding, check whether the last two elements are equal and remove the trailing one (
values.slice(0, last)). The fix is O(1) and preserves all other output properties including-0semantics.Also renames
getTickValuesFixedDomain.test.ts→getTickValuesFixedDomain.spec.tsso that vitest (configured withinclude: ['test/**/*.spec.ts?(x)']) actually picks up those existing tests.Verification
All 113
test/util/scale/tests pass, 962 unit tests pass overall.Summary by CodeRabbit
Bug Fixes
Tests