Ticklabelindex fixes for issues #7875 and #7876#7877
Open
my-tien wants to merge 19 commits into
Open
Conversation
If ticklabelIndex was set, calcMinor was set to its value instead of being set to true. This didn't cause a bug but calcMinor is expected to be a boolean.
The previous name "text" is a little confusing since the return value is later used as a tick in calcTicks.
Also, don't just add one minor tick but as many as there should be between two major ticks. This is part of the fix for the last period label sometimes not being visible if ticklabelindex is negative. We need all minor ticks between the last visible tick and the major helper tick so that going back ticklabelindex steps from that helper tick lands on the correct minor tick.
Now uses an array "periodEndTicks" that contains the ending tick for each starting tick of a labeled period to determine label positioning.
Simplify code. labelIndex > 0 and labelIndex === 0 are actually treated in the same way.
- Fix description of lower plot. - Fix missing trailing newline.
There are only minimal pixel differences
This is just to be consistent with the way major ticks are set. In axes_test we test for empty string, instead of for undefined.
For period axes, a label is visible if one of the enclosing ticks is visible.
…ex is used. If tickformat isn't set, the old code just used the major dtick to determine the tickformat but when using ticklabelindex we need to look at the minor dtick. Only add additional ticks to the end if labels are drawn left of ticks.
my-tien
force-pushed
the
7875-7876-ticklabelindex-fixes
branch
from
1e5605e to
d42829b
Compare
Also reduce impact on charts that don't use ticklabelindex. This is not really necessary, but a precaution.
The editTitle function could previously select the editNode from a previous call if the blur event didn't remove it from the DOM in time.
Contributor
Author
|
Hi guys, I was able to fix almost all failing CI tests. However, I think that the failing |
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
1 participant
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.
Fixes #7876 by rewriting the way period label positioning works when using ticklabelindex. Previously, the next labeled tick was used as the period end for the previous tick. Now
positionPeriodTickscan accept an array ofperiodEndTicksinstead,Fixes #7875 by adding an additional major tick at the end of the axis range + all minor ticks between the last visible tick and that helper.
Previously, there was already code that added one major tick and one minor tick before tick0. Now the behavior is symmetrical to the axis range end (1 major tick and all minor ticks between that and tick0).
We need all minor ticks between the first/last visible tick and the major helper tick so that walking
ticklabelindexsteps from that helper tick lands on the correct minor tick.