From 7ca5f91b10632668ffb743d717807a9a03152021 Mon Sep 17 00:00:00 2001 From: Evert Timberg Date: Wed, 6 Jul 2016 18:22:42 -0400 Subject: [PATCH] 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. --- src/controllers/controller.bar.js | 53 +++++++++++++++-------------- src/controllers/controller.line.js | 4 +-- test/controller.bar.tests.js | 54 ++++++++++++++++++++++++++++++ test/controller.line.tests.js | 46 +++++++++++++++++++++++++ 4 files changed, 129 insertions(+), 28 deletions(-) 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', -- 2.47.3