From fc158eea8977fe2066fad71828a0f84c3c3d641d Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Fri, 20 Dec 2019 18:14:21 -0800 Subject: [PATCH] Fix log scale autoSkip issues (#6853) --- src/core/core.scale.js | 2 +- src/core/core.ticks.js | 11 +--- src/scales/scale.logarithmic.js | 95 +++++++++++++-------------- test/specs/scale.logarithmic.tests.js | 10 +-- 4 files changed, 55 insertions(+), 63 deletions(-) diff --git a/src/core/core.scale.js b/src/core/core.scale.js index 49432e228..164ee564a 100644 --- a/src/core/core.scale.js +++ b/src/core/core.scale.js @@ -911,7 +911,7 @@ class Scale extends Element { if (numMajorIndices > 0) { let i, ilen; - const avgMajorSpacing = numMajorIndices > 1 ? (last - first) / (numMajorIndices - 1) : null; + const avgMajorSpacing = numMajorIndices > 1 ? Math.round((last - first) / (numMajorIndices - 1)) : null; skip(ticks, newTicks, spacing, helpers.isNullOrUndef(avgMajorSpacing) ? 0 : first - avgMajorSpacing, first); for (i = 0, ilen = numMajorIndices - 1; i < ilen; i++) { skip(ticks, newTicks, spacing, majorIndices[i], majorIndices[i + 1]); diff --git a/src/core/core.ticks.js b/src/core/core.ticks.js index 3e7a39702..4d8b5f27f 100644 --- a/src/core/core.ticks.js +++ b/src/core/core.ticks.js @@ -65,15 +65,8 @@ module.exports = { return tickString; }, - logarithmic: function(tickValue, index, ticks) { - var remain = tickValue / (Math.pow(10, Math.floor(math.log10(tickValue)))); - - if (tickValue === 0) { - return '0'; - } else if (remain === 1 || remain === 2 || remain === 5 || index === 0 || index === ticks.length - 1) { - return tickValue.toExponential(); - } - return ''; + logarithmic: function(tickValue) { + return tickValue === 0 ? '0' : tickValue.toExponential(); } } }; diff --git a/src/scales/scale.logarithmic.js b/src/scales/scale.logarithmic.js index f42187396..9811dab4c 100644 --- a/src/scales/scale.logarithmic.js +++ b/src/scales/scale.logarithmic.js @@ -10,6 +10,11 @@ import Ticks from '../core/core.ticks'; const valueOrDefault = helpers.valueOrDefault; const log10 = helpers.math.log10; +function isMajor(tickVal) { + const remain = tickVal / (Math.pow(10, Math.floor(log10(tickVal)))); + return remain === 1; +} + /** * Generate a set of logarithmic ticks * @param generationOptions the options used to generate the ticks @@ -17,28 +22,23 @@ const log10 = helpers.math.log10; * @returns {number[]} array of tick values */ function generateTicks(generationOptions, dataRange) { - var ticks = []; - - var tickVal = valueOrDefault(generationOptions.min, Math.pow(10, Math.floor(log10(dataRange.min)))); - - var endExp = Math.floor(log10(dataRange.max)); - var endSignificand = Math.ceil(dataRange.max / Math.pow(10, endExp)); - var exp, significand; + const endExp = Math.floor(log10(dataRange.max)); + const endSignificand = Math.ceil(dataRange.max / Math.pow(10, endExp)); + const ticks = []; + let tickVal = valueOrDefault(generationOptions.min, Math.pow(10, Math.floor(log10(dataRange.min)))); + let exp, significand; if (tickVal === 0) { - exp = Math.floor(log10(dataRange.minNotZero)); - significand = Math.floor(dataRange.minNotZero / Math.pow(10, exp)); - - ticks.push({value: tickVal}); - tickVal = significand * Math.pow(10, exp); + exp = 0; + significand = 0; } else { exp = Math.floor(log10(tickVal)); significand = Math.floor(tickVal / Math.pow(10, exp)); } - var precision = exp < 0 ? Math.pow(10, Math.abs(exp)) : 1; + let precision = exp < 0 ? Math.pow(10, Math.abs(exp)) : 1; do { - ticks.push({value: tickVal}); + ticks.push({value: tickVal, major: isMajor(tickVal)}); ++significand; if (significand === 10) { @@ -50,8 +50,8 @@ function generateTicks(generationOptions, dataRange) { tickVal = Math.round(significand * Math.pow(10, exp) * precision) / precision; } while (exp < endExp || (exp === endExp && significand < endSignificand)); - var lastTick = valueOrDefault(generationOptions.max, tickVal); - ticks.push({value: lastTick}); + const lastTick = valueOrDefault(generationOptions.max, tickVal); + ticks.push({value: lastTick, major: isMajor(tickVal)}); return ticks; } @@ -59,7 +59,10 @@ function generateTicks(generationOptions, dataRange) { const defaultConfig = { // label settings ticks: { - callback: Ticks.formatters.logarithmic + callback: Ticks.formatters.logarithmic, + major: { + enabled: true + } } }; @@ -70,11 +73,11 @@ class LogarithmicScale extends Scale { } determineDataLimits() { - var me = this; - var minmax = me._getMinMax(true); - var min = minmax.min; - var max = minmax.max; - var minPositive = minmax.minPositive; + const me = this; + const minmax = me._getMinMax(true); + const min = minmax.min; + const max = minmax.max; + const minPositive = minmax.minPositive; me.min = helpers.isFinite(min) ? Math.max(0, min) : null; me.max = helpers.isFinite(max) ? Math.max(0, max) : null; @@ -84,11 +87,11 @@ class LogarithmicScale extends Scale { } handleTickRangeOptions() { - var me = this; - var DEFAULT_MIN = 1; - var DEFAULT_MAX = 10; - var min = me.min; - var max = me.max; + const me = this; + const DEFAULT_MIN = 1; + const DEFAULT_MAX = 10; + let min = me.min; + let max = me.max; if (min === max) { if (min !== 0 && min !== null) { @@ -121,15 +124,15 @@ class LogarithmicScale extends Scale { } buildTicks() { - var me = this; - var opts = me.options; - var reverse = !me.isHorizontal(); + const me = this; + const opts = me.options; - var generationOptions = { + const generationOptions = { min: me._userMin, max: me._userMax }; - var ticks = generateTicks(generationOptions, me); + const ticks = generateTicks(generationOptions, me); + let reverse = !me.isHorizontal(); // At this point, we need to update our max and min given the tick values since we have expanded the // range of the scale @@ -149,18 +152,12 @@ class LogarithmicScale extends Scale { return ticks; } - generateTickLabels(ticks) { - this._tickValues = ticks.map(t => t.value); - - return Scale.prototype.generateTickLabels.call(this, ticks); - } - getPixelForTick(index) { - var ticks = this._tickValues; + const ticks = this.ticks; if (index < 0 || index > ticks.length - 1) { return null; } - return this.getPixelForValue(ticks[index]); + return this.getPixelForValue(ticks[index].value); } /** @@ -170,16 +167,16 @@ class LogarithmicScale extends Scale { * @private */ _getFirstTickValue(value) { - var exp = Math.floor(log10(value)); - var significand = Math.floor(value / Math.pow(10, exp)); + const exp = Math.floor(log10(value)); + const significand = Math.floor(value / Math.pow(10, exp)); return significand * Math.pow(10, exp); } _configure() { - var me = this; - var start = me.min; - var offset = 0; + const me = this; + let start = me.min; + let offset = 0; Scale.prototype._configure.call(me); @@ -194,8 +191,8 @@ class LogarithmicScale extends Scale { } getPixelForValue(value) { - var me = this; - var decimal = 0; + const me = this; + let decimal = 0; if (value > me.min && value > 0) { decimal = (log10(value) - me._startValue) / me._valueRange + me._valueOffset; @@ -204,8 +201,8 @@ class LogarithmicScale extends Scale { } getValueForPixel(pixel) { - var me = this; - var decimal = me.getDecimalForPixel(pixel); + const me = this; + const decimal = me.getDecimalForPixel(pixel); return decimal === 0 && me.min === 0 ? 0 : Math.pow(10, me._startValue + (decimal - me._valueOffset) * me._valueRange); diff --git a/test/specs/scale.logarithmic.tests.js b/test/specs/scale.logarithmic.tests.js index 23e41d513..b2233e82b 100644 --- a/test/specs/scale.logarithmic.tests.js +++ b/test/specs/scale.logarithmic.tests.js @@ -40,7 +40,9 @@ describe('Logarithmic Scale tests', function() { autoSkipPadding: 0, labelOffset: 0, minor: {}, - major: {}, + major: { + enabled: true + }, }, }); @@ -591,7 +593,7 @@ describe('Logarithmic Scale tests', function() { var scale = chart.scales.y; // Counts down because the lines are drawn top to bottom - expect(getLabels(scale)).toEqual([30, 20, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0.9, 0.8, 0]); + expect(getLabels(scale)).toEqual([30, 20, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0]); expect(scale.start).toEqual(0); expect(scale.end).toEqual(30); }); @@ -652,7 +654,7 @@ describe('Logarithmic Scale tests', function() { }); var scale = chart.scales.y; - expect(getLabels(scale)).toEqual([0, 9, 10, 20, 30]); + expect(getLabels(scale)).toEqual([0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 20, 30]); expect(scale.start).toEqual(30); expect(scale.end).toEqual(0); }); @@ -675,7 +677,7 @@ describe('Logarithmic Scale tests', function() { } }); - expect(getLabels(chart.scales.y)).toEqual(['8e+1', '', '', '5e+1', '', '', '2e+1', '1e+1', '', '', '', '', '5e+0', '', '', '2e+0', '1e+0', '0']); + expect(getLabels(chart.scales.y)).toEqual(['8e+1', '7e+1', '6e+1', '5e+1', '4e+1', '3e+1', '2e+1', '1e+1', '9e+0', '8e+0', '7e+0', '6e+0', '5e+0', '4e+0', '3e+0', '2e+0', '1e+0', '0']); }); it('should build labels using the user supplied callback', function() { -- 2.47.2