From 9e8cf8e96a0f8d169af3c64ac67f0b5effac4e6a Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Fri, 8 Feb 2019 20:57:28 -0800 Subject: [PATCH 1/7] Ignore invalid log scale min and max --- src/scales/scale.logarithmic.js | 8 +++---- test/specs/scale.logarithmic.tests.js | 31 +++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/src/scales/scale.logarithmic.js b/src/scales/scale.logarithmic.js index 91f90845da3..5fc6287913f 100644 --- a/src/scales/scale.logarithmic.js +++ b/src/scales/scale.logarithmic.js @@ -174,8 +174,8 @@ module.exports = Scale.extend({ var DEFAULT_MIN = 1; var DEFAULT_MAX = 10; - me.min = valueOrDefault(tickOpts.min, me.min); - me.max = valueOrDefault(tickOpts.max, me.max); + me.min = tickOpts.min >= 0 ? tickOpts.min : me.min; + me.max = tickOpts.max >= 0 ? tickOpts.max : me.max; if (me.min === me.max) { if (me.min !== 0 && me.min !== null) { @@ -211,8 +211,8 @@ module.exports = Scale.extend({ var reverse = !me.isHorizontal(); var generationOptions = { - min: tickOpts.min, - max: tickOpts.max + min: tickOpts.min >= 0 ? tickOpts.min : undefined, + max: tickOpts.max >= 0 ? tickOpts.max : undefined }; var ticks = me.ticks = generateTicks(generationOptions, me); diff --git a/test/specs/scale.logarithmic.tests.js b/test/specs/scale.logarithmic.tests.js index cb44f108d36..a1f2dbcbfde 100644 --- a/test/specs/scale.logarithmic.tests.js +++ b/test/specs/scale.logarithmic.tests.js @@ -477,6 +477,37 @@ describe('Logarithmic Scale tests', function() { expect(yScale.ticks[tickCount - 1]).toBe(10); }); + it('should ignore invalid min and max options', function() { + var chart = window.acquireChart({ + type: 'bar', + data: { + datasets: [{ + data: [1, 1, 1, 2, 1, 0] + }], + labels: [] + }, + options: { + scales: { + yAxes: [{ + id: 'yScale', + type: 'logarithmic', + ticks: { + min: -10, + max: -1010, + callback: function(value) { + return value; + } + } + }] + } + } + }); + + var yScale = chart.scales.yScale; + expect(yScale.min).toBe(0); + expect(yScale.max).toBe(2); + }); + it('should generate tick marks', function() { var chart = window.acquireChart({ type: 'bar', From d9483252fc197a5a847120953b1eb9794b987b5e Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Tue, 12 Feb 2019 07:59:03 -0800 Subject: [PATCH 2/7] Check for additional invalid values --- src/scales/scale.logarithmic.js | 12 ++++++---- test/specs/scale.logarithmic.tests.js | 33 ++++++++++++++++++++++++++- 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/src/scales/scale.logarithmic.js b/src/scales/scale.logarithmic.js index 5fc6287913f..1396a1acd89 100644 --- a/src/scales/scale.logarithmic.js +++ b/src/scales/scale.logarithmic.js @@ -62,6 +62,10 @@ var defaultConfig = { } }; +function isNonNegative(value) { + return typeof(value) === 'number' && value >= 0; +} + module.exports = Scale.extend({ determineDataLimits: function() { var me = this; @@ -174,8 +178,8 @@ module.exports = Scale.extend({ var DEFAULT_MIN = 1; var DEFAULT_MAX = 10; - me.min = tickOpts.min >= 0 ? tickOpts.min : me.min; - me.max = tickOpts.max >= 0 ? tickOpts.max : me.max; + me.min = isNonNegative(tickOpts.min) ? tickOpts.min : me.min; + me.max = isNonNegative(tickOpts.max) ? tickOpts.max : me.max; if (me.min === me.max) { if (me.min !== 0 && me.min !== null) { @@ -211,8 +215,8 @@ module.exports = Scale.extend({ var reverse = !me.isHorizontal(); var generationOptions = { - min: tickOpts.min >= 0 ? tickOpts.min : undefined, - max: tickOpts.max >= 0 ? tickOpts.max : undefined + min: isNonNegative(tickOpts.min) ? tickOpts.min : undefined, + max: isNonNegative(tickOpts.max) ? tickOpts.max : undefined }; var ticks = me.ticks = generateTicks(generationOptions, me); diff --git a/test/specs/scale.logarithmic.tests.js b/test/specs/scale.logarithmic.tests.js index a1f2dbcbfde..dd7c7cce94a 100644 --- a/test/specs/scale.logarithmic.tests.js +++ b/test/specs/scale.logarithmic.tests.js @@ -477,7 +477,7 @@ describe('Logarithmic Scale tests', function() { expect(yScale.ticks[tickCount - 1]).toBe(10); }); - it('should ignore invalid min and max options', function() { + it('should ignore negative min and max options', function() { var chart = window.acquireChart({ type: 'bar', data: { @@ -508,6 +508,37 @@ describe('Logarithmic Scale tests', function() { expect(yScale.max).toBe(2); }); + it('should ignore invalid min and max options', function() { + var chart = window.acquireChart({ + type: 'bar', + data: { + datasets: [{ + data: [1, 1, 1, 2, 1, 0] + }], + labels: [] + }, + options: { + scales: { + yAxes: [{ + id: 'yScale', + type: 'logarithmic', + ticks: { + min: '', + max: false, + callback: function(value) { + return value; + } + } + }] + } + } + }); + + var yScale = chart.scales.yScale; + expect(yScale.min).toBe(0); + expect(yScale.max).toBe(2); + }); + it('should generate tick marks', function() { var chart = window.acquireChart({ type: 'bar', From 034e2221bd7de7e9f47dcf2985482d86fb1f6d85 Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Tue, 12 Feb 2019 08:45:03 -0800 Subject: [PATCH 3/7] Fix lint error --- src/scales/scale.logarithmic.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/scales/scale.logarithmic.js b/src/scales/scale.logarithmic.js index 1396a1acd89..0db177774a3 100644 --- a/src/scales/scale.logarithmic.js +++ b/src/scales/scale.logarithmic.js @@ -63,7 +63,7 @@ var defaultConfig = { }; function isNonNegative(value) { - return typeof(value) === 'number' && value >= 0; + return typeof value === 'number' && value >= 0; } module.exports = Scale.extend({ From 46bd3aab0d103e606ea1401f2fff0bcdb021e2f5 Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Wed, 13 Feb 2019 07:15:03 -0800 Subject: [PATCH 4/7] Add a TODO for v3 --- src/scales/scale.logarithmic.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/scales/scale.logarithmic.js b/src/scales/scale.logarithmic.js index 0db177774a3..3234c48996f 100644 --- a/src/scales/scale.logarithmic.js +++ b/src/scales/scale.logarithmic.js @@ -62,6 +62,7 @@ var defaultConfig = { } }; +// TODO(v3): change this to isPositive function isNonNegative(value) { return typeof value === 'number' && value >= 0; } From 46aa706237356bb2e57563c11a69d726987f2e98 Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Wed, 13 Feb 2019 08:35:43 -0800 Subject: [PATCH 5/7] Use helpers.isFinite --- src/scales/scale.logarithmic.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/scales/scale.logarithmic.js b/src/scales/scale.logarithmic.js index 3234c48996f..6911ed76062 100644 --- a/src/scales/scale.logarithmic.js +++ b/src/scales/scale.logarithmic.js @@ -64,7 +64,7 @@ var defaultConfig = { // TODO(v3): change this to isPositive function isNonNegative(value) { - return typeof value === 'number' && value >= 0; + return helpers.isFinite(value) && value >= 0; } module.exports = Scale.extend({ From 4fc7704a8f57f71debe73c00f009e78546ac9e65 Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Wed, 13 Feb 2019 16:05:44 -0800 Subject: [PATCH 6/7] nonNegativeOrDefault --- src/scales/scale.logarithmic.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/scales/scale.logarithmic.js b/src/scales/scale.logarithmic.js index 6911ed76062..a2fc3d7ee09 100644 --- a/src/scales/scale.logarithmic.js +++ b/src/scales/scale.logarithmic.js @@ -62,9 +62,9 @@ var defaultConfig = { } }; -// TODO(v3): change this to isPositive -function isNonNegative(value) { - return helpers.isFinite(value) && value >= 0; +// TODO(v3): change this to isPositiveOrDefault +function nonNegativeOrDefault(value, defaultValue) { + return helpers.isFinite(value) && value >= 0 ? value : defaultValue; } module.exports = Scale.extend({ @@ -179,8 +179,8 @@ module.exports = Scale.extend({ var DEFAULT_MIN = 1; var DEFAULT_MAX = 10; - me.min = isNonNegative(tickOpts.min) ? tickOpts.min : me.min; - me.max = isNonNegative(tickOpts.max) ? tickOpts.max : me.max; + me.min = nonNegativeOrDefault(tickOpts.min, me.min); + me.max = nonNegativeOrDefault(tickOpts.max, me.max); if (me.min === me.max) { if (me.min !== 0 && me.min !== null) { @@ -216,8 +216,8 @@ module.exports = Scale.extend({ var reverse = !me.isHorizontal(); var generationOptions = { - min: isNonNegative(tickOpts.min) ? tickOpts.min : undefined, - max: isNonNegative(tickOpts.max) ? tickOpts.max : undefined + min: nonNegativeOrDefault(tickOpts.min), + max: nonNegativeOrDefault(tickOpts.max) }; var ticks = me.ticks = generateTicks(generationOptions, me); From 8f15c80aae25cf748a2817f473169f65268cc8e5 Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Wed, 13 Feb 2019 17:43:39 -0800 Subject: [PATCH 7/7] fix comment --- src/scales/scale.logarithmic.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/scales/scale.logarithmic.js b/src/scales/scale.logarithmic.js index a2fc3d7ee09..f52bd38ce42 100644 --- a/src/scales/scale.logarithmic.js +++ b/src/scales/scale.logarithmic.js @@ -62,7 +62,7 @@ var defaultConfig = { } }; -// TODO(v3): change this to isPositiveOrDefault +// TODO(v3): change this to positiveOrDefault function nonNegativeOrDefault(value, defaultValue) { return helpers.isFinite(value) && value >= 0 ? value : defaultValue; }