From aae05a08da0e216371caafa17228b3bff97d9fa9 Mon Sep 17 00:00:00 2001 From: Akihiko Kusanagi Date: Tue, 1 Jan 2019 21:38:09 +0800 Subject: [PATCH] Improve tick generation for linear scales (#5938) * Improve tick generation for linear scales * Simplify the tick generation code * Refactor getTickLimit --- src/scales/scale.linear.js | 17 +++--- src/scales/scale.linearbase.js | 71 ++++++++++++++++++-------- src/scales/scale.logarithmic.js | 4 -- src/scales/scale.radialLinear.js | 8 ++- test/specs/scale.linear.tests.js | 49 +++++++++++++++++- test/specs/scale.radialLinear.tests.js | 37 ++++++++++++++ 6 files changed, 145 insertions(+), 41 deletions(-) diff --git a/src/scales/scale.linear.js b/src/scales/scale.linear.js index a980615c5d1..60a291e626f 100644 --- a/src/scales/scale.linear.js +++ b/src/scales/scale.linear.js @@ -1,6 +1,5 @@ 'use strict'; -var defaults = require('../core/core.defaults'); var helpers = require('../helpers/index'); var scaleService = require('../core/core.scaleService'); var Ticks = require('../core/core.ticks'); @@ -133,20 +132,16 @@ module.exports = function(Chart) { // Common base implementation to handle ticks.min, ticks.max, ticks.beginAtZero this.handleTickRangeOptions(); }, - getTickLimit: function() { - var maxTicks; + // Returns the maximum number of ticks based on the scale dimension + _computeTickLimit: function() { var me = this; - var tickOpts = me.options.ticks; + var tickFont; if (me.isHorizontal()) { - maxTicks = Math.min(tickOpts.maxTicksLimit ? tickOpts.maxTicksLimit : 11, Math.ceil(me.width / 50)); - } else { - // The factor of 2 used to scale the font size has been experimentally determined. - var tickFontSize = helpers.valueOrDefault(tickOpts.fontSize, defaults.global.defaultFontSize); - maxTicks = Math.min(tickOpts.maxTicksLimit ? tickOpts.maxTicksLimit : 11, Math.ceil(me.height / (2 * tickFontSize))); + return Math.ceil(me.width / 40); } - - return maxTicks; + tickFont = helpers.options._parseFont(me.options.ticks); + return Math.ceil(me.height / tickFont.lineHeight); }, // Called after the ticks are built. We need handleDirectionalChanges: function() { diff --git a/src/scales/scale.linearbase.js b/src/scales/scale.linearbase.js index e2898e1ea38..8dfe15a08cb 100644 --- a/src/scales/scale.linearbase.js +++ b/src/scales/scale.linearbase.js @@ -16,35 +16,41 @@ function generateTicks(generationOptions, dataRange) { // for details. var stepSize = generationOptions.stepSize; + var unit = stepSize || 1; + var maxNumSpaces = generationOptions.maxTicks - 1; var min = generationOptions.min; var max = generationOptions.max; - var spacing, precision, factor, niceRange, niceMin, niceMax, numSpaces; - - if (stepSize && stepSize > 0) { - spacing = stepSize; - } else { - niceRange = helpers.niceNum(dataRange.max - dataRange.min, false); - spacing = helpers.niceNum(niceRange / (generationOptions.maxTicks - 1), true); - - precision = generationOptions.precision; - if (!helpers.isNullOrUndef(precision)) { - // If the user specified a precision, round to that number of decimal places - factor = Math.pow(10, precision); - spacing = Math.ceil(spacing * factor) / factor; - } + var precision = generationOptions.precision; + var spacing, factor, niceMin, niceMax, numSpaces; + + // spacing is set to a nice number of the dataRange divided by maxNumSpaces. + // stepSize is used as a minimum unit if it is specified. + spacing = helpers.niceNum((dataRange.max - dataRange.min) / maxNumSpaces / unit) * unit; + numSpaces = Math.ceil(dataRange.max / spacing) - Math.floor(dataRange.min / spacing); + if (numSpaces > maxNumSpaces) { + // If the calculated num of spaces exceeds maxNumSpaces, recalculate it + spacing = helpers.niceNum(numSpaces * spacing / maxNumSpaces / unit) * unit; } - // If a precision is not specified, calculate factor based on spacing - if (!factor) { + + if (stepSize || helpers.isNullOrUndef(precision)) { + // If a precision is not specified, calculate factor based on spacing factor = Math.pow(10, helpers.decimalPlaces(spacing)); + } else { + // If the user specified a precision, round to that number of decimal places + factor = Math.pow(10, precision); + spacing = Math.ceil(spacing * factor) / factor; } + niceMin = Math.floor(dataRange.min / spacing) * spacing; niceMax = Math.ceil(dataRange.max / spacing) * spacing; // If min, max and stepSize is set and they make an evenly spaced scale use it. - if (!helpers.isNullOrUndef(min) && !helpers.isNullOrUndef(max) && stepSize) { + if (stepSize) { // If very close to our whole number, use it. - if (helpers.almostWhole((max - min) / stepSize, spacing / 1000)) { + if (!helpers.isNullOrUndef(min) && helpers.almostWhole(min / spacing, spacing / 1000)) { niceMin = min; + } + if (!helpers.isNullOrUndef(max) && helpers.almostWhole(max / spacing, spacing / 1000)) { niceMax = max; } } @@ -146,7 +152,32 @@ module.exports = function(Chart) { } } }, - getTickLimit: noop, + + getTickLimit: function() { + var me = this; + var tickOpts = me.options.ticks; + var stepSize = tickOpts.stepSize; + var maxTicksLimit = tickOpts.maxTicksLimit; + var maxTicks; + + if (stepSize) { + maxTicks = Math.ceil(me.max / stepSize) - Math.floor(me.min / stepSize) + 1; + } else { + maxTicks = me._computeTickLimit(); + maxTicksLimit = maxTicksLimit || 11; + } + + if (maxTicksLimit) { + maxTicks = Math.min(maxTicksLimit, maxTicks); + } + + return maxTicks; + }, + + _computeTickLimit: function() { + return Number.POSITIVE_INFINITY; + }, + handleDirectionalChanges: noop, buildTicks: function() { @@ -155,7 +186,7 @@ module.exports = function(Chart) { var tickOpts = opts.ticks; // Figure out what the max number of ticks we can support it is based on the size of - // the axis area. For now, we say that the minimum tick spacing in pixels must be 50 + // the axis area. For now, we say that the minimum tick spacing in pixels must be 40 // We also limit the maximum number of ticks to 11 which gives a nice 10 squares on // the graph. Make sure we always have at least 2 ticks var maxTicks = me.getTickLimit(); diff --git a/src/scales/scale.logarithmic.js b/src/scales/scale.logarithmic.js index 0365568e772..240df0faf46 100644 --- a/src/scales/scale.logarithmic.js +++ b/src/scales/scale.logarithmic.js @@ -15,10 +15,6 @@ function generateTicks(generationOptions, dataRange) { var ticks = []; var valueOrDefault = helpers.valueOrDefault; - // Figure out what the max number of ticks we can support it is based on the size of - // the axis area. For now, we say that the minimum tick spacing in pixels must be 50 - // We also limit the maximum number of ticks to 11 which gives a nice 10 squares on - // the graph var tickVal = valueOrDefault(generationOptions.min, Math.pow(10, Math.floor(helpers.log10(dataRange.min)))); var endExp = Math.floor(helpers.log10(dataRange.max)); diff --git a/src/scales/scale.radialLinear.js b/src/scales/scale.radialLinear.js index f0c38317e8a..bc8129845ce 100644 --- a/src/scales/scale.radialLinear.js +++ b/src/scales/scale.radialLinear.js @@ -357,11 +357,9 @@ module.exports = function(Chart) { // Common base implementation to handle ticks.min, ticks.max, ticks.beginAtZero me.handleTickRangeOptions(); }, - getTickLimit: function() { - var opts = this.options; - var tickOpts = opts.ticks; - var tickBackdropHeight = getTickBackdropHeight(opts); - return Math.min(tickOpts.maxTicksLimit ? tickOpts.maxTicksLimit : 11, Math.ceil(this.drawingArea / tickBackdropHeight)); + // Returns the maximum number of ticks based on the scale dimension + _computeTickLimit: function() { + return Math.ceil(this.drawingArea / getTickBackdropHeight(this.options)); }, convertTicksToLabels: function() { var me = this; diff --git a/test/specs/scale.linear.tests.js b/test/specs/scale.linear.tests.js index ad35e2a4347..a186c6cc7fe 100644 --- a/test/specs/scale.linear.tests.js +++ b/test/specs/scale.linear.tests.js @@ -570,7 +570,7 @@ describe('Linear Scale', function() { expect(chart.scales.yScale0).not.toEqual(undefined); // must construct expect(chart.scales.yScale0.min).toBe(1); expect(chart.scales.yScale0.max).toBe(11); - expect(chart.scales.yScale0.ticks).toEqual(['11', '9', '7', '5', '3', '1']); + expect(chart.scales.yScale0.ticks).toEqual(['11', '10', '8', '6', '4', '2', '1']); }); it('Should create decimal steps if stepSize is a decimal number', function() { @@ -749,6 +749,53 @@ describe('Linear Scale', function() { expect(chart.scales.yScale0.ticks).toEqual(['0.06', '0.05', '0.04', '0.03', '0.02', '0.01', '0']); }); + it('Should correctly limit the maximum number of ticks', function() { + var chart = window.acquireChart({ + type: 'bar', + data: { + labels: ['a', 'b'], + datasets: [{ + data: [0.5, 2.5] + }] + }, + options: { + scales: { + yAxes: [{ + id: 'yScale' + }] + } + } + }); + + expect(chart.scales.yScale.ticks).toEqual(['2.5', '2.0', '1.5', '1.0', '0.5']); + + chart.options.scales.yAxes[0].ticks.maxTicksLimit = 11; + chart.update(); + + expect(chart.scales.yScale.ticks).toEqual(['2.5', '2.0', '1.5', '1.0', '0.5']); + + chart.options.scales.yAxes[0].ticks.maxTicksLimit = 21; + chart.update(); + + expect(chart.scales.yScale.ticks).toEqual([ + '2.5', '2.4', '2.3', '2.2', '2.1', '2.0', '1.9', '1.8', '1.7', '1.6', + '1.5', '1.4', '1.3', '1.2', '1.1', '1.0', '0.9', '0.8', '0.7', '0.6', + '0.5' + ]); + + chart.options.scales.yAxes[0].ticks.maxTicksLimit = 11; + chart.options.scales.yAxes[0].ticks.stepSize = 0.01; + chart.update(); + + expect(chart.scales.yScale.ticks).toEqual(['2.5', '2.0', '1.5', '1.0', '0.5']); + + chart.options.scales.yAxes[0].ticks.min = 0.3; + chart.options.scales.yAxes[0].ticks.max = 2.8; + chart.update(); + + expect(chart.scales.yScale.ticks).toEqual(['2.8', '2.5', '2.0', '1.5', '1.0', '0.5', '0.3']); + }); + it('Should build labels using the user supplied callback', function() { var chart = window.acquireChart({ type: 'bar', diff --git a/test/specs/scale.radialLinear.tests.js b/test/specs/scale.radialLinear.tests.js index 49ef34b46d6..2102bfaefc9 100644 --- a/test/specs/scale.radialLinear.tests.js +++ b/test/specs/scale.radialLinear.tests.js @@ -282,6 +282,43 @@ describe('Test the radial linear scale', function() { expect(chart.scale.end).toBe(0); }); + it('Should correctly limit the maximum number of ticks', function() { + var chart = window.acquireChart({ + type: 'radar', + data: { + labels: ['label1', 'label2', 'label3'], + datasets: [{ + data: [0.5, 1.5, 2.5] + }] + }, + options: { + scale: { + pointLabels: { + display: false + } + } + } + }); + + expect(chart.scale.ticks).toEqual(['0.5', '1.0', '1.5', '2.0', '2.5']); + + chart.options.scale.ticks.maxTicksLimit = 11; + chart.update(); + + expect(chart.scale.ticks).toEqual(['0.5', '1.0', '1.5', '2.0', '2.5']); + + chart.options.scale.ticks.stepSize = 0.01; + chart.update(); + + expect(chart.scale.ticks).toEqual(['0.5', '1.0', '1.5', '2.0', '2.5']); + + chart.options.scale.ticks.min = 0.3; + chart.options.scale.ticks.max = 2.8; + chart.update(); + + expect(chart.scale.ticks).toEqual(['0.3', '0.5', '1.0', '1.5', '2.0', '2.5', '2.8']); + }); + it('Should build labels using the user supplied callback', function() { var chart = window.acquireChart({ type: 'radar',