From f5d9892ad886233f279d5f5842f9bb00e9dd07c5 Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Mon, 3 Feb 2020 04:36:31 -0800 Subject: [PATCH] Improved formatting of numeric scale labels (#7007) * Improved formatting of numeric scale labels * Put locale on options * Use scientific notation for big ticks * Remove extra parameter --- docs/getting-started/v3-migration.md | 1 + src/core/core.ticks.js | 53 +++++++++++--------------- src/scales/scale.linear.js | 2 +- src/scales/scale.linearbase.js | 2 +- src/scales/scale.logarithmic.js | 4 +- src/scales/scale.radialLinear.js | 2 +- test/specs/core.ticks.tests.js | 3 +- test/specs/scale.linear.tests.js | 4 +- test/specs/scale.logarithmic.tests.js | 2 +- test/specs/scale.radialLinear.tests.js | 2 +- test/utils.js | 1 + 11 files changed, 35 insertions(+), 41 deletions(-) diff --git a/docs/getting-started/v3-migration.md b/docs/getting-started/v3-migration.md index fb36be459..97728b1c2 100644 --- a/docs/getting-started/v3-migration.md +++ b/docs/getting-started/v3-migration.md @@ -13,6 +13,7 @@ Chart.js 3.0 introduces a number of breaking changes. Chart.js 2.0 was released * `options.ticks.userCallback` was renamed to `options.ticks.callback` * `options.ticks.major` and `options.ticks.minor` were replaced with scriptable options for tick fonts. +* `Chart.Ticks.formatters.linear` and `Chart.Ticks.formatters.logarithmic` were replaced with `Chart.Ticks.formatters.numeric`. ### Tooltip diff --git a/src/core/core.ticks.js b/src/core/core.ticks.js index ae0180736..1e4fa0711 100644 --- a/src/core/core.ticks.js +++ b/src/core/core.ticks.js @@ -24,49 +24,42 @@ export default { }, /** - * Formatter for linear numeric ticks - * @method Chart.Ticks.formatters.linear + * Formatter for numeric ticks + * @method Chart.Ticks.formatters.numeric * @param tickValue {number} the value to be formatted * @param index {number} the position of the tickValue parameter in the ticks array * @param ticks {object[]} the list of ticks being converted * @return {string} string representation of the tickValue parameter */ - linear: function(tickValue, index, ticks) { + numeric: function(tickValue, index, ticks) { + if (tickValue === 0) { + return '0'; // never show decimal places for 0 + } + // If we have lots of ticks, don't use the ones - var delta = ticks.length > 3 ? ticks[2].value - ticks[1].value : ticks[1].value - ticks[0].value; + let delta = ticks.length > 3 ? ticks[2].value - ticks[1].value : ticks[1].value - ticks[0].value; // If we have a number like 2.5 as the delta, figure out how many decimal places we need - if (Math.abs(delta) > 1) { - if (tickValue !== Math.floor(tickValue)) { - // not an integer - delta = tickValue - Math.floor(tickValue); - } + if (Math.abs(delta) > 1 && tickValue !== Math.floor(tickValue)) { + // not an integer + delta = tickValue - Math.floor(tickValue); } - var logDelta = log10(Math.abs(delta)); - var tickString = ''; + const logDelta = log10(Math.abs(delta)); - if (tickValue !== 0) { - var maxTick = Math.max(Math.abs(ticks[0].value), Math.abs(ticks[ticks.length - 1].value)); - if (maxTick < 1e-4) { // all ticks are small numbers; use scientific notation - var logTick = log10(Math.abs(tickValue)); - var numExponential = Math.floor(logTick) - Math.floor(logDelta); - numExponential = Math.max(Math.min(numExponential, 20), 0); - tickString = tickValue.toExponential(numExponential); - } else { - var numDecimal = -1 * Math.floor(logDelta); - numDecimal = Math.max(Math.min(numDecimal, 20), 0); // toFixed has a max of 20 decimal places - tickString = tickValue.toFixed(numDecimal); - } - } else { - tickString = '0'; // never show decimal places for 0 + const maxTick = Math.max(Math.abs(ticks[0].value), Math.abs(ticks[ticks.length - 1].value)); + const minTick = Math.min(Math.abs(ticks[0].value), Math.abs(ticks[ticks.length - 1].value)); + const locale = this.chart.options.locale; + if (maxTick < 1e-4 || minTick > 1e+7) { // all ticks are small or big numbers; use scientific notation + const logTick = log10(Math.abs(tickValue)); + let numExponential = Math.floor(logTick) - Math.floor(logDelta); + numExponential = Math.max(Math.min(numExponential, 20), 0); + return new Intl.NumberFormat(locale, {notation: 'scientific', minimumFractionDigits: numExponential, maximumFractionDigits: numExponential}).format(tickValue); } - return tickString; - }, - - logarithmic: function(tickValue) { - return tickValue === 0 ? '0' : tickValue.toExponential(); + let numDecimal = -1 * Math.floor(logDelta); + numDecimal = Math.max(Math.min(numDecimal, 20), 0); // toFixed has a max of 20 decimal places + return new Intl.NumberFormat(locale, {minimumFractionDigits: numDecimal, maximumFractionDigits: numDecimal}).format(tickValue); } } }; diff --git a/src/scales/scale.linear.js b/src/scales/scale.linear.js index 27a67a76f..189d20962 100644 --- a/src/scales/scale.linear.js +++ b/src/scales/scale.linear.js @@ -7,7 +7,7 @@ import Ticks from '../core/core.ticks'; const defaultConfig = { ticks: { - callback: Ticks.formatters.linear + callback: Ticks.formatters.numeric } }; diff --git a/src/scales/scale.linearbase.js b/src/scales/scale.linearbase.js index f0d555adb..0932642dd 100644 --- a/src/scales/scale.linearbase.js +++ b/src/scales/scale.linearbase.js @@ -270,7 +270,7 @@ class LinearScaleBase extends Scale { } getLabelForValue(value) { - return new Intl.NumberFormat().format(value); + return new Intl.NumberFormat(this.options.locale).format(value); } } diff --git a/src/scales/scale.logarithmic.js b/src/scales/scale.logarithmic.js index 3df308276..33de08c9b 100644 --- a/src/scales/scale.logarithmic.js +++ b/src/scales/scale.logarithmic.js @@ -52,7 +52,7 @@ function generateTicks(generationOptions, dataRange) { const defaultConfig = { // label settings ticks: { - callback: Ticks.formatters.logarithmic, + callback: Ticks.formatters.numeric, major: { enabled: true } @@ -136,7 +136,7 @@ class LogarithmicScale extends Scale { } getLabelForValue(value) { - return value === undefined ? 0 : new Intl.NumberFormat().format(value); + return value === undefined ? 0 : new Intl.NumberFormat(this.options.locale).format(value); } getPixelForTick(index) { diff --git a/src/scales/scale.radialLinear.js b/src/scales/scale.radialLinear.js index 657d07f16..03d2c4163 100644 --- a/src/scales/scale.radialLinear.js +++ b/src/scales/scale.radialLinear.js @@ -44,7 +44,7 @@ const defaultConfig = { // Number - The backdrop padding to the side of the label in pixels backdropPaddingX: 2, - callback: Ticks.formatters.linear + callback: Ticks.formatters.numeric }, pointLabels: { diff --git a/test/specs/core.ticks.tests.js b/test/specs/core.ticks.tests.js index 26b260d0c..1352ba860 100644 --- a/test/specs/core.ticks.tests.js +++ b/test/specs/core.ticks.tests.js @@ -8,8 +8,7 @@ describe('Test tick generators', function() { expect(typeof Chart.Ticks).toBeDefined(); expect(typeof Chart.Ticks.formatters).toBeDefined(); expect(typeof Chart.Ticks.formatters.values).toBe('function'); - expect(typeof Chart.Ticks.formatters.linear).toBe('function'); - expect(typeof Chart.Ticks.formatters.logarithmic).toBe('function'); + expect(typeof Chart.Ticks.formatters.numeric).toBe('function'); }); it('Should generate linear spaced ticks with correct precision', function() { diff --git a/test/specs/scale.linear.tests.js b/test/specs/scale.linear.tests.js index 073aa31dc..4f4c31dd9 100644 --- a/test/specs/scale.linear.tests.js +++ b/test/specs/scale.linear.tests.js @@ -541,8 +541,8 @@ describe('Linear Scale', function() { expect(chart.scales.y.min).toBe(-1010); expect(chart.scales.y.max).toBe(1010); var labels = getLabels(chart.scales.y); - expect(labels[0]).toBe('1010'); - expect(labels[labels.length - 1]).toBe('-1010'); + expect(labels[0]).toBe('1,010'); + expect(labels[labels.length - 1]).toBe('-1,010'); }); it('Should use min, max and stepSize to create fixed spaced ticks', function() { diff --git a/test/specs/scale.logarithmic.tests.js b/test/specs/scale.logarithmic.tests.js index 0cab2eef7..15c3065f0 100644 --- a/test/specs/scale.logarithmic.tests.js +++ b/test/specs/scale.logarithmic.tests.js @@ -679,7 +679,7 @@ describe('Logarithmic Scale tests', function() { } }); - 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']); + expect(getLabels(chart.scales.y)).toEqual(['80', '70', '60', '50', '40', '30', '20', '10', '9', '8', '7', '6', '5', '4', '3', '2', '1']); }); it('should build labels using the user supplied callback', function() { diff --git a/test/specs/scale.radialLinear.tests.js b/test/specs/scale.radialLinear.tests.js index 576d0b4dc..4958cfb0d 100644 --- a/test/specs/scale.radialLinear.tests.js +++ b/test/specs/scale.radialLinear.tests.js @@ -214,7 +214,7 @@ describe('Test the radial linear scale', function() { expect(chart.scales.r.min).toBe(-1010); expect(chart.scales.r.max).toBe(1010); - expect(getLabels(chart.scales.r)).toEqual(['-1010', '-1000', '-500', '0', '500', '1000', '1010']); + expect(getLabels(chart.scales.r)).toEqual(['-1,010', '-1,000', '-500', '0', '500', '1,000', '1,010']); }); it('should forcibly include 0 in the range if the beginAtZero option is used', function() { diff --git a/test/utils.js b/test/utils.js index daae1626c..0a376f7ea 100644 --- a/test/utils.js +++ b/test/utils.js @@ -56,6 +56,7 @@ function acquireChart(config, options) { config.options.animation = config.options.animation === undefined ? false : config.options.animation; config.options.responsive = config.options.responsive === undefined ? false : config.options.responsive; config.options.fontFamily = config.options.fontFamily || 'Arial'; + config.options.locale = config.options.locale || 'en-US'; wrapper.appendChild(canvas); window.document.body.appendChild(wrapper); -- 2.47.2