From 7014b57041e827b4477015fc588f6bec020a0414 Mon Sep 17 00:00:00 2001 From: Shelby Sturgis Date: Wed, 25 Feb 2015 12:39:39 -0500 Subject: [PATCH 1/5] adding initial fix for issue with misaligned colors in the pie chart and legend --- src/kibana/components/vislib/lib/data.js | 30 +++++++++++++++++++ .../vislib/visualizations/pie_chart.js | 2 +- 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/src/kibana/components/vislib/lib/data.js b/src/kibana/components/vislib/lib/data.js index 596414417ee2..f5c72a579159 100644 --- a/src/kibana/components/vislib/lib/data.js +++ b/src/kibana/components/vislib/lib/data.js @@ -6,6 +6,7 @@ define(function (require) { var orderKeys = Private(require('components/vislib/components/zero_injection/ordered_x_keys')); var getLabels = Private(require('components/vislib/components/labels/labels')); var color = Private(require('components/vislib/components/color/color')); + var errors = require('errors'); /** * Provides an API for pulling values off the data @@ -510,6 +511,33 @@ define(function (require) { } }; + /** + * Checks whether all pie slices have zero values. + * If so, an error is thrown. + */ + Data.prototype._validatePieData = function () { + var visData = this.getVisData(); + + visData.forEach(function (chartData) { + chartData.slices = (function withoutZeroSlices(slices) { + if (!slices.children) return slices; + + slices = _.clone(slices); + slices.children = slices.children.reduce(function (children, child) { + if (child.size !== 0) { + children.push(withoutZeroSlices(child)); + } + return children; + }, []); + return slices; + }(chartData.slices)); + + if (chartData.slices.children.length === 0) { + throw new errors.PieContainsAllZeros(); + } + }); + }; + /** * Returns an array of names ordered by appearance in the nested array * of objects @@ -521,6 +549,8 @@ define(function (require) { var self = this; var names = []; + this._validatePieData(); + _.forEach(this.getVisData(), function (obj) { var columns = obj.raw ? obj.raw.columns : undefined; diff --git a/src/kibana/components/vislib/visualizations/pie_chart.js b/src/kibana/components/vislib/visualizations/pie_chart.js index 5fb03fd52937..d23f031ee559 100644 --- a/src/kibana/components/vislib/visualizations/pie_chart.js +++ b/src/kibana/components/vislib/visualizations/pie_chart.js @@ -25,7 +25,7 @@ define(function (require) { PieChart.Super.apply(this, arguments); // Check whether pie chart should be rendered. - this._validatePieData(); + //this._validatePieData(); this._attr = _.defaults(handler._attr || {}, { isDonut: handler._attr.isDonut || false From 67e58191f134dbc3b6346ea332764e84b1bc0942 Mon Sep 17 00:00:00 2001 From: Shelby Sturgis Date: Wed, 25 Feb 2015 15:16:46 -0500 Subject: [PATCH 2/5] removing validate pie data from pie chart class and placing in data class. Beginning ideas for testing. --- .../vislib/visualizations/pie_chart.js | 26 ------------------- .../specs/vislib/visualizations/pie_chart.js | 24 +++++++++++++++++ 2 files changed, 24 insertions(+), 26 deletions(-) diff --git a/src/kibana/components/vislib/visualizations/pie_chart.js b/src/kibana/components/vislib/visualizations/pie_chart.js index d23f031ee559..debfa5d48bc6 100644 --- a/src/kibana/components/vislib/visualizations/pie_chart.js +++ b/src/kibana/components/vislib/visualizations/pie_chart.js @@ -24,9 +24,6 @@ define(function (require) { } PieChart.Super.apply(this, arguments); - // Check whether pie chart should be rendered. - //this._validatePieData(); - this._attr = _.defaults(handler._attr || {}, { isDonut: handler._attr.isDonut || false }); @@ -150,29 +147,6 @@ define(function (require) { return path; }; - /** - * Checks whether all pie slices have zero values. - * If so, an error is thrown. - */ - PieChart.prototype._validatePieData = function () { - this.chartData.slices = (function withoutZeroSlices(slices) { - if (!slices.children) return slices; - - slices = _.clone(slices); - slices.children = slices.children.reduce(function (children, child) { - if (child.size !== 0) { - children.push(withoutZeroSlices(child)); - } - return children; - }, []); - return slices; - }(this.chartData.slices)); - - if (this.chartData.slices.children.length === 0) { - throw new errors.PieContainsAllZeros(); - } - }; - /** * Renders d3 visualization * diff --git a/test/unit/specs/vislib/visualizations/pie_chart.js b/test/unit/specs/vislib/visualizations/pie_chart.js index ab264773f136..523b046be6d1 100644 --- a/test/unit/specs/vislib/visualizations/pie_chart.js +++ b/test/unit/specs/vislib/visualizations/pie_chart.js @@ -165,6 +165,30 @@ define(function (require) { vis = null; }); + describe('align color of legend values with pie values', function () { + var legendItems; + var legendItem; + var paths; + var value; + + beforeEach(function () { + inject(function (d3) { + legendItems = vis.handler.data.pieNames(); + vis.handler.charts.forEach(function (chart) { + paths = d3.select(chart.chartEl).selectAll('path')[0]; + }); + }); + }); + + it('should have the same value as the legend', function () { + legendItem = legendItems[legendItems.length - 1]; + value = paths.filter(function (obj) { + return obj.__data__.name === legendItem; + }); + console.log(value); + }); + }); + describe('addPathEvents method', function () { var path; var d3selectedPath; From 61c6a59cb05b7c298da57511befeadda99f519ed Mon Sep 17 00:00:00 2001 From: Shelby Sturgis Date: Wed, 25 Feb 2015 15:18:29 -0500 Subject: [PATCH 3/5] removing tests from pie_chart --- .../specs/vislib/visualizations/pie_chart.js | 24 ------------------- 1 file changed, 24 deletions(-) diff --git a/test/unit/specs/vislib/visualizations/pie_chart.js b/test/unit/specs/vislib/visualizations/pie_chart.js index 523b046be6d1..ab264773f136 100644 --- a/test/unit/specs/vislib/visualizations/pie_chart.js +++ b/test/unit/specs/vislib/visualizations/pie_chart.js @@ -165,30 +165,6 @@ define(function (require) { vis = null; }); - describe('align color of legend values with pie values', function () { - var legendItems; - var legendItem; - var paths; - var value; - - beforeEach(function () { - inject(function (d3) { - legendItems = vis.handler.data.pieNames(); - vis.handler.charts.forEach(function (chart) { - paths = d3.select(chart.chartEl).selectAll('path')[0]; - }); - }); - }); - - it('should have the same value as the legend', function () { - legendItem = legendItems[legendItems.length - 1]; - value = paths.filter(function (obj) { - return obj.__data__.name === legendItem; - }); - console.log(value); - }); - }); - describe('addPathEvents method', function () { var path; var d3selectedPath; From 7453c014f286f9e7b252617be6507cef55fbf5c2 Mon Sep 17 00:00:00 2001 From: Shelby Sturgis Date: Wed, 25 Feb 2015 23:30:26 -0500 Subject: [PATCH 4/5] adding tests --- test/unit/specs/vislib/lib/legend.js | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/test/unit/specs/vislib/lib/legend.js b/test/unit/specs/vislib/lib/legend.js index 396047a27901..aa2e78c9716f 100644 --- a/test/unit/specs/vislib/lib/legend.js +++ b/test/unit/specs/vislib/lib/legend.js @@ -48,6 +48,32 @@ define(function (require) { vis = null; }); + describe('legend item color matches slice color', function () { + var items; + var paths; + var getColor; + + if (chartTypes[i] === 'pie') { + it('should match the slice color', function () { + paths = $(vis.el).find('path').toArray(); + items = vis.handler.legend.labels; + getColor = vis.handler.legend.color; + + items.forEach(function (label) { + var slice = paths.filter(function (path) { + if (path.__data__.name === undefined) return false; + return path.__data__.name.toString() === label; + }); + + slice.forEach(function (s) { + var hexColor = s.classList[2].replace('c', '#'); + expect(hexColor).to.be(getColor(label)); + }); + }); + }); + } + }); + describe('header method', function () { it('should append the legend header', function () { expect($(vis.el).find('.header').length).to.be(1); From e0922c76954086020bbb7d84c37e740d7ae71d2c Mon Sep 17 00:00:00 2001 From: Shelby Sturgis Date: Wed, 25 Feb 2015 23:57:19 -0500 Subject: [PATCH 5/5] fixing the tests --- test/unit/specs/vislib/lib/legend.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/test/unit/specs/vislib/lib/legend.js b/test/unit/specs/vislib/lib/legend.js index aa2e78c9716f..a09e47c5f099 100644 --- a/test/unit/specs/vislib/lib/legend.js +++ b/test/unit/specs/vislib/lib/legend.js @@ -60,14 +60,15 @@ define(function (require) { getColor = vis.handler.legend.color; items.forEach(function (label) { - var slice = paths.filter(function (path) { + var slices = paths.filter(function (path) { if (path.__data__.name === undefined) return false; return path.__data__.name.toString() === label; + }).map(function (path) { + return $(path).attr('class').split(/\s+/)[2].replace('c', '#'); }); - slice.forEach(function (s) { - var hexColor = s.classList[2].replace('c', '#'); - expect(hexColor).to.be(getColor(label)); + slices.forEach(function (hex) { + expect(hex).to.be(getColor(label)); }); }); });