Skip to content
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

[BUG] maxTicksLimit doesn't limit the number of ticks #5917

Closed
nagix opened this issue Dec 16, 2018 · 0 comments · Fixed by #5922
Closed

[BUG] maxTicksLimit doesn't limit the number of ticks #5917

nagix opened this issue Dec 16, 2018 · 0 comments · Fixed by #5922
Milestone

Comments

@nagix
Copy link
Contributor

nagix commented Dec 16, 2018

Expected Behavior

maxTicksLimit should limit the number of ticks.

Current Behavior

maxTicksLimit doesn't limit the number of ticks. See https://jsfiddle.net/nagix/x68L9dw3/.

screen shot 2018-12-16 at 3 45 10 pm

Possible Solution

spacing = helpers.niceNum(niceRange / (generationOptions.maxTicks - 1), true);
sets the second argument to true, and rounding the first argument in
helpers.niceNum = function(range, round) {
var exponent = Math.floor(helpers.log10(range));
var fraction = range / Math.pow(10, exponent);
var niceFraction;
if (round) {
if (fraction < 1.5) {
niceFraction = 1;
} else if (fraction < 3) {
niceFraction = 2;
} else if (fraction < 7) {
niceFraction = 5;
} else {
niceFraction = 10;
}
causes the problem.

However, when setting the second argument to false, multiple tests failed as the generated ticks were changed. It seems to me that a whole logic for ticks generation needs to be improved because results are not consistent even if the number of ticks is below maxTicksLimit.

screen shot 2018-12-16 at 4 10 58 pm

In particular, I don't understand why niceRange is used as the base for calculation at the first place.

niceRange = helpers.niceNum(dataRange.max - dataRange.min, false);

Context

The same logic is used for a radialLinear scale, and this causes overlaps between tick labels in #5914.

Environment

  • Chart.js version: 2.4.0 and later
  • Browser name and version: All
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants