-
Notifications
You must be signed in to change notification settings - Fork 11.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix ticks.minor and ticks.major configuration issues #6102
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for splitting up the PRs. It's much easier to review this way.
I noticed that mergeTicksOptions
is called in the time scale. I'm not sure why that is or why it's called both there and in buildOrUpdateControllers
. It doesn't seem like logic that should be specific to the time scale and can be removed.
Regarding |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nagix Looks great, anything else before merging?
No, this is ready to merge. |
This is a fork from #5961 separating the major/minor tick config issues from the rotation and label layout issues.
Problems
scale.ticks.*
options at runtime because the scale class copies the minor and major ticks options fromticks.*
on the initial chart creation, and it doesn’t update them on the following update calls as the values already exist ([BUG] Can't update scale.ticks.* options at runtime since 2.7.0 #4896).ticks.major.enabled
is set tofalse
by default, butticks.major
options are effective. The behavior should be consistent with theticks.major.enabled
setting.Solutions
mergeTicksOptions
and addsparseTickFontOptions
to parse minor and major tick font options. Omitted options are inherited fromticks
at runtime.ticks.major.enabled
isfalse
.Master: https://jsfiddle.net/nagix/76fLvg8u/
This PR: https://jsfiddle.net/nagix/4n5rd2gz/
Fixes #4896