Addressing comments left on pull request #281: https://github.com/elasticsearch/kibana4/pull/281

src/kibana/apps/visualize/editor/nesting_indicator.js

@@ -8,7 +8,7 @@ define(function (require) {
 
     var getColor = (function () {
       var i = 0;
-      var colorPool = Private(require('components/vislib/components/_functions/color/color_palette'))(100);
+      var colorPool = Private(require('components/vislib/components/color/color_palette'))(100);
       var assigned = {};
       return function (item) {
         var key = item.$$hashKey;

src/kibana/components/vislib/components/_functions/color/color.js

@@ -1,23 +0,0 @@
-define(function (require) {
-  return function ColorUtilService(Private) {
-    var _ = require('lodash');
-
-    var createColorPalette = Private(require('components/vislib/components/_functions/color/color_palette'));
-    var createColorObj = Private(require('components/vislib/components/_functions/color/color_obj'));
-
-    // Takes an array of strings or numbers
-    return function (arr) {
-      if (!_.isArray(arr)) {
-        throw new Error(typeof arr + ' should be an array of strings or numbers');
-      }
-
-      var colorObj = createColorObj(arr, createColorPalette(arr.length));
-
-      // Returns a function that accepts a value (i.e. a string or number)
-      // and returns a hex color from the colorObj
-      return function (value) {
-        return colorObj[value];
-      };
-    };
-  };
-});

src/kibana/components/vislib/components/_functions/color/color_obj.js

@@ -1,12 +0,0 @@
-define(function (require) {
-  return function ColorObjUtilService() {
-    var _ = require('lodash');
-
-    // Accepts 2 arrays of strings or numbers
-    return function (arr1, arr2) {
-      // Returns an object with arr1 values as keys
-      // and arr2 values as values
-      return _.zipObject(arr1, arr2);
-    };
-  };
-});

src/kibana/components/vislib/components/_functions/zero_injection/replace_index.js

@@ -1,18 +0,0 @@
-define(function () {
-  return function ReplaceIndexUtilService() {
-    /*
-     * Replaces an object in an array at a specific index
-     *
-     * Accepts an array of objects
-     * an index (num)
-     * and an obj
-     */
-    return function (arr, index, obj) {
-      arr.splice(index, 1);
-      arr.splice(index, 0, obj);
-
-      // Returns an array with a replaced object
-      return arr;
-    };
-  };
-});

src/kibana/components/vislib/components/color/color.js

@@ -0,0 +1,28 @@
+define(function (require) {
+  return function ColorUtilService(Private) {
+    var _ = require('lodash');
+
+    var createColorPalette = Private(require('components/vislib/components/color/color_palette'));
+
+    /*
+     * Accepts an array of strings or numbers that are used to create a
+     * a lookup table that associates the values (key) with a hex color (value).
+     * Returns a function that accepts a value (i.e. a string or number)
+     * and returns a hex color associated with that value
+     */
+
+    return function (arrayOfStringsOrNumbers) {
+      // Takes an array of strings or numbers
+      if (!_.isArray(arrayOfStringsOrNumbers)) {
+        throw new Error('ColorUtil expects an array of strings or numbers');
+      }
+
+      // Creates lookup table of values (keys) and hex colors (values).
+      var colorObj = _.zipObject(arrayOfStringsOrNumbers, createColorPalette(arrayOfStringsOrNumbers.length));
+
+      return function (value) {
+        return colorObj[value];
+      };
+    };
+  };
+});

src/kibana/components/vislib/components/color/color_palette.js

@@ -2,7 +2,7 @@ define(function (require) {
   return function ColorPaletteUtilService(d3, Private) {
     var _ = require('lodash');
 
-    var seedColors = Private(require('components/vislib/components/_functions/color/seed_colors'));
+    var seedColors = Private(require('components/vislib/components/color/seed_colors'));
 
     // Accepts a number that represents a length of an array
     return function (num) {

src/kibana/components/vislib/components/color/seed_colors.js

@@ -1,4 +1,9 @@
 define(function () {
+  /*
+   * Using a random color generator presented awful colors and unpredictable color schemes.
+   * So we needed to come up with one of our own that creates consistent, pleasing color patterns.
+   * The order allows us to guarantee that 1st, 2nd, 3rd, etc values always get the same color.
+   */
   return function SeedColorUtilService() {
     // returns an array of 72 seed colors
     return [

src/kibana/components/vislib/components/labels/data_array.js

@@ -2,7 +2,7 @@ define(function (require) {
   return function GetArrayUtilService(Private) {
     var _ = require('lodash');
 
-    var flattenSeries = Private(require('components/vislib/components/_functions/labels/get_series'));
+    var flattenSeries = Private(require('components/vislib/components/labels/flatten_series'));
 
     /* Takes a kibana obj object
      * for example:

src/kibana/components/vislib/components/labels/flatten_series.js

@@ -18,7 +18,6 @@ define(function (require) {
        */
 
       return _.chain(obj)
-        .pluck()
         .pluck('series')
         .flatten()
         .value();

src/kibana/components/vislib/components/labels/labels.js

@@ -1,7 +1,9 @@
 define(function (require) {
   return function LabelUtilService(Private) {
-    var getArr = Private(require('components/vislib/components/_functions/labels/data_array'));
-    var getArrOfUniqLabels = Private(require('components/vislib/components/_functions/labels/uniq_labels'));
+    var _ = require('lodash');
+
+    var getArr = Private(require('components/vislib/components/labels/data_array'));
+    var getArrOfUniqLabels = Private(require('components/vislib/components/labels/uniq_labels'));
 
     /* Takes a kibana data object
      * for example:
@@ -14,8 +16,8 @@ define(function (require) {
      * Data object can have a key that has rows, columns, or series.
      */
     return function (obj) {
-      if (!obj instanceof Object) {
-        throw new Error(obj + ' should be an object');
+      if (!_.isObject(obj)) {
+        throw new Error('LabelUtil expects an object');
       }
 
       // Returns an array of unique chart labels

src/kibana/components/vislib/components/labels/uniq_labels.js

@@ -4,8 +4,8 @@ define(function (require) {
 
     // Takes an array of objects
     return function (arr) {
-      if (!arr instanceof Array) {
-        throw TypeError(arr + ' should be an array of objects');
+      if (!_.isArray(arr)) {
+        throw TypeError('UniqLabelUtil expects an array of objects');
       }
 
       // Returns a array of unique chart labels

src/kibana/components/vislib/components/layouts/splits/column_chart/chart_split.js

@@ -10,7 +10,13 @@ define(function () {
         var div = d3.select(this)
           .attr('class', function () {
             // Determine the parent class
-            return data.rows ? 'chart-wrapper-row' : data.columns ? 'chart-wrapper-column' : 'chart-wrapper';
+            if (data.rows) {
+              return 'chart-wrapper-row';
+            } else if (data.columns) {
+              return 'chart-wrapper-column';
+            } else {
+              return 'chart-wrapper';
+            }
           });
         var divClass;
 
@@ -18,10 +24,19 @@ define(function () {
           .append('div')
           .data(function (d) {
             // Determine the child class
-            divClass = d.rows ? 'chart-row' : d.columns ? 'chart-column' : 'chart';
-            return d.rows ? d.rows : d.columns ? d.columns : [d];
+            if (d.rows) {
+              divClass = 'chart-row';
+              return d.rows;
+            } else if (d.columns) {
+              divClass = 'chart-column';
+              return d.columns;
+            } else {
+              divClass = 'chart';
+              return [d];
+            }
           })
-          .enter().append('div')
+          .enter()
+          .append('div')
           .attr('class', function () {
             return divClass;
           });

src/kibana/components/vislib/components/layouts/splits/column_chart/chart_title_split.js

@@ -11,11 +11,13 @@ define(function () {
         var div = d3.select(this);
 
         if (!data.series) {
-          div.selectAll('.chart-title').append('div')
+          div.selectAll('.chart-title')
+            .append('div')
             .data(function (d) {
               return d.rows ? d.rows : d.columns;
             })
-            .enter().append('div')
+            .enter()
+            .append('div')
             .attr('class', 'chart-title');
 
           if (data.rows) {

src/kibana/components/vislib/components/zero_injection/inject_zeros.js

@@ -2,9 +2,9 @@ define(function (require) {
   return function ZeroInjectionUtilService(Private) {
     var _ = require('lodash');
 
-    var orderXValues = Private(require('components/vislib/components/_functions/zero_injection/ordered_x_keys'));
-    var createZeroFilledArray = Private(require('components/vislib/components/_functions/zero_injection/zero_filled_array'));
-    var zeroFillDataArray = Private(require('components/vislib/components/_functions/zero_injection/zero_fill_data_array'));
+    var orderXValues = Private(require('components/vislib/components/zero_injection/ordered_x_keys'));
+    var createZeroFilledArray = Private(require('components/vislib/components/zero_injection/zero_filled_array'));
+    var zeroFillDataArray = Private(require('components/vislib/components/zero_injection/zero_fill_data_array'));
 
     // Takes the kibana data objects
     return function (obj) {

src/kibana/components/vislib/components/zero_injection/ordered_x_keys.js

@@ -1,7 +1,7 @@
 define(function (require) {
   return function OrderedXKeysUtilService(Private) {
     var _ = require('lodash');
-    var getUniqKeys = Private(require('components/vislib/components/_functions/zero_injection/uniq_keys'));
+    var getUniqKeys = Private(require('components/vislib/components/zero_injection/uniq_keys'));
 
     // Takes a kibana data objects
     return function (obj) {

src/kibana/components/vislib/components/zero_injection/uniq_keys.js

@@ -2,7 +2,7 @@ define(function (require) {
   return function UniqueXValuesUtilService(Private) {
     var _ = require('lodash');
 
-    var flattenDataArray = Private(require('components/vislib/components/_functions/zero_injection/flatten_data'));
+    var flattenDataArray = Private(require('components/vislib/components/zero_injection/flatten_data'));
 
     // accepts a kibana data.series array of objects
     return function (obj) {

src/kibana/components/vislib/components/zero_injection/zero_fill_data_array.js

@@ -2,8 +2,6 @@ define(function (require) {
   return function ZeroFillDataArrayUtilService(Private) {
     var _ = require('lodash');
 
-    var replaceIndex = Private(require('components/vislib/components/_functions/zero_injection/replace_index'));
-
     // Accepts an array of zero-filled y value objects
     // and a kibana data.series[i].values array of objects
     return function (arr1, arr2) {
@@ -18,7 +16,7 @@ define(function (require) {
       for (i = 0; i < max; i++) {
         val = arr2[i];
         index = _.findIndex(arr1, getX);
-        replaceIndex(arr1, index, val);
+        arr1.splice(index, 1, val);
       }
 
       // Return a zero-filled array of objects

src/kibana/components/vislib/lib/data.js

@@ -2,10 +2,10 @@ define(function (require) {
   return function DataFactory(d3, Private) {
     var _ = require('lodash');
 
-    var injectZeros = Private(require('components/vislib/components/_functions/zero_injection/inject_zeros'));
-    var orderKeys = Private(require('components/vislib/components/_functions/zero_injection/ordered_x_keys'));
-    var getLabels = Private(require('components/vislib/components/_functions/labels/labels'));
-    var color = Private(require('components/vislib/components/_functions/color/color'));
+    var injectZeros = Private(require('components/vislib/components/zero_injection/inject_zeros'));
+    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'));
 
     /*
      * Provides an API for pulling values off the data

src/kibana/components/vislib/lib/legend.js

@@ -1,6 +1,7 @@
 define(function (require) {
   return function LegendFactory(d3, Private) {
     var _ = require('lodash');
+    var legendHeaderTemplate = _.template(require('text!components/vislib/partials/legend_header.html'));
 
     // Dynamically adds css file
     require('css!components/vislib/components/styles/main');
@@ -39,13 +40,8 @@ define(function (require) {
         .attr('class', 'header')
         .append('div')
         .attr('class', 'column-labels')
-        .html(function (d) {
-          if (args._attr.isOpen) {
-            return '<span class="btn btn-xs btn-default legend-toggle">' +
-              '<i class="fa fa-chevron-right"></i></span>';
-          }
-          return '<span class="btn btn-xs btn-default legend-toggle">' +
-            '<i class="fa fa-chevron-left"></i></span>';
+        .html(function () {
+          return legendHeaderTemplate(args._attr);
         });
     };
 

src/kibana/components/vislib/lib/x_axis.js

@@ -90,7 +90,6 @@ define(function (require) {
 
     // Create the d3 xAxis function
     XAxis.prototype.getXAxis = function (width) {
-      this.xAxisFormatter = this.xAxisFormatter;
       // save a reference to the xScale
       this.xScale = this.getXScale(this.ordered, width);
 
@@ -216,7 +215,7 @@ define(function (require) {
         // truncate str
         selection.selectAll('.tick text')
           .text(function (d) {
-            str = self.xAxisFormatter(d);
+            str = d;
             if (maxWidth > size) {
               endChar = 0;
               if (Math.floor((size / pixPerChar) - 4) >= 4) {

src/kibana/components/vislib/partials/legend_header.html

@@ -0,0 +1,3 @@
+<span class="btn btn-xs btn-default legend-toggle">
+  <i class="fa fa-chevron-<%= (isOpen) ? 'right' : 'left' %>"></i>
+</span>

test/unit/specs/vislib/color.js

@@ -22,8 +22,8 @@ define(function (require) {
 
       beforeEach(function () {
         inject(function (d3, Private) {
-          seedColors = Private(require('components/vislib/components/_functions/color/seed_colors'));
-          getColors = Private(require('components/vislib/components/_functions/color/color'));
+          seedColors = Private(require('components/vislib/components/color/seed_colors'));
+          getColors = Private(require('components/vislib/components/color/color'));
 //          error = getColors(str);
           color = getColors(arr);
         });
@@ -62,36 +62,6 @@ define(function (require) {
       });
     });
 
-    describe('Color Object', function () {
-      var createColorObj;
-      var arr1 = ['rashid', 'juan', 'chris', 'spencer'];
-      var arr2 = ['guru', 'datavis', 'architect', 'javascript'];
-      var dict;
-
-      beforeEach(function () {
-        module('ColorObjUtilService');
-      });
-
-      beforeEach(function () {
-        inject(function (d3, Private) {
-          createColorObj = Private(require('components/vislib/components/_functions/color/color_obj'));
-          dict = createColorObj(arr1, arr2);
-        });
-      });
-
-      it('should be a function', function () {
-        expect(typeof createColorObj).to.be('function');
-      });
-
-      it('should return an object', function () {
-        expect(dict instanceof Object).to.be(true);
-      });
-
-      it('should return the correct value', function () {
-        expect(dict[arr1[0]]).to.be(arr2[0]);
-      });
-    });
-
     describe('Color Palette', function () {
       var num1 = 45;
       var num2 = 72;
@@ -105,7 +75,7 @@ define(function (require) {
 
       beforeEach(function () {
         inject(function (d3, Private) {
-          createColorPalette = Private(require('components/vislib/components/_functions/color/color_palette'));
+          createColorPalette = Private(require('components/vislib/components/color/color_palette'));
           colorPalette = createColorPalette(num1);
         });
       });

test/unit/specs/vislib/labels.js

@@ -116,7 +116,7 @@ define(function (require) {
 
       beforeEach(function () {
         inject(function (d3, Private) {
-          getLabels = Private(require('components/vislib/components/_functions/labels/labels'));
+          getLabels = Private(require('components/vislib/components/labels/labels'));
           seriesLabels = getLabels(seriesData);
           rowsLabels = getLabels(rowsData);
           seriesArr = _.isArray(seriesLabels);
@@ -142,9 +142,10 @@ define(function (require) {
         expect(rowsArr).to.be(true);
       });
 
-      it('should return empty array if input is not an object', function () {
-        error = getLabels('string not object');
-        expect(error.length).to.be(0);
+      it('should throw an error if input is not an object', function () {
+        expect(function () {
+          getLabels('string not object');
+        }).to.throwError();
       });
 
       it('should return unique label values', function () {
@@ -166,7 +167,7 @@ define(function (require) {
 
       beforeEach(function () {
         inject(function (d3, Private) {
-          dataArray = Private(require('components/vislib/components/_functions/labels/data_array'));
+          dataArray = Private(require('components/vislib/components/labels/data_array'));
           seriesLabels = dataArray(seriesData);
           rowsLabels = dataArray(rowsData);
           testSeries = _.isArray(seriesLabels);
@@ -222,7 +223,7 @@ define(function (require) {
 
       beforeEach(function () {
         inject(function (d3, Private) {
-          uniqLabels = Private(require('components/vislib/components/_functions/labels/uniq_labels'));
+          uniqLabels = Private(require('components/vislib/components/labels/uniq_labels'));
           uniq = uniqLabels(arrObj);
           testArr = _.isArray(uniq);
         });
@@ -258,7 +259,7 @@ define(function (require) {
 
       beforeEach(function () {
         inject(function (d3, Private) {
-          getSeries = Private(require('components/vislib/components/_functions/labels/get_series'));
+          getSeries = Private(require('components/vislib/components/labels/flatten_series'));
           columnsLabels = getSeries(columnsData);
           rowsLabels = getSeries(rowsData);
           seriesLabels = getSeries(seriesData);

test/unit/specs/vislib/zero_injection.js

@@ -58,7 +58,7 @@ define(function (require) {
 
       beforeEach(function () {
         inject(function (Private) {
-          injectZeros = Private(require('components/vislib/components/_functions/zero_injection/inject_zeros'));
+          injectZeros = Private(require('components/vislib/components/zero_injection/inject_zeros'));
           sample1 = injectZeros(seriesData);
           sample2 = injectZeros(multiSeriesData);
         });
@@ -113,7 +113,7 @@ define(function (require) {
 
       beforeEach(function () {
         inject(function (Private) {
-          orderXValues = Private(require('components/vislib/components/_functions/zero_injection/ordered_x_keys'));
+          orderXValues = Private(require('components/vislib/components/zero_injection/ordered_x_keys'));
           results = orderXValues(multiSeriesData);
         });
       });
@@ -145,7 +145,7 @@ define(function (require) {
 
       beforeEach(function () {
         inject(function (Private) {
-          uniqueKeys = Private(require('components/vislib/components/_functions/zero_injection/uniq_keys'));
+          uniqueKeys = Private(require('components/vislib/components/zero_injection/uniq_keys'));
           results = uniqueKeys(multiSeriesData.series);
         });
       });
@@ -163,41 +163,6 @@ define(function (require) {
       });
     });
 
-    describe('Replace Index', function () {
-      var replaceIndex;
-      var arr = [
-        { x: 1, y: 2},
-        { x: 2, y: 3},
-        { x: 3, y: 4}
-      ];
-      var index = 1;
-      var obj = { x: 2, y: 5 };
-      var results;
-
-      beforeEach(function () {
-        module('ReplaceIndexUtilService');
-      });
-
-      beforeEach(function () {
-        inject(function (Private) {
-          replaceIndex = Private(require('components/vislib/components/_functions/zero_injection/replace_index'));
-          results = replaceIndex(arr, index, obj);
-        });
-      });
-
-      it('should return a function', function () {
-        expect(_.isFunction(replaceIndex)).to.be(true);
-      });
-
-      it('should return an array', function () {
-        expect(_.isArray(results)).to.be(true);
-      });
-
-      it('should replace the object at the index in the array with the new object', function () {
-        expect(results[1].y).to.be(5);
-      });
-    });
-
     describe('Flatten Data', function () {
       var flattenData;
       var results;
@@ -208,7 +173,7 @@ define(function (require) {
 
       beforeEach(function () {
         inject(function (Private) {
-          flattenData = Private(require('components/vislib/components/_functions/zero_injection/flatten_data'));
+          flattenData = Private(require('components/vislib/components/zero_injection/flatten_data'));
           results = flattenData(multiSeriesData);
         });
       });
@@ -241,7 +206,7 @@ define(function (require) {
 
       beforeEach(function () {
         inject(function (Private) {
-          createZeroArray = Private(require('components/vislib/components/_functions/zero_injection/zero_filled_array'));
+          createZeroArray = Private(require('components/vislib/components/zero_injection/zero_filled_array'));
           results1 = createZeroArray(arr1);
           results2 = createZeroArray(arr2);
         });
@@ -293,7 +258,7 @@ define(function (require) {
       var xValueArr = [1, 2, 3, 4, 5];
       var createZeroArray;
       var arr1;
-      var arr2 = multiSeriesData.series[2].values;
+      var arr2 = [ {x: 3, y: 834} ];
       var results;
 
       beforeEach(function () {
@@ -302,8 +267,8 @@ define(function (require) {
 
       beforeEach(function () {
         inject(function (Private) {
-          zeroFillArray = Private(require('components/vislib/components/_functions/zero_injection/zero_fill_data_array'));
-          createZeroArray = Private(require('components/vislib/components/_functions/zero_injection/zero_filled_array'));
+          zeroFillArray = Private(require('components/vislib/components/zero_injection/zero_fill_data_array'));
+          createZeroArray = Private(require('components/vislib/components/zero_injection/zero_filled_array'));
           arr1 = createZeroArray(xValueArr);
           // Takes zero array as 1st arg and data array as 2nd arg
           results = zeroFillArray(arr1, arr2);
@@ -327,7 +292,7 @@ define(function (require) {
       it('should return an array with zeros injected in the appropriate objects as y values', function () {
         expect(results[0].y).to.be(0);
         expect(results[1].y).to.be(0);
-        expect(results[2].y).to.be(0);
+        expect(results[3].y).to.be(0);
         expect(results[4].y).to.be(0);
       });
     });