diff --git a/src/controllers/controller.bar.js b/src/controllers/controller.bar.js index 4242b145811..7c30a6a4324 100644 --- a/src/controllers/controller.bar.js +++ b/src/controllers/controller.bar.js @@ -384,7 +384,7 @@ module.exports = DatasetController.extend({ value = custom.barStart; length = custom.barEnd - custom.barStart; // bars crossing origin are not stacked - if (value !== 0 && Math.sign(value) !== Math.sign(custom.barEnd)) { + if (value !== 0 && helpers.sign(value) !== helpers.sign(custom.barEnd)) { start = 0; } start += value; diff --git a/src/core/core.controller.js b/src/core/core.controller.js index fb42f119571..5b2f5230ef3 100644 --- a/src/core/core.controller.js +++ b/src/core/core.controller.js @@ -391,6 +391,10 @@ helpers.extend(Chart.prototype, /** @lends Chart */ { scales[scale.id] = scale; } + // parse min/max value, so we can properly determine min/max for other scales + 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 // make the logic easier and remove some useless? custom code. diff --git a/src/core/core.datasetController.js b/src/core/core.datasetController.js index 43afbc11378..e3b5123708f 100644 --- a/src/core/core.datasetController.js +++ b/src/core/core.datasetController.js @@ -146,7 +146,7 @@ function applyStack(stack, value, dsIndex, allOther) { break; } otherValue = stack.values[datasetIndex]; - if (!isNaN(otherValue) && (value === 0 || Math.sign(value) === Math.sign(otherValue))) { + if (!isNaN(otherValue) && (value === 0 || helpers.sign(value) === helpers.sign(otherValue))) { value += otherValue; } } @@ -185,6 +185,13 @@ function getFirstScaleId(chart, axis) { return (scalesOpts && scalesOpts[prop] && scalesOpts[prop].length && scalesOpts[prop][0].id) || scaleId; } +function getUserBounds(scale) { + var {min, max, minDefined, maxDefined} = scale._getUserBounds(); + return { + min: minDefined ? min : Number.NEGATIVE_INFINITY, + max: maxDefined ? max : Number.POSITIVE_INFINITY + }; +} // Base class for all dataset controllers (line, bar, etc) var DatasetController = function(chart, datasetIndex) { this.initialize(chart, datasetIndex); @@ -298,6 +305,15 @@ helpers.extend(DatasetController.prototype, { return this.getScaleForId(this._getIndexScaleId()); }, + /** + * @private + */ + _getOtherScale: function(scale) { + return scale.id === this._getIndexScaleId() + ? this._getValueScale() + : this._getIndexScale(); + }, + reset: function() { this._update(true); }, @@ -615,7 +631,9 @@ helpers.extend(DatasetController.prototype, { var max = Number.NEGATIVE_INFINITY; var stacked = canStack && meta._stacked; var indices = getSortedDatasetIndices(chart, true); - var i, item, value, parsed, stack, min, minPositive; + var otherScale = this._getOtherScale(scale); + var {min: otherMin, max: otherMax} = getUserBounds(otherScale); + var i, item, value, parsed, stack, min, minPositive, otherValue; min = minPositive = Number.POSITIVE_INFINITY; @@ -623,7 +641,9 @@ helpers.extend(DatasetController.prototype, { item = metaData[i]; parsed = item._parsed; value = parsed[scale.id]; - if (item.hidden || isNaN(value)) { + otherValue = parsed[otherScale.id]; + if (item.hidden || isNaN(value) || + otherMin > otherValue || otherMax < otherValue) { continue; } if (stacked) { diff --git a/src/core/core.scale.js b/src/core/core.scale.js index 52012877d1e..7fafa704a59 100644 --- a/src/core/core.scale.js +++ b/src/core/core.scale.js @@ -6,6 +6,7 @@ const helpers = require('../helpers/index'); const Ticks = require('./core.ticks'); const isArray = helpers.isArray; +const isFinite = helpers.isFinite; const isNullOrUndef = helpers.isNullOrUndef; const valueOrDefault = helpers.valueOrDefault; const resolve = helpers.options.resolve; @@ -347,26 +348,49 @@ class Scale extends Element { return null; } + /** + * @private + * @since 3.0 + */ + _getUserBounds() { + var min = this._userMin; + var max = this._userMax; + if (isNullOrUndef(min) || isNaN(min)) { + min = Number.POSITIVE_INFINITY; + } + if (isNullOrUndef(max) || isNaN(max)) { + max = Number.NEGATIVE_INFINITY; + } + return {min, max, minDefined: isFinite(min), maxDefined: isFinite(max)}; + } + + /** + * @private + * @since 3.0 + */ _getMinMax(canStack) { var me = this; - var metas = me._getMatchingVisibleMetas(); - var min = Number.POSITIVE_INFINITY; - var max = Number.NEGATIVE_INFINITY; + var {min, max, minDefined, maxDefined} = me._getUserBounds(); var minPositive = Number.POSITIVE_INFINITY; - var i, ilen, minmax; + var i, ilen, metas, minmax; + + if (minDefined && maxDefined) { + return {min, max}; + } + metas = me._getMatchingVisibleMetas(); for (i = 0, ilen = metas.length; i < ilen; ++i) { minmax = metas[i].controller._getMinMax(me, canStack); - min = Math.min(min, minmax.min); - max = Math.max(max, minmax.max); + if (!minDefined) { + min = Math.min(min, minmax.min); + } + if (!maxDefined) { + max = Math.max(max, minmax.max); + } minPositive = Math.min(minPositive, minmax.minPositive); } - return { - min: min, - max: max, - minPositive: minPositive - }; + return {min, max, minPositive}; } _invalidateCaches() {} diff --git a/src/scales/scale.category.js b/src/scales/scale.category.js index cb07a6751b2..1c9d3d91e35 100644 --- a/src/scales/scale.category.js +++ b/src/scales/scale.category.js @@ -24,44 +24,20 @@ module.exports = Scale.extend({ determineDataLimits: function() { var me = this; - var labels = me._getLabels(); - var ticksOpts = me.options.ticks; - var min = ticksOpts.min; - var max = ticksOpts.max; - var minIndex = 0; - var maxIndex = labels.length - 1; - var findIndex; - - if (min !== undefined) { - // user specified min value - findIndex = labels.indexOf(min); - if (findIndex >= 0) { - minIndex = findIndex; - } - } - - if (max !== undefined) { - // user specified max value - findIndex = labels.indexOf(max); - if (findIndex >= 0) { - maxIndex = findIndex; - } - } + var max = me._getLabels().length - 1; - me.minIndex = minIndex; - me.maxIndex = maxIndex; - me.min = labels[minIndex]; - me.max = labels[maxIndex]; + me.min = Math.max(me._userMin || 0, 0); + me.max = Math.min(me._userMax || max, max); }, buildTicks: function() { var me = this; var labels = me._getLabels(); - var minIndex = me.minIndex; - var maxIndex = me.maxIndex; + var min = me.min; + var max = me.max; // If we are viewing some subset of labels, slice the original array - labels = (minIndex === 0 && maxIndex === labels.length - 1) ? labels : labels.slice(minIndex, maxIndex + 1); + labels = (min === 0 && max === labels.length - 1) ? labels : labels.slice(min, max + 1); return labels.map(function(l) { return {value: l}; }); @@ -93,7 +69,7 @@ module.exports = Scale.extend({ return; } - me._startValue = me.minIndex - (offset ? 0.5 : 0); + me._startValue = me.min - (offset ? 0.5 : 0); me._valueRange = Math.max(ticks.length - (offset ? 0 : 1), 1); }, @@ -112,7 +88,7 @@ module.exports = Scale.extend({ var ticks = this.ticks; return index < 0 || index > ticks.length - 1 ? null - : this.getPixelForValue(index + this.minIndex); + : this.getPixelForValue(index + this.min); }, getValueForPixel: function(pixel) { diff --git a/src/scales/scale.linearbase.js b/src/scales/scale.linearbase.js index 2401801aeb9..cff6830ea15 100644 --- a/src/scales/scale.linearbase.js +++ b/src/scales/scale.linearbase.js @@ -84,7 +84,7 @@ function generateTicks(generationOptions, dataRange) { } module.exports = Scale.extend({ - _parse: function(raw) { + _parse: function(raw, index) { // eslint-disable-line no-unused-vars if (helpers.isNullOrUndef(raw)) { return NaN; } diff --git a/src/scales/scale.logarithmic.js b/src/scales/scale.logarithmic.js index f85ed9549d6..ae5e65824f4 100644 --- a/src/scales/scale.logarithmic.js +++ b/src/scales/scale.logarithmic.js @@ -64,13 +64,11 @@ var defaultConfig = { } }; -// TODO(v3): change this to positiveOrDefault -function nonNegativeOrDefault(value, defaultValue) { - return helpers.isFinite(value) && value >= 0 ? value : defaultValue; -} - module.exports = Scale.extend({ - _parse: LinearScaleBase.prototype._parse, + _parse: function(raw, index) { // eslint-disable-line no-unused-vars + const value = LinearScaleBase.prototype._parse.apply(this, arguments); + return helpers.isFinite(value) && value >= 0 ? value : undefined; + }, determineDataLimits: function() { var me = this; @@ -88,39 +86,39 @@ module.exports = Scale.extend({ handleTickRangeOptions: function() { var me = this; - var tickOpts = me.options.ticks; var DEFAULT_MIN = 1; var DEFAULT_MAX = 10; + var min = me.min; + var 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) { - me.min = Math.pow(10, Math.floor(log10(me.min)) - 1); - me.max = Math.pow(10, Math.floor(log10(me.max)) + 1); + if (min === max) { + if (min !== 0 && min !== null) { + min = Math.pow(10, Math.floor(log10(min)) - 1); + max = Math.pow(10, Math.floor(log10(max)) + 1); } else { - me.min = DEFAULT_MIN; - me.max = DEFAULT_MAX; + min = DEFAULT_MIN; + max = DEFAULT_MAX; } } - if (me.min === null) { - me.min = Math.pow(10, Math.floor(log10(me.max)) - 1); + if (min === null) { + min = Math.pow(10, Math.floor(log10(max)) - 1); } - if (me.max === null) { - me.max = me.min !== 0 - ? Math.pow(10, Math.floor(log10(me.min)) + 1) + if (max === null) { + max = min !== 0 + ? Math.pow(10, Math.floor(log10(min)) + 1) : DEFAULT_MAX; } if (me.minNotZero === null) { - if (me.min > 0) { - me.minNotZero = me.min; - } else if (me.max < 1) { - me.minNotZero = Math.pow(10, Math.floor(log10(me.max))); + if (min > 0) { + me.minNotZero = min; + } else if (max < 1) { + me.minNotZero = Math.pow(10, Math.floor(log10(max))); } else { me.minNotZero = DEFAULT_MIN; } } + me.min = min; + me.max = max; }, buildTicks: function() { @@ -129,8 +127,8 @@ module.exports = Scale.extend({ var reverse = !me.isHorizontal(); var generationOptions = { - min: nonNegativeOrDefault(tickOpts.min), - max: nonNegativeOrDefault(tickOpts.max) + min: me._userMin, + max: me._userMax }; var ticks = generateTicks(generationOptions, me); diff --git a/src/scales/scale.time.js b/src/scales/scale.time.js index c3d56680a17..d0bf44aca34 100644 --- a/src/scales/scale.time.js +++ b/src/scales/scale.time.js @@ -177,7 +177,11 @@ function interpolate(table, skey, sval, tkey) { return prev[tkey] + offset; } -function toTimestamp(scale, input) { +function parse(scale, input) { + if (helpers.isNullOrUndef(input)) { + return null; + } + var adapter = scale._adapter; var options = scale.options.time; var parser = options.parser; @@ -194,25 +198,15 @@ function toTimestamp(scale, input) { : adapter.parse(value); } - return value !== null ? +value : value; -} - -function parse(scale, input) { - if (helpers.isNullOrUndef(input)) { - return null; - } - - var options = scale.options.time; - var value = toTimestamp(scale, input); if (value === null) { return value; } if (options.round) { - value = +scale._adapter.startOf(value, options.round); + value = scale._adapter.startOf(value, options.round); } - return value; + return +value; } /** @@ -364,42 +358,60 @@ function ticksFromTimestamps(scale, values, majorUnit) { return (ilen === 0 || !majorUnit) ? ticks : setMajorTicks(scale, ticks, map, majorUnit); } - function getDataTimestamps(scale) { var timestamps = scale._cache.data || []; var i, ilen, metas; - if (!timestamps.length) { - metas = scale._getMatchingVisibleMetas(); - for (i = 0, ilen = metas.length; i < ilen; ++i) { - timestamps = timestamps.concat(metas[i].controller._getAllParsedValues(scale)); - } - timestamps = scale._cache.data = arrayUnique(timestamps).sort(sorter); + if (timestamps.length) { + return timestamps; } - return timestamps; + + metas = scale._getMatchingVisibleMetas(); + for (i = 0, ilen = metas.length; i < ilen; ++i) { + timestamps = timestamps.concat(metas[i].controller._getAllParsedValues(scale)); + } + + // We can not assume data is in order or unique - not even for single dataset + // It seems to be somewhat faster to do sorting first + return (scale._cache.data = arrayUnique(timestamps.sort(sorter))); } function getLabelTimestamps(scale) { var timestamps = scale._cache.labels || []; var i, ilen, labels; - if (!timestamps.length) { - labels = scale._getLabels(); - for (i = 0, ilen = labels.length; i < ilen; ++i) { - timestamps.push(parse(scale, labels[i])); - } - timestamps = scale._cache.labels = arrayUnique(timestamps).sort(sorter); + if (timestamps.length) { + return timestamps; } - return timestamps; + + labels = scale._getLabels(); + for (i = 0, ilen = labels.length; i < ilen; ++i) { + timestamps.push(parse(scale, labels[i])); + } + + // We could assume labels are in order and unique - but let's not + return (scale._cache.labels = arrayUnique(timestamps.sort(sorter))); } function getAllTimestamps(scale) { var timestamps = scale._cache.all || []; + var label, data; + + if (timestamps.length) { + return timestamps; + } - if (!timestamps.length) { - timestamps = getDataTimestamps(scale).concat(getLabelTimestamps(scale)); - timestamps = scale._cache.all = arrayUnique(timestamps).sort(sorter); + data = getDataTimestamps(scale); + label = getLabelTimestamps(scale); + if (data.length && label.length) { + // If combining labels and data (data might not contain all labels), + // we need to recheck uniqueness and sort + timestamps = arrayUnique(data.concat(label).sort(sorter)); + } else { + timestamps = data.length ? data : label; } + timestamps = scale._cache.all = timestamps; + return timestamps; } @@ -423,6 +435,24 @@ function getTimestampsForTicks(scale) { return timestamps; } +function getTimestampsForTable(scale) { + return scale.options.distribution === 'series' + ? getAllTimestamps(scale) + : [scale.min, scale.max]; +} + +function getLabelBounds(scale) { + var min = Number.POSITIVE_INFINITY; + var max = Number.NEGATIVE_INFINITY; + var arr = getLabelTimestamps(scale); + + if (arr.length) { + min = arr[0]; + max = arr[arr.length - 1]; + } + return {min, max}; +} + /** * Return subset of `timestamps` between `min` and `max`. * Timestamps are assumend to be in sorted order. @@ -501,7 +531,7 @@ module.exports = Scale.extend({ if (raw === undefined) { return NaN; } - return toTimestamp(this, raw); + return parse(this, raw); }, _parseObject: function(obj, axis, index) { @@ -539,30 +569,35 @@ module.exports = Scale.extend({ determineDataLimits: function() { var me = this; - var adapter = me._adapter; var options = me.options; - var tickOpts = options.ticks; + var adapter = me._adapter; var unit = options.time.unit || 'day'; - var min = Number.POSITIVE_INFINITY; - var max = Number.NEGATIVE_INFINITY; - var minmax = me._getMinMax(false); - var i, ilen, labels; - - min = Math.min(min, minmax.min); - max = Math.max(max, minmax.max); - - labels = getLabelTimestamps(me); - for (i = 0, ilen = labels.length; i < ilen; ++i) { - min = Math.min(min, labels[i]); - max = Math.max(max, labels[i]); + var {min, max, minDefined, maxDefined} = me._getUserBounds(); + + function _applyBounds(bounds) { + if (!minDefined && !isNaN(bounds.min)) { + min = Math.min(min, bounds.min); + } + if (!maxDefined && !isNaN(bounds.max)) { + max = Math.max(max, bounds.max); + } + } + + // If we have user provided `min` and `max` labels / data bounds can be ignored + if (!minDefined || !maxDefined) { + // Labels are always considered, when user did not force bounds + _applyBounds(getLabelBounds(me)); + + // If `bounds` is `'ticks'` and `ticks.source` is `'labels'`, + // data bounds are ignored (and don't need to be determined) + if (options.bounds !== 'ticks' || options.ticks.source !== 'labels') { + _applyBounds(me._getMinMax(false)); + } } min = helpers.isFinite(min) && !isNaN(min) ? min : +adapter.startOf(Date.now(), unit); max = helpers.isFinite(max) && !isNaN(max) ? max : +adapter.endOf(Date.now(), unit) + 1; - min = parse(me, tickOpts.min) || min; - max = parse(me, tickOpts.max) || max; - // Make sure that max is strictly higher than min (required by the lookup table) me.min = Math.min(min, max); me.max = Math.max(min + 1, max); @@ -580,8 +615,8 @@ module.exports = Scale.extend({ timestamps = getTimestampsForTicks(me); if (options.bounds === 'ticks' && timestamps.length) { - me.min = parse(me, tickOpts.min) || timestamps[0]; - me.max = parse(me, tickOpts.max) || timestamps[timestamps.length - 1]; + me.min = me._userMin || timestamps[0]; + me.max = me._userMax || timestamps[timestamps.length - 1]; } min = me.min; @@ -597,7 +632,7 @@ module.exports = Scale.extend({ : determineUnitForFormatting(me, ticks.length, timeOpts.minUnit, me.min, me.max)); me._majorUnit = !tickOpts.major.enabled || me._unit === 'year' ? undefined : determineMajorUnit(me._unit); - me._table = buildLookupTable(getAllTimestamps(me), min, max, distribution); + me._table = buildLookupTable(getTimestampsForTable(me), min, max, distribution); me._offsets = computeOffsets(me._table, ticks, min, max, options); if (tickOpts.reverse) { diff --git a/test/fixtures/controller.line/clip/default-x-max.png b/test/fixtures/controller.line/clip/default-x-max.png index 18a4eb29afa..1b1ff84f819 100644 Binary files a/test/fixtures/controller.line/clip/default-x-max.png and b/test/fixtures/controller.line/clip/default-x-max.png differ diff --git a/test/fixtures/controller.line/clip/default-x.png b/test/fixtures/controller.line/clip/default-x.png index 70ad431e86a..f6601de6983 100644 Binary files a/test/fixtures/controller.line/clip/default-x.png and b/test/fixtures/controller.line/clip/default-x.png differ diff --git a/test/fixtures/controller.line/clip/default-y-max.png b/test/fixtures/controller.line/clip/default-y-max.png index 1089e94bd6a..651e1a7d3c9 100644 Binary files a/test/fixtures/controller.line/clip/default-y-max.png and b/test/fixtures/controller.line/clip/default-y-max.png differ diff --git a/test/fixtures/controller.line/clip/default-y.png b/test/fixtures/controller.line/clip/default-y.png index 6a1e9816fdc..923cf75aa87 100644 Binary files a/test/fixtures/controller.line/clip/default-y.png and b/test/fixtures/controller.line/clip/default-y.png differ diff --git a/test/specs/controller.bar.tests.js b/test/specs/controller.bar.tests.js index b46352d9c19..def8879aee8 100644 --- a/test/specs/controller.bar.tests.js +++ b/test/specs/controller.bar.tests.js @@ -1495,7 +1495,7 @@ describe('Chart.controllers.bar', function() { var totalBarWidth = 0; for (var i = 0; i < chart.data.datasets.length; i++) { var bars = chart.getDatasetMeta(i).data; - for (var j = xScale.minIndex; j <= xScale.maxIndex; j++) { + for (var j = xScale.min; j <= xScale.max; j++) { totalBarWidth += bars[j]._model.width; } if (stacked) { @@ -1573,7 +1573,7 @@ describe('Chart.controllers.bar', function() { var totalBarHeight = 0; for (var i = 0; i < chart.data.datasets.length; i++) { var bars = chart.getDatasetMeta(i).data; - for (var j = yScale.minIndex; j <= yScale.maxIndex; j++) { + for (var j = yScale.min; j <= yScale.max; j++) { totalBarHeight += bars[j]._model.height; } if (stacked) { diff --git a/test/specs/core.scale.tests.js b/test/specs/core.scale.tests.js index d92ea698875..582f952d117 100644 --- a/test/specs/core.scale.tests.js +++ b/test/specs/core.scale.tests.js @@ -581,4 +581,40 @@ describe('Core.scale', function() { }); }); + + describe('min and max', function() { + it('should be limited to visible data', function() { + var chart = window.acquireChart({ + type: 'scatter', + data: { + datasets: [{ + data: [{x: 100, y: 100}, {x: -100, y: -100}] + }, { + data: [{x: 10, y: 10}, {x: -10, y: -10}] + }] + }, + options: { + scales: { + xAxes: [{ + id: 'x', + type: 'linear', + ticks: { + min: -20, + max: 20 + } + }], + yAxes: [{ + id: 'y', + type: 'linear' + }] + } + } + }); + + expect(chart.scales.x.min).toEqual(-20); + expect(chart.scales.x.max).toEqual(20); + expect(chart.scales.y.min).toEqual(-10); + expect(chart.scales.y.max).toEqual(10); + }); + }); }); diff --git a/test/specs/scale.logarithmic.tests.js b/test/specs/scale.logarithmic.tests.js index 58ff809b33d..78de94635d3 100644 --- a/test/specs/scale.logarithmic.tests.js +++ b/test/specs/scale.logarithmic.tests.js @@ -515,7 +515,7 @@ describe('Logarithmic Scale tests', function() { datasets: [{ data: [1, 1, 1, 2, 1, 0] }], - labels: [] + labels: ['a', 'b', 'c', 'd', 'e', 'f'] }, options: { scales: { @@ -523,8 +523,8 @@ describe('Logarithmic Scale tests', function() { id: 'yScale', type: 'logarithmic', ticks: { - min: '', - max: false, + min: 'zero', + max: null, callback: function(value) { return value; }