From a1948a3ee4e52ae8dd2ca211b3949a503dc4d9ea Mon Sep 17 00:00:00 2001 From: Spencer Alger Date: Tue, 28 Apr 2015 14:12:03 -0700 Subject: [PATCH 1/3] [vislib/pointSeries] support specifying a yAxisFormatter --- src/kibana/components/vislib/lib/data.js | 15 +++-------- .../vislib/lib/handler/types/point_series.js | 6 ++++- src/kibana/components/vislib/lib/y_axis.js | 27 ++----------------- src/kibana/utils/_mixins_chainable.js | 13 ++++++--- 4 files changed, 20 insertions(+), 41 deletions(-) diff --git a/src/kibana/components/vislib/lib/data.js b/src/kibana/components/vislib/lib/data.js index f5c72a5791599..e89ed976a8dfa 100644 --- a/src/kibana/components/vislib/lib/data.js +++ b/src/kibana/components/vislib/lib/data.js @@ -265,18 +265,9 @@ define(function (require) { * @param thing {String} Data object key * @returns {*} Data object value */ - Data.prototype.get = function (thing) { - var data; - - if (this.data.rows) { - data = this.data.rows; - } else if (this.data.columns) { - data = this.data.columns; - } else { - data = [this.data]; - } - - return _.pluck(data, thing)[0]; + Data.prototype.get = function (thing, def) { + var source = (this.data.rows || this.data.columns || [this.data])[0]; + return _.get(source, thing, def); }; /** diff --git a/src/kibana/components/vislib/lib/handler/types/point_series.js b/src/kibana/components/vislib/lib/handler/types/point_series.js index c5fa17ae6bd60..078c2ca5a285a 100644 --- a/src/kibana/components/vislib/lib/handler/types/point_series.js +++ b/src/kibana/components/vislib/lib/handler/types/point_series.js @@ -27,6 +27,9 @@ define(function (require) { data = new Data(vis.data, vis._attr); } + var isPercentage = (vis._attr.mode === 'percentage'); + var tickFormat = isPercentage ? d3.format('%') : data.get('yAxisFormatter', d3.format('n')); + return new Handler(vis, { data: data, legend: new Legend(vis, vis.el, data.labels, data.color, vis._attr), @@ -45,7 +48,8 @@ define(function (require) { el : vis.el, yMin : data.getYMin(), yMax : data.getYMax(), - _attr: vis._attr + _attr: vis._attr, + tickFormat: tickFormat }) }); }; diff --git a/src/kibana/components/vislib/lib/y_axis.js b/src/kibana/components/vislib/lib/y_axis.js index 06a44e8b0735a..7c2092176326e 100644 --- a/src/kibana/components/vislib/lib/y_axis.js +++ b/src/kibana/components/vislib/lib/y_axis.js @@ -18,6 +18,7 @@ define(function (require) { this.el = args.el; this.yMin = args.yMin; this.yMax = args.yMax; + this.tickFormat = args.tickFormat; this._attr = args._attr || {}; } @@ -103,20 +104,6 @@ define(function (require) { .nice(); }; - /** - * By default, d3.format('s') returns billion values - * with a `G` instead of a `B`. @method formatAxisLabel returns - * billion values with a B instead of a G. Else, it defaults - * to the d3.format('s') value. - * - * @method formatAxisLabel - * @param d {Number} - * @returns {*} - */ - YAxis.prototype.formatAxisLabel = function (d) { - return numeral(d).format('0.[0]a'); - }; - /** * Creates the d3 y axis function * @@ -126,16 +113,6 @@ define(function (require) { */ YAxis.prototype.getYAxis = function (height) { var yScale = this.getYScale(height); - var isPercentage = (this._attr.mode === 'percentage'); - var tickFormat; - - if (isPercentage) { - tickFormat = d3.format('%'); - } else if (this.yMax <= 100 && this.yMin >= -100 && !isPercentage) { - tickFormat = d3.format('n'); - } else { - tickFormat = this.formatAxisLabel; - } // y scale should never be `NaN` if (!yScale || _.isNaN(yScale)) { @@ -145,7 +122,7 @@ define(function (require) { // Create the d3 yAxis function this.yAxis = d3.svg.axis() .scale(yScale) - .tickFormat(tickFormat) + .tickFormat(this.tickFormat) .ticks(this.tickScale(height)) .orient('left'); diff --git a/src/kibana/utils/_mixins_chainable.js b/src/kibana/utils/_mixins_chainable.js index 1ec07749c537d..27285c8f2b41d 100644 --- a/src/kibana/utils/_mixins_chainable.js +++ b/src/kibana/utils/_mixins_chainable.js @@ -250,11 +250,18 @@ define(function (require) { }, /** - * Shortcut for the simple version of _.deepGet + * Shortcut for the simple version of _.deepGet with support for default + * values added + * + * @param {obj} any - the value to read from + * @param {string|array} path - the location of the value to return as + * a dot-notated string or array of keys. + * @param {any} def - when the value is null or undefined return this instead * @return {any} */ - get: function (obj, path) { - return _.deepGet(obj, path); + get: function (obj, path, def) { + var val = _.deepGet(obj, path); + return (val == null && def != null) ? def : val; }, /** From 0a7596149c11d3ccc6898404ef65668a38c9d6a5 Mon Sep 17 00:00:00 2001 From: Spencer Alger Date: Thu, 30 Apr 2015 03:20:39 -0700 Subject: [PATCH 2/3] [vislib/yAxis] calculate the tickFormat when it is used --- .../vislib/lib/handler/types/point_series.js | 7 +------ src/kibana/components/vislib/lib/y_axis.js | 12 +++++++++--- test/unit/specs/vislib/lib/y_axis.js | 16 +--------------- 3 files changed, 11 insertions(+), 24 deletions(-) diff --git a/src/kibana/components/vislib/lib/handler/types/point_series.js b/src/kibana/components/vislib/lib/handler/types/point_series.js index 078c2ca5a285a..ed27c00f335e7 100644 --- a/src/kibana/components/vislib/lib/handler/types/point_series.js +++ b/src/kibana/components/vislib/lib/handler/types/point_series.js @@ -1,7 +1,5 @@ define(function (require) { return function ColumnHandler(d3, Private) { - var $ = require('jquery'); - var injectZeros = Private(require('components/vislib/components/zero_injection/inject_zeros')); var Handler = Private(require('components/vislib/lib/handler/handler')); var Data = Private(require('components/vislib/lib/data')); @@ -27,9 +25,6 @@ define(function (require) { data = new Data(vis.data, vis._attr); } - var isPercentage = (vis._attr.mode === 'percentage'); - var tickFormat = isPercentage ? d3.format('%') : data.get('yAxisFormatter', d3.format('n')); - return new Handler(vis, { data: data, legend: new Legend(vis, vis.el, data.labels, data.color, vis._attr), @@ -49,7 +44,7 @@ define(function (require) { yMin : data.getYMin(), yMax : data.getYMax(), _attr: vis._attr, - tickFormat: tickFormat + tickFormat: data.get('yAxisFormatter') }) }); }; diff --git a/src/kibana/components/vislib/lib/y_axis.js b/src/kibana/components/vislib/lib/y_axis.js index 7c2092176326e..90ce6918c513f 100644 --- a/src/kibana/components/vislib/lib/y_axis.js +++ b/src/kibana/components/vislib/lib/y_axis.js @@ -2,7 +2,6 @@ define(function (require) { return function YAxisFactory(d3, Private) { var _ = require('lodash'); var $ = require('jquery'); - var numeral = require('numeral'); var errors = require('errors'); var ErrorHandler = Private(require('components/vislib/lib/_error_handler')); @@ -18,7 +17,7 @@ define(function (require) { this.el = args.el; this.yMin = args.yMin; this.yMax = args.yMax; - this.tickFormat = args.tickFormat; + this.yAxisFormatter = args.yAxisFormatter; this._attr = args._attr || {}; } @@ -104,6 +103,13 @@ define(function (require) { .nice(); }; + YAxis.prototype.tickFormat = function () { + var isPercentage = this._attr.mode === 'percentage'; + if (isPercentage) return d3.format('%'); + if (this.yAxisFormatter) return this.yAxisFormatter; + return d3.format('n'); + }; + /** * Creates the d3 y axis function * @@ -122,7 +128,7 @@ define(function (require) { // Create the d3 yAxis function this.yAxis = d3.svg.axis() .scale(yScale) - .tickFormat(this.tickFormat) + .tickFormat(this.tickFormat()) .ticks(this.tickScale(height)) .orient('left'); diff --git a/test/unit/specs/vislib/lib/y_axis.js b/test/unit/specs/vislib/lib/y_axis.js index 3a7f2233676f3..043d2ae3b77c8 100644 --- a/test/unit/specs/vislib/lib/y_axis.js +++ b/test/unit/specs/vislib/lib/y_axis.js @@ -206,20 +206,6 @@ define(function (require) { }); }); - describe('formatAxisLabel method', function () { - var num = 1e9; - var val; - - beforeEach(function () { - createData(defaultGraphData); - val = yAxis.formatAxisLabel(num); - }); - - it('should return a string with suffix B', function () { - expect(val).to.be('1b'); - }); - }); - describe('getScaleType method', function () { var fnNames = ['linear', 'log', 'square root']; @@ -361,4 +347,4 @@ define(function (require) { }); }); }); -}); \ No newline at end of file +}); From ff3e5bc3998328b68c9fbac0d1023c5dfafda2b6 Mon Sep 17 00:00:00 2001 From: Spencer Alger Date: Fri, 1 May 2015 15:50:27 -0700 Subject: [PATCH 3/3] [vislib/yAxis] added tests for the tickLabel method --- test/unit/specs/vislib/lib/y_axis.js | 50 +++++++++++++++++++++++----- 1 file changed, 42 insertions(+), 8 deletions(-) diff --git a/test/unit/specs/vislib/lib/y_axis.js b/test/unit/specs/vislib/lib/y_axis.js index 043d2ae3b77c8..2784dcb5ffc26 100644 --- a/test/unit/specs/vislib/lib/y_axis.js +++ b/test/unit/specs/vislib/lib/y_axis.js @@ -6,6 +6,7 @@ define(function (require) { var YAxis; var Data; var el; + var buildYAxis; var yAxis; var yAxisDiv; @@ -70,14 +71,18 @@ define(function (require) { defaultYMin: true }); - yAxis = new YAxis({ - el: node, - yMin: dataObj.getYMin(), - yMax: dataObj.getYMax(), - _attr: { - margin: { top: 0, right: 0, bottom: 0, left: 0 } - } - }); + buildYAxis = function (params) { + return new YAxis(_.merge({}, params, { + el: node, + yMin: dataObj.getYMin(), + yMax: dataObj.getYMax(), + _attr: { + margin: { top: 0, right: 0, bottom: 0, left: 0 } + } + })); + }; + + yAxis = buildYAxis(); } describe('Vislib yAxis Class Test Suite', function () { @@ -346,5 +351,34 @@ define(function (require) { expect(yAxis.tickScale(20)).to.be(0); }); }); + + describe('#tickFormat()', function () { + var formatter = function () {}; + + it('returns a basic number formatter by default', function () { + var yAxis = buildYAxis(); + expect(yAxis.tickFormat()).to.not.be(formatter); + expect(yAxis.tickFormat()(1)).to.be('1'); + }); + + it('returns the yAxisFormatter when passed', function () { + var yAxis = buildYAxis({ + yAxisFormatter: formatter + }); + expect(yAxis.tickFormat()).to.be(formatter); + }); + + it('returns a percentage formatter when the vis is in percentage mode', function () { + var yAxis = buildYAxis({ + yAxisFormatter: formatter, + _attr: { + mode: 'percentage' + } + }); + + expect(yAxis.tickFormat()).to.not.be(formatter); + expect(yAxis.tickFormat()(1)).to.be('100%'); + }); + }); }); });