From: Evert Timberg Date: Wed, 6 Jul 2016 22:22:42 +0000 (-0400) Subject: Fix bar and line controllers to convert strings to numbers when considering the value... X-Git-Tag: v2.2.0-rc.1~3^2~1 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=7ca5f91b10632668ffb743d717807a9a03152021;p=thirdparty%2FChart.js.git Fix bar and line controllers to convert strings to numbers when considering the values for stacking the chart. Simplified the base calculation for the bar charts and added test coverage to ensure that strings will work correctly. --- diff --git a/src/controllers/controller.bar.js b/src/controllers/controller.bar.js index 29256cd30..0c2e5b91c 100644 --- a/src/controllers/controller.bar.js +++ b/src/controllers/controller.bar.js @@ -106,23 +106,14 @@ module.exports = function(Chart) { if (yScale.options.stacked) { var chart = me.chart; var datasets = chart.data.datasets; - var value = datasets[datasetIndex].data[index]; + var value = Number(datasets[datasetIndex].data[index]); - if (value < 0) { - for (var i = 0; i < datasetIndex; i++) { - var negDS = datasets[i]; - var negDSMeta = chart.getDatasetMeta(i); - if (negDSMeta.bar && negDSMeta.yAxisID === yScale.id && chart.isDatasetVisible(i)) { - base += negDS.data[index] < 0 ? negDS.data[index] : 0; - } - } - } else { - for (var j = 0; j < datasetIndex; j++) { - var posDS = datasets[j]; - var posDSMeta = chart.getDatasetMeta(j); - if (posDSMeta.bar && posDSMeta.yAxisID === yScale.id && chart.isDatasetVisible(j)) { - base += posDS.data[index] > 0 ? posDS.data[index] : 0; - } + for (var i = 0; i < datasetIndex; i++) { + var currentDs = datasets[i]; + var currentDsMeta = chart.getDatasetMeta(i); + if (currentDsMeta.bar && currentDsMeta.yAxisID === yScale.id && chart.isDatasetVisible(i)) { + var currentVal = Number(currentDs.data[index]); + base += value < 0 ? Math.min(currentVal, 0) : Math.max(currentVal, 0); } } @@ -216,7 +207,7 @@ module.exports = function(Chart) { var me = this; var meta = me.getMeta(); var yScale = me.getScaleForId(meta.yAxisID); - var value = me.getDataset().data[index]; + var value = Number(me.getDataset().data[index]); if (yScale.options.stacked) { @@ -227,10 +218,11 @@ module.exports = function(Chart) { var ds = me.chart.data.datasets[i]; var dsMeta = me.chart.getDatasetMeta(i); if (dsMeta.bar && dsMeta.yAxisID === yScale.id && me.chart.isDatasetVisible(i)) { - if (ds.data[index] < 0) { - sumNeg += ds.data[index] || 0; + var stackedVal = Number(ds.data[index]); + if (stackedVal < 0) { + sumNeg += stackedVal || 0; } else { - sumPos += ds.data[index] || 0; + sumPos += stackedVal || 0; } } } @@ -455,9 +447,9 @@ module.exports = function(Chart) { if (xScale.options.stacked) { - var value = me.chart.data.datasets[datasetIndex].data[index]; + var value = Number(me.chart.data.datasets[datasetIndex].data[index]); - if (value < 0) { + /*if (value < 0) { for (var i = 0; i < datasetIndex; i++) { var negDS = me.chart.data.datasets[i]; var negDSMeta = me.chart.getDatasetMeta(i); @@ -473,6 +465,14 @@ module.exports = function(Chart) { base += posDS.data[index] > 0 ? posDS.data[index] : 0; } } + }*/ + for (var i = 0; i < datasetIndex; i++) { + var currentDs = datasets[i]; + var currentDsMeta = chart.getDatasetMeta(i); + if (currentDsMeta.bar && currentDsMeta.xAxisID === xScale.id && chart.isDatasetVisible(i)) { + var currentVal = Number(currentDs.data[index]); + base += value < 0 ? Math.min(currentVal, 0) : Math.max(currentVal, 0); + } } return xScale.getPixelForValue(base); @@ -528,7 +528,7 @@ module.exports = function(Chart) { var me = this; var meta = me.getMeta(); var xScale = me.getScaleForId(meta.xAxisID); - var value = me.getDataset().data[index]; + var value = Number(me.getDataset().data[index]); if (xScale.options.stacked) { @@ -539,10 +539,11 @@ module.exports = function(Chart) { var ds = me.chart.data.datasets[i]; var dsMeta = me.chart.getDatasetMeta(i); if (dsMeta.bar && dsMeta.xAxisID === xScale.id && me.chart.isDatasetVisible(i)) { - if (ds.data[index] < 0) { - sumNeg += ds.data[index] || 0; + var stackedVal = Number(ds.data[index]); + if (stackedVal < 0) { + sumNeg += stackedVal || 0; } else { - sumPos += ds.data[index] || 0; + sumPos += stackedVal || 0; } } } diff --git a/src/controllers/controller.line.js b/src/controllers/controller.line.js index f3d7328d4..669627694 100644 --- a/src/controllers/controller.line.js +++ b/src/controllers/controller.line.js @@ -221,7 +221,7 @@ module.exports = function(Chart) { ds = chart.data.datasets[i]; dsMeta = chart.getDatasetMeta(i); if (dsMeta.type === 'line' && chart.isDatasetVisible(i)) { - var stackedRightValue = yScale.getRightValue(ds.data[index]); + var stackedRightValue = Number(yScale.getRightValue(ds.data[index])); if (stackedRightValue < 0) { sumNeg += stackedRightValue || 0; } else { @@ -230,7 +230,7 @@ module.exports = function(Chart) { } } - var rightValue = yScale.getRightValue(value); + var rightValue = Number(yScale.getRightValue(value)); if (rightValue < 0) { return yScale.getPixelForValue(sumNeg + rightValue); } else { diff --git a/test/controller.bar.tests.js b/test/controller.bar.tests.js index 560b76794..f997dcb4b 100644 --- a/test/controller.bar.tests.js +++ b/test/controller.bar.tests.js @@ -285,6 +285,60 @@ describe('Bar controller tests', function() { }); }); + it('should update elements when the scales are stacked and data is strings', function() { + var chart = window.acquireChart({ + type: 'bar', + data: { + datasets: [{ + data: ['10', '-10', '10', '-10'], + label: 'dataset1' + }, { + data: ['10', '15', '0', '-4'], + label: 'dataset2' + }], + labels: ['label1', 'label2', 'label3', 'label4'] + }, + options: { + scales: { + xAxes: [{ + type: 'category', + stacked: true + }], + yAxes: [{ + type: 'linear', + stacked: true + }] + } + } + }); + + var meta0 = chart.getDatasetMeta(0); + + [ { b: 290, w: 91, x: 95, y: 161 }, + { b: 290, w: 91, x: 209, y: 419 }, + { b: 290, w: 91, x: 322, y: 161 }, + { b: 290, w: 91, x: 436, y: 419 } + ].forEach(function(values, i) { + expect(meta0.data[i]._model.base).toBeCloseToPixel(values.b); + expect(meta0.data[i]._model.width).toBeCloseToPixel(values.w); + expect(meta0.data[i]._model.x).toBeCloseToPixel(values.x); + expect(meta0.data[i]._model.y).toBeCloseToPixel(values.y); + }); + + var meta1 = chart.getDatasetMeta(1); + + [ { b: 161, w: 91, x: 95, y: 32 }, + { b: 290, w: 91, x: 209, y: 97 }, + { b: 161, w: 91, x: 322, y: 161 }, + { b: 419, w: 91, x: 436, y: 471 } + ].forEach(function(values, i) { + expect(meta1.data[i]._model.base).toBeCloseToPixel(values.b); + expect(meta1.data[i]._model.width).toBeCloseToPixel(values.w); + expect(meta1.data[i]._model.x).toBeCloseToPixel(values.x); + expect(meta1.data[i]._model.y).toBeCloseToPixel(values.y); + }); + }); + it('should draw all bars', function() { var chart = window.acquireChart({ type: 'bar', diff --git a/test/controller.line.tests.js b/test/controller.line.tests.js index 78aecf2d2..041705475 100644 --- a/test/controller.line.tests.js +++ b/test/controller.line.tests.js @@ -348,6 +348,52 @@ describe('Line controller tests', function() { }); + it('should update elements when the y scale is stacked and data is strings', function() { + var chart = window.acquireChart({ + type: 'line', + data: { + datasets: [{ + data: ['10', '-10', '10', '-10'], + label: 'dataset1' + }, { + data: ['10', '15', '0', '-4'], + label: 'dataset2' + }], + labels: ['label1', 'label2', 'label3', 'label4'] + }, + options: { + scales: { + yAxes: [{ + stacked: true + }] + } + } + }); + + var meta0 = chart.getDatasetMeta(0); + + [ { x: 38, y: 161 }, + { x: 189, y: 419 }, + { x: 341, y: 161 }, + { x: 492, y: 419 } + ].forEach(function(values, i) { + expect(meta0.data[i]._model.x).toBeCloseToPixel(values.x); + expect(meta0.data[i]._model.y).toBeCloseToPixel(values.y); + }); + + var meta1 = chart.getDatasetMeta(1); + + [ { x: 38, y: 32 }, + { x: 189, y: 97 }, + { x: 341, y: 161 }, + { x: 492, y: 471 } + ].forEach(function(values, i) { + expect(meta1.data[i]._model.x).toBeCloseToPixel(values.x); + expect(meta1.data[i]._model.y).toBeCloseToPixel(values.y); + }); + + }); + it('should find the correct scale zero when the data is all positive', function() { var chart = window.acquireChart({ type: 'line',