Addressing comments left on pull request #281: https://github.com/ela…

…sticsearch/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/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/_functions/color/color_palette.js → 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/_functions/color/seed_colors.js → 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/_functions/labels/data_array.js → 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/_functions/labels/get_series.js → 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/_functions/labels/labels.js → 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/_functions/labels/uniq_labels.js → 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,18 +10,33 @@ 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;
 
         var charts = div.selectAll('charts')
           .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/_functions/zero_injection/inject_zeros.js → 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/_functions/zero_injection/ordered_x_keys.js → 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/_functions/zero_injection/uniq_keys.js → 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/_functions/zero_injection/zero_fill_data_array.js → 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);
         });
       });