fix(scale): detect even major-tick spacing when first major is at index 0 - #12287
Open
contactjawad wants to merge 1 commit into
Open
fix(scale): detect even major-tick spacing when first major is at index 0#12287contactjawad wants to merge 1 commit into
contactjawad wants to merge 1 commit into
Conversation
…ex 0 getEvenSpacing seeded the reference gap with arr[0] (an absolute index) and started comparing at i=1, so any evenly-spaced set of major ticks whose first index differs from the spacing (e.g. majors at 0, 20, 40) was wrongly reported as unevenly spaced. Seed the gap with the first actual difference (arr[1] - arr[0]) and start at i=2.
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
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.
What
getEvenSpacing(used by autoSkip to place minor ticks so they divide evenly spaced major ticks into even chunks) failed to recognize an evenly spaced set of major-tick indices whenever the first major index differs from the spacing value — most notably when the first major tick is at index 0.Why
The reference gap was seeded with
arr[0](an absolute index) while the loop body compares consecutive index differences:For majors at indices
[0, 20, 40],diffstarts as0, so the very first comparisonarr[1] - arr[0] (20) !== 0returnsfalse, incorrectly reporting the majors as unevenly spaced. autoSkip then falls back to rawticks.length / ticksLimitspacing instead of a factor of the major spacing.How
Seed the gap with the first actual difference and begin comparing at the second gap:
Sets that genuinely start at their spacing (e.g.
[20, 40, 60]) are unchanged, non-even sets still returnfalse, and even sets that start at index 0 are now detected correctly.Test
Added a spec in
test/specs/scale.time.tests.jsusing an hourly time axis that starts on a day boundary, so day-start major ticks fall at indices0, 24, 48, .... Withmajor.enabledandautoSkip, the kept minor ticks now divide the 24h major interval into even chunks (8h spacing, 18 ticks). Before the fix the even spacing was not detected and the raw ratio was used (6h spacing, 24 ticks); the test asserts the 8h spacing and fails on the previous behavior.