fixing based on CJs comments

elastic · Dec 5, 2016 · 02a22d0 · 02a22d0
1 parent 9bd80be
commit 02a22d0
Show file tree

Hide file tree

Showing 12 changed files with 148 additions and 57 deletions.
diff --git a/src/ui/public/vislib/__tests__/lib/vis_config.js b/src/ui/public/vislib/__tests__/lib/vis_config.js
@@ -0,0 +1,113 @@
+import d3 from 'd3';
+import _ from 'lodash';
+import ngMock from 'ng_mock';
+import expect from 'expect.js';
+import VislibLibVisConfigProvider from 'ui/vislib/lib/vis_config';
+import PersistedStatePersistedStateProvider from 'ui/persisted_state/persisted_state';
+
+describe('Vislib VisConfig Class Test Suite', function () {
+  let visConfig;
+  let el;
+  const data = {
+    hits: 621,
+    ordered: {
+      date: true,
+      interval: 30000,
+      max: 1408734982458,
+      min: 1408734082458
+    },
+    series: [
+      {
+        label: 'Count',
+        values: [
+          {
+            x: 1408734060000,
+            y: 8
+          },
+          {
+            x: 1408734090000,
+            y: 23
+          },
+          {
+            x: 1408734120000,
+            y: 30
+          },
+          {
+            x: 1408734150000,
+            y: 28
+          },
+          {
+            x: 1408734180000,
+            y: 36
+          },
+          {
+            x: 1408734210000,
+            y: 30
+          },
+          {
+            x: 1408734240000,
+            y: 26
+          },
+          {
+            x: 1408734270000,
+            y: 22
+          },
+          {
+            x: 1408734300000,
+            y: 29
+          },
+          {
+            x: 1408734330000,
+            y: 24
+          }
+        ]
+      }
+    ],
+    xAxisLabel: 'Date Histogram',
+    yAxisLabel: 'Count'
+  };
+
+  beforeEach(ngMock.module('kibana'));
+  beforeEach(ngMock.inject(function (Private) {
+    const VisConfig = Private(VislibLibVisConfigProvider);
+    const PersistedState = Private(PersistedStatePersistedStateProvider);
+    el = d3.select('body')
+      .attr('class', 'vis-wrapper')
+      .node();
+
+    visConfig = new VisConfig({
+      type: 'histogram',
+      el: el
+    }, data, new PersistedState());
+  }));
+
+  describe('get Method', function () {
+    it('should be a function', function () {
+      expect(_.isFunction(visConfig.set)).to.be(true);
+    });
+
+    it('should get the property', function () {
+      expect(visConfig.get('el')).to.be(el);
+      expect(visConfig.get('type')).to.be('histogram');
+    });
+
+    it('should return defaults if property does not exist', function () {
+      expect(visConfig.get('this.does.not.exist', 'defaults')).to.be('defaults');
+    });
+
+    it('should throw an error if property does not exist and defaults were not provided', function () {
+      expect(visConfig.get('this.does.not.exist')).to.throwError();
+    });
+  });
+
+  describe('set Method', function () {
+    it('should be a function', function () {
+      expect(_.isFunction(visConfig.set)).to.be(true);
+    });
+
+    it('should set a property', function () {
+      visConfig.set('this.does.not.exist', 'it.does.now');
+      expect(visConfig.get('this.does.not.exist')).to.be('it.does.now');
+    });
+  });
+});
diff --git a/src/ui/public/vislib/__tests__/visualizations/column_chart.js b/src/ui/public/vislib/__tests__/visualizations/column_chart.js
@@ -69,7 +69,7 @@ dataTypesArray.forEach(function (dataType, i) {
         });
       });
 
-      it('should append a d.y0 key to the data object', function () {
+      it('should not stack values if grouped mode is set', function () {
         if (mode === 'grouped') {
           expect(isStacked).to.be(false);
         } else {
@@ -93,17 +93,6 @@ dataTypesArray.forEach(function (dataType, i) {
       });
     });
 
-    describe('updateBars method', function () {
-      beforeEach(function () {
-        vis.handler.visConfig.set('mode', 'grouped');
-        vis.render(vis.data, persistedState);
-      });
-
-      it('should returned grouped bars', function () {
-        vis.handler.charts.forEach(function (chart) {});
-      });
-    });
-
     describe('addBarEvents method', function () {
       function checkChart(chart) {
         const rect = $(chart.chartEl).find('.series rect').get(0);

diff --git a/src/ui/public/vislib/__tests__/visualizations/tile_maps/map.js b/src/ui/public/vislib/__tests__/visualizations/tile_maps/map.js
@@ -16,7 +16,7 @@ import VislibVisualizationsMapProvider from 'ui/vislib/visualizations/_map';
 //   ['rows', require('fixtures/vislib/mock_data/geohash/_rows')],
 // ];
 
-//
+// TODO: Test the specific behavior of each these
 // const mapTypes = [
 //   'Scaled Circle Markers',
 //   'Shaded Circle Markers',

diff --git a/src/ui/public/vislib/components/zero_injection/inject_zeros.js b/src/ui/public/vislib/components/zero_injection/inject_zeros.js
@@ -20,7 +20,7 @@ export default function ZeroInjectionUtilService(Private) {
    */
 
   return function (obj, data, orderBucketsBySum = false) {
-    const keys = orderXValues(data, orderBucketsBySum); //_(obj).map('values').flatten().map('x').uniq().value();//
+    const keys = orderXValues(data, orderBucketsBySum);
 
     obj.forEach(function (series) {
       const zeroArray = createZeroFilledArray(keys, series.label);

diff --git a/src/ui/public/vislib/lib/axis/axis.js b/src/ui/public/vislib/lib/axis/axis.js
@@ -7,6 +7,7 @@ import AxisLabelsProvider from './axis_labels';
 import AxisScaleProvider from './axis_scale';
 import AxisConfigProvider from './axis_config';
 import errors from 'ui/errors';
+import SCALE_MODES from './scale_modes';
 
 export default function AxisFactory(Private) {
   const ErrorHandler = Private(ErrorHandlerProvider);
@@ -40,12 +41,6 @@ export default function AxisFactory(Private) {
         return d.y;
       })
       .offset(this.axisConfig.get('scale.offset', 'zero'));
-
-      if (this.axisConfig.get('scale.mode') === 'stacked'/* && visConfigArgs.type === 'histogram'*/) {
-        this.stack.out((d, y0, y) => {
-          return this._stackNegAndPosVals(d, y0, y);
-        });
-      }
     }
 
     /**
@@ -204,7 +199,7 @@ export default function AxisFactory(Private) {
       return Math.ceil(yTickScale(length));
     }
 
-    getLength(el, n) {
+    getLength(el) {
       if (this.axisConfig.isHorizontal()) {
         return $(el).width();
       } else {
@@ -214,7 +209,6 @@ export default function AxisFactory(Private) {
 
     adjustSize() {
       const config = this.axisConfig;
-      const xAxisPadding = 15;
       const style = config.get('style');
       const margin = this.visConfig.get('style.margin');
       const chartEl = this.visConfig.get('el');
@@ -268,12 +262,6 @@ export default function AxisFactory(Private) {
       if (this.axisConfig.isLogScale() && this.axisConfig.isPercentage()) {
         throw new errors.VislibError(`Can't mix percentage mode with log scale.`);
       }
-
-      const isWiggle = this.visConfig.get('mode', 'normal') === 'wiggle';
-      if (isWiggle && !this.axisConfig.isTimeDomain()) {
-        throw new errors.VislibError('In wiggle mode the area chart requires ordered values on the x-axis. ' +
-          'Try using a Histogram or Date Histogram aggregation.');
-      }
     }
 
     draw() {

diff --git a/src/ui/public/vislib/lib/axis/axis_config.js b/src/ui/public/vislib/lib/axis/axis_config.js
@@ -1,5 +1,7 @@
 import _ from 'lodash';
 import d3 from 'd3';
+import SCALE_MODES from './scale_modes';
+
 export default function AxisConfigFactory() {
 
   const defaults = {
@@ -15,7 +17,7 @@ export default function AxisConfigFactory() {
       defaultYExtents: null,
       min: null,
       max: null,
-      mode: 'normal' // [percentage, normal, wiggle, silhouette]
+      mode: SCALE_MODES.NORMAL
     },
     style: {
       color: '#ddd',
@@ -82,10 +84,12 @@ export default function AxisConfigFactory() {
         }
       }
 
-      if (this._values.type === 'value') {
-        const isWiggleOrSilluete = chartConfig.get('mode') === 'wiggle' || chartConfig.get('mode') === 'silhouette';
+      if (this.get('type') === 'value') {
+        const isWiggleOrSilhouette =
+          this.get('scale.mode') === SCALE_MODES.WIGGLE ||
+          this.get('scale.mode') === SCALE_MODES.SILHOUETTE;
         // if show was not explicitly set and wiggle or silhouette option was checked
-        if (isWiggleOrSilluete) {
+        if (isWiggleOrSilhouette) {
           this._values.scale.defaultYExtents = false;
 
           if (!axisConfigArgs.show) {
@@ -115,15 +119,11 @@ export default function AxisConfigFactory() {
       let offset;
       let stacked = true;
       switch (this.get('scale.mode')) {
-        case 'normal':
+        case SCALE_MODES.NORMAL:
           offset = 'zero';
           stacked = false;
           break;
-        case 'grouped':
-          offset = 'group';
-          stacked = false;
-          break;
-        case 'percentage':
+        case SCALE_MODES.PERCENTAGE:
           offset = 'expand';
           break;
         default:
@@ -160,7 +160,7 @@ export default function AxisConfigFactory() {
     };
 
     isPercentage() {
-      return this._values.scale.mode === 'percentage';
+      return this._values.scale.mode === SCALE_MODES.PERCENTAGE;
     };
 
     isUserDefined() {

diff --git a/src/ui/public/vislib/lib/axis/scale_modes.js b/src/ui/public/vislib/lib/axis/scale_modes.js
@@ -0,0 +1,9 @@
+const SCALE_MODES = {
+  NORMAL: 'normal',
+  PERCENTAGE: 'percentage',
+  WIGGLE: 'wiggle',
+  SILHOUETTE: 'silhouette',
+  ALL: ['normal', 'percentage', 'wiggle', 'silhouette']
+};
+
+export default SCALE_MODES;
diff --git a/src/ui/public/vislib/lib/dispatch.js b/src/ui/public/vislib/lib/dispatch.js
@@ -40,7 +40,7 @@ export default function DispatchClass(Private, config) {
       const slices = isSlices ? data.slices : undefined;
       const handler = this.handler;
       const color = _.get(handler, 'data.color');
-      const isPercentage = (handler && handler.visConfig.mode === 'percentage');
+      const isPercentage = (handler && handler.visConfig.get('mode') === 'percentage');
 
       const eventData = {
         value: d.y,

diff --git a/src/ui/public/vislib/lib/vis_config.js b/src/ui/public/vislib/lib/vis_config.js
@@ -1,13 +1,15 @@
+/**
+ * Provides vislib configuration, throws error if invalid property is accessed without providing defaults
+ */
 import _ from 'lodash';
 import VisTypesProvider from './types';
 import VislibLibDataProvider from './data';
 
-
 export default function VisConfigFactory(Private) {
 
   const Data = Private(VislibLibDataProvider);
   const visTypes = Private(VisTypesProvider);
-  const defaults = {
+  const DEFAULT_VIS_CONFIG = {
     style: {
       margin : { top: 10, right: 3, bottom: 5, left: 3 }
     },
@@ -21,8 +23,9 @@ export default function VisConfigFactory(Private) {
     constructor(visConfigArgs, data, uiState) {
       this.data = new Data(data, uiState);
 
-      const typeDefaults = visTypes[visConfigArgs.type](visConfigArgs, this.data);
-      this._values = _.defaultsDeep({}, typeDefaults, defaults);
+      const visType = visTypes[visConfigArgs.type];
+      const typeDefaults = visType(visConfigArgs, this.data);
+      this._values = _.defaultsDeep({}, typeDefaults, DEFAULT_VIS_CONFIG);
     };
 
     get(property, defaults) {

diff --git a/src/ui/public/vislib/visualizations/pie_chart.js b/src/ui/public/vislib/visualizations/pie_chart.js
@@ -12,8 +12,8 @@ export default function PieChartFactory(Private) {
     showTooltip: true,
     color: undefined,
     fillColor: undefined,
-    xValue: function (d) { return d.x; },
-    yValue: function (d) { return d.y; }
+    xValue: d => d.x,
+    yValue: d => d.y
   };
   /**
    * Pie Chart Visualization

diff --git a/src/ui/public/vislib/visualizations/point_series/column_chart.js b/src/ui/public/vislib/visualizations/point_series/column_chart.js
@@ -121,14 +121,6 @@ export default function ColumnChartFactory(Private) {
         if (d.y < 0) {
           return Math.abs(yScale(d.y0 + d.y) - yScale(d.y0));
         }
-        // todo:
-        // Due to an issue with D3 not returning zeros correctly when using
-        // an offset='expand', need to add conditional statement to handle zeros
-        // appropriately
-        //if (d._input.y === 0) {
-        //  return 0;
-        //}
-
         // for split bars or for one series,
         // last series will have d.y0 = 0
         if (d.y0 === 0 && yMin > 0) {

diff --git a/src/ui/public/vislib_vis_type/__tests__/_vislib_renderbot.js b/src/ui/public/vislib_vis_type/__tests__/_vislib_renderbot.js
@@ -94,13 +94,10 @@ describe('renderbot', function exportWrapper() {
       }, mockVisType)
     };
     let createVisSpy;
-    let getParamsStub;
     let renderbot;
 
     beforeEach(function () {
       createVisSpy = sinon.spy(VislibRenderbot.prototype, '_createVis');
-      // getParamsStub = sinon.stub(VislibRenderbot.prototype, '_getVislibParams', _identity);
-      // getParamsStub.returns(params);
       renderbot = new VislibRenderbot(vis, $el, persistedState);
     });