Review comment + stop comparing nulls

chartjs · Nov 6, 2019 · 1880a7f · 1880a7f
1 parent 73ea927
commit 1880a7f
Show file tree

Hide file tree

Showing 5 changed files with 24 additions and 10 deletions.
diff --git a/src/core/core.controller.js b/src/core/core.controller.js
@@ -383,8 +383,8 @@ helpers.extend(Chart.prototype, /** @lends Chart */ {
 			}
 
 			// parse min/max value, so we can properly determine min/max for other scales
-			scale._parsedMin = scale._parse(scale.options.ticks.min);
-			scale._parsedMax = scale._parse(scale.options.ticks.max);
+			scale._userMin = scale._parse(scale.options.ticks.min);
+			scale._userMax = scale._parse(scale.options.ticks.max);
 
 			// TODO(SB): I think we should be able to remove this custom case (options.scale)
 			// and consider it as a regular scale part of the "scales"" map only! This would

diff --git a/src/core/core.datasetController.js b/src/core/core.datasetController.js
@@ -3,6 +3,7 @@
 var helpers = require('../helpers/index');
 
 var resolve = helpers.options.resolve;
+var isNullOrUndef = helpers.isNullOrUndef;
 
 var arrayEvents = ['push', 'pop', 'shift', 'splice', 'unshift'];
 
@@ -138,6 +139,18 @@ function getFirstScaleId(chart, axis) {
 	return (scalesOpts && scalesOpts[prop] && scalesOpts[prop].length && scalesOpts[prop][0].id) || scaleId;
 }
 
+function getUserBounds(scale) {
+	var min = scale._userMin;
+	var max = scale._userMax;
+	if (isNullOrUndef(min) || isNaN(min)) {
+		min = Number.NEGATIVE_INFINITY;
+	}
+	if (isNullOrUndef(max) || isNaN(max)) {
+		max = Number.POSITIVE_INFINITY;
+	}
+	return {min, max};
+}
+
 // Base class for all dataset controllers (line, bar, etc)
 var DatasetController = function(chart, datasetIndex) {
 	this.initialize(chart, datasetIndex);
@@ -575,6 +588,7 @@ helpers.extend(DatasetController.prototype, {
 		var stacked = canStack && meta._stacked;
 		var indices = getSortedDatasetIndices(chart, true);
 		var otherScale = this._getOtherScale(scale);
+		var otherScaleBounds = getUserBounds(otherScale);
 		var i, item, value, parsed, stack, min, minPositive, otherValue;
 
 		min = minPositive = Number.POSITIVE_INFINITY;
@@ -585,7 +599,7 @@ helpers.extend(DatasetController.prototype, {
 			value = parsed[scale.id];
 			otherValue = parsed[otherScale.id];
 			if (item.hidden || isNaN(value) ||
-				otherScale._parsedMin > otherValue || otherScale._parsedMax < otherValue) {
+				otherScaleBounds.min > otherValue || otherScaleBounds.max < otherValue) {
 				continue;
 			}
 			if (stacked) {

diff --git a/src/core/core.scale.js b/src/core/core.scale.js
@@ -361,10 +361,10 @@ var Scale = Element.extend({
 		var minPositive = Number.POSITIVE_INFINITY;
 		var i, ilen, metas, minmax;
 
-		if (isFinite(me._parsedMin) && isFinite(me._parsedMax)) {
+		if (isFinite(me._userMin) && isFinite(me._userMax)) {
 			return {
-				min: me._parsedMin,
-				max: me._parsedMax
+				min: me._userMin,
+				max: me._userMax
 			};
 		}
 

diff --git a/src/scales/scale.category.js b/src/scales/scale.category.js
@@ -26,8 +26,8 @@ module.exports = Scale.extend({
 		var me = this;
 		var max = me._getLabels().length - 1;
 
-		me.min = Math.max(0, me._parsedMin || 0);
-		me.max = Math.min(me._parsedMax || max, max);
+		me.min = Math.max(me._userMin || 0, 0);
+		me.max = Math.min(me._userMax || max, max);
 	},
 
 	buildTicks: function() {

diff --git a/src/scales/scale.logarithmic.js b/src/scales/scale.logarithmic.js
@@ -91,8 +91,8 @@ module.exports = Scale.extend({
 		var me = this;
 		var DEFAULT_MIN = 1;
 		var DEFAULT_MAX = 10;
-		var min = positiveOrDefault(me._parsedMin, me.min);
-		var max = positiveOrDefault(me._parsedMax, me.max);
+		var min = positiveOrDefault(me._userMin, me.min);
+		var max = positiveOrDefault(me._userMax, me.max);
 
 		if (min === max) {
 			if (min !== 0 && min !== null) {