visualizations emit 'renderComplete' event when done rendering (elastic#9113)

elastic-jasper · ppisljar · commit 239332918cf4 · 2016-11-17T09:05:48.000+01:00
Backports PR elastic#8914 **Commit 1:** visualize emits renderComplete event once its done rendering * Original sha: 2195692 * Authored by ppisljar <peter.pisljar@gmail.com> on 2016-11-04T09:32:29Z **Commit 2:** fixing test * Original sha: 97a390a * Authored by ppisljar <peter.pisljar@gmail.com> on 2016-11-04T09:48:23Z **Commit 3:** adding renderComplete event to timelion * Original sha: b833873 * Authored by ppisljar <peter.pisljar@gmail.com> on 2016-11-10T15:14:16Z **Commit 4:** adding renderComplete event to timelion * Original sha: ab94eef * Authored by ppisljar <peter.pisljar@gmail.com> on 2016-11-11T09:20:32Z **Commit 5:** adding implementsRenderComplete property to vis * Original sha: b2ff874 * Authored by ppisljar <peter.pisljar@gmail.com> on 2016-11-16T06:50:19Z **Commit 6:** adding helper method on vis * Original sha: fc78562 * Authored by ppisljar <peter.pisljar@gmail.com> on 2016-11-16T18:21:46Z **Commit 7:** adding onRenderComplete method * Original sha: 451ab9a * Authored by ppisljar <peter.pisljar@gmail.com> on 2016-11-16T18:48:48Z
diff --git a/src/core_plugins/markdown_vis/public/__tests__/markdown_vis_controller.js b/src/core_plugins/markdown_vis/public/__tests__/markdown_vis_controller.js
@@ -8,16 +8,17 @@ describe('markdown vis controller', function () {
   beforeEach(ngMock.module('kibana/markdown_vis'));
   beforeEach(ngMock.inject(function ($rootScope, $controller) {
     $scope = $rootScope.$new();
+    $scope.vis = {
+      emit: function () {}
+    };
     $controller('KbnMarkdownVisController', {$scope: $scope});
     $scope.$digest();
   }));
 
   it('should set html from markdown params', function () {
     expect($scope).to.not.have.property('html');
-    $scope.vis = {
-      params: {
-        markdown: 'This is a test of the [markdown](http://daringfireball.net/projects/markdown) vis.'
-      }
+    $scope.vis.params = {
+      markdown: 'This is a test of the [markdown](http://daringfireball.net/projects/markdown) vis.'
     };
     $scope.$digest();
 
diff --git a/src/core_plugins/markdown_vis/public/markdown_vis.js b/src/core_plugins/markdown_vis/public/markdown_vis.js
@@ -24,7 +24,8 @@ function MarkdownVisProvider(Private) {
     params: {
       editor: markdownVisParamsTemplate
     },
-    requiresSearch: false
+    requiresSearch: false,
+    implementsRenderComplete: true,
   });
 }
 
diff --git a/src/core_plugins/markdown_vis/public/markdown_vis_controller.js b/src/core_plugins/markdown_vis/public/markdown_vis_controller.js
@@ -11,7 +11,9 @@ marked.setOptions({
 const module = uiModules.get('kibana/markdown_vis', ['kibana', 'ngSanitize']);
 module.controller('KbnMarkdownVisController', function ($scope) {
   $scope.$watch('vis.params.markdown', function (html) {
-    if (!html) return;
-    $scope.html = marked(html);
+    if (html) {
+      $scope.html = marked(html);
+    }
+    $scope.vis.emit('renderComplete');
   });
 });
diff --git a/src/core_plugins/metric_vis/public/metric_vis.js b/src/core_plugins/metric_vis/public/metric_vis.js
@@ -31,6 +31,7 @@ function MetricVisProvider(Private) {
       },
       editor: metricVisParamsTemplate
     },
+    implementsRenderComplete: true,
     schemas: new Schemas([
       {
         group: 'metrics',
diff --git a/src/core_plugins/metric_vis/public/metric_vis_controller.js b/src/core_plugins/metric_vis/public/metric_vis_controller.js
@@ -34,6 +34,7 @@ module.controller('KbnMetricVisController', function ($scope, Private) {
     if (resp) {
       metrics.length = 0;
       $scope.processTableGroups(tabifyAggResponse($scope.vis, resp));
+      $scope.vis.emit('renderComplete');
     }
   });
 });
diff --git a/src/core_plugins/table_vis/public/table_vis.js b/src/core_plugins/table_vis/public/table_vis.js
@@ -48,6 +48,7 @@ function TableVisTypeProvider(Private) {
       },
       editor: '<table-vis-params></table-vis-params>'
     },
+    implementsRenderComplete: true,
     hierarchicalData: function (vis) {
       return Boolean(vis.params.showPartialRows || vis.params.showMeticsAtAllLevels);
     },
diff --git a/src/core_plugins/table_vis/public/table_vis_controller.js b/src/core_plugins/table_vis/public/table_vis_controller.js
@@ -43,6 +43,8 @@ module.controller('KbnTableVisController', function ($scope, Private) {
         if (table.tables) return table.tables.some(haveRows);
         return table.rows.length > 0;
       });
+
+      $scope.vis.emit('renderComplete');
     }
 
     $scope.hasSomeRows = hasSomeRows;
diff --git a/src/core_plugins/timelion/public/panels/timechart/schema.js b/src/core_plugins/timelion/public/panels/timechart/schema.js
@@ -229,6 +229,10 @@ module.exports = function timechartFn(Private, config, $rootScope, timefilter, $
             setTimeout(drawPlot, 500);
           }
 
+          if ($scope.plot) {
+            $scope.$emit('renderComplete');
+          }
+
           legendScope.$destroy();
           legendScope = $scope.$new();
           // Used to toggle the series, and for displaying values on hover
diff --git a/src/core_plugins/timelion/public/vis/index.js b/src/core_plugins/timelion/public/vis/index.js
@@ -24,7 +24,8 @@ define(function (require) {
       params: {
         editor: require('plugins/timelion/vis/timelion_vis_params.html')
       },
-      requiresSearch: false
+      requiresSearch: false,
+      implementsRenderComplete: true,
     });
   }
 
diff --git a/src/core_plugins/timelion/public/vis/timelion_vis_controller.js b/src/core_plugins/timelion/public/vis/timelion_vis_controller.js
@@ -59,5 +59,10 @@ define(function (require) {
 
     $scope.$on('fetch', $scope.search);
 
+    $scope.$on('renderComplete', event => {
+      event.stopPropagation();
+      $scope.vis.emit('renderComplete');
+    });
+
   });
 });
diff --git a/src/ui/public/vis/vis.js b/src/ui/public/vis/vis.js
@@ -13,17 +13,21 @@ import AggTypesIndexProvider from 'ui/agg_types/index';
 import RegistryVisTypesProvider from 'ui/registry/vis_types';
 import VisAggConfigsProvider from 'ui/vis/agg_configs';
 import PersistedStateProvider from 'ui/persisted_state/persisted_state';
+import EventsProvider from 'ui/events';
+
 export default function VisFactory(Notifier, Private) {
   let aggTypes = Private(AggTypesIndexProvider);
   let visTypes = Private(RegistryVisTypesProvider);
   let AggConfigs = Private(VisAggConfigsProvider);
   const PersistedState = Private(PersistedStateProvider);
+  const EventEmitter = Private(EventsProvider);
 
   let notify = new Notifier({
     location: 'Vis'
   });
 
   function Vis(indexPattern, state, uiState) {
+    EventEmitter.call(this);
     state = state || {};
 
     if (_.isString(state)) {
@@ -36,6 +40,8 @@ export default function VisFactory(Notifier, Private) {
 
     this.setState(state);
     this.setUiState(uiState);
+
+    this.on('renderComplete', () => this._renderComplete = true);
   }
 
   Vis.convertOldState = function (type, oldState) {
@@ -79,6 +85,7 @@ export default function VisFactory(Notifier, Private) {
     };
   };
 
+  Vis.prototype = Object.create(EventEmitter.prototype);
   Vis.prototype.type = 'histogram';
 
   Vis.prototype.setState = function (state) {
@@ -162,6 +169,18 @@ export default function VisFactory(Notifier, Private) {
     return this.__uiState;
   };
 
+  Vis.prototype.implementsRenderComplete = function () {
+    return this.type.implementsRenderComplete;
+  };
+
+  Vis.prototype.onRenderComplete = function (cb) {
+    this.on('renderComplete', cb);
+    if (this._renderComplete) {
+      // guarantees that caller receives event in case visualization already finished rendering
+      this.emit('renderComplete');
+    }
+  };
+
   /**
    * Currently this is only used to extract map-specific information
    * (e.g. mapZoom, mapCenter).
diff --git a/src/ui/public/vis/vis_type.js b/src/ui/public/vis/vis_type.js
@@ -15,7 +15,12 @@ export default function VisTypeFactory(Private) {
     this.schemas = opts.schemas || new VisTypeSchemas();
     this.params = opts.params || {};
     this.requiresSearch = opts.requiresSearch == null ? true : opts.requiresSearch; // Default to true unless otherwise specified
+    this.implementsRenderComplete = false;
   }
 
+  VisType.prototype.createRenderbot = function (vis, $el, uiState) {
+    throw new Error('not implemented');
+  };
+
   return VisType;
 };
diff --git a/src/ui/public/vislib/lib/handler/handler.js b/src/ui/public/vislib/lib/handler/handler.js
@@ -185,9 +185,11 @@ export default function HandlerBaseClass(Private) {
 
         div.append('div').attr('class', 'item bottom');
         return div;
+      } else {
+        div.append('h4').text(message);
       }
-
-      return div.append('h4').text(message);
+      this.vis.emit('renderComplete');
+      return div;
     };
 
     /**
diff --git a/src/ui/public/vislib/vis.js b/src/ui/public/vislib/vis.js
@@ -61,7 +61,12 @@ export default function VisFactory(Private) {
 
       if (!this.uiState) {
         this.uiState = uiState;
-        uiState.on('change', this._uiStateChangeHandler = () => this.render(this.data, this.uiState));
+        this._uiStateChangeHandler = () => {
+          if (document.body.contains(this.el)) {
+            this.render(this.data, this.uiState);
+          }
+        };
+        uiState.on('change', this._uiStateChangeHandler);
       }
 
       this.handler = handlerTypes[chartType](this) || handlerTypes.column(this);
diff --git a/src/ui/public/vislib_vis_type/__tests__/_vislib_renderbot.js b/src/ui/public/vislib_vis_type/__tests__/_vislib_renderbot.js
@@ -77,7 +77,7 @@ describe('renderbot', function exportWrapper() {
     });
 
     it('should attach listeners and set vislibVis', function () {
-      expect(listenerSpy.callCount).to.be(3);
+      expect(listenerSpy.callCount).to.be(4);
       expect(listenerSpy.calledWith('test', _.noop)).to.be(true);
       expect(renderbot.vislibVis).to.be.a(Vis);
     });
diff --git a/src/ui/public/vislib_vis_type/vislib_renderbot.js b/src/ui/public/vislib_vis_type/vislib_renderbot.js
@@ -15,18 +15,22 @@ module.exports = function VislibRenderbotFactory(Private, $injector) {
   }
 
   VislibRenderbot.prototype._createVis = function () {
-    let self = this;
+    if (this.vislibVis) this.destroy();
 
-    if (self.vislibVis) self.destroy();
+    this.vislibParams = this._getVislibParams();
+    this.vislibVis = new vislib.Vis(this.$el[0], this.vislibParams);
 
-    self.vislibParams = self._getVislibParams();
-    self.vislibVis = new vislib.Vis(self.$el[0], self.vislibParams);
+    _.each(this.vis.listeners, (listener, event) => {
+      this.vislibVis.on(event, listener);
+    });
 
-    _.each(self.vis.listeners, function (listener, event) {
-      self.vislibVis.on(event, listener);
+    this.vislibVis.on('renderComplete', () => {
+      this.vis.emit('renderComplete');
     });
 
-    if (this.chartData) self.vislibVis.render(this.chartData, this.uiState);
+    if (this.chartData) {
+      this.vislibVis.render(this.chartData, this.uiState);
+    }
   };
 
   VislibRenderbot.prototype._getVislibParams = function () {
diff --git a/src/ui/public/vislib_vis_type/vislib_vis_type.js b/src/ui/public/vislib_vis_type/vislib_vis_type.js
@@ -26,6 +26,7 @@ export default function VislibVisTypeFactory(Private) {
     }
 
     this.listeners = opts.listeners || {};
+    this.implementsRenderComplete = true;
   }
 
   VislibVisType.prototype.createRenderbot = function (vis, $el, uiState) {
diff --git a/src/ui/public/visualize/visualize.js b/src/ui/public/visualize/visualize.js
@@ -144,7 +144,9 @@ uiModules
         }
 
         if (oldVis) $scope.renderbot = null;
-        if (vis) $scope.renderbot = vis.type.createRenderbot(vis, $visEl, $scope.uiState);
+        if (vis) {
+          $scope.renderbot = vis.type.createRenderbot(vis, $visEl, $scope.uiState);
+        }
       }));
 
       $scope.$watchCollection('vis.params', prereq(function () {
@@ -164,6 +166,7 @@ uiModules
         }).catch(notify.fatal);
 
         searchSource.onError(e => {
+          $scope.vis.emit('renderComplete');
           if (isTermSizeZeroError(e)) {
             return notify.error(
               `Your visualization ('${$scope.vis.title}') has an error: it has a term ` +

Original file line number	Diff line number	Diff line change
`@@ -24,7 +24,8 @@ function MarkdownVisProvider(Private) {`
`24`	`24`	`params: {`
`25`	`25`	`editor: markdownVisParamsTemplate`
`26`	`26`	`},`
`27`		`- requiresSearch: false`
	`27`	`+ requiresSearch: false,`
	`28`	`+ implementsRenderComplete: true,`
`28`	`29`	`});`
`29`	`30`	`}`
`30`	`31`
Original file line number	Diff line number	Diff line change
`@@ -31,6 +31,7 @@ function MetricVisProvider(Private) {`
`31`	`31`	`},`
`32`	`32`	`editor: metricVisParamsTemplate`
`33`	`33`	`},`
	`34`	`+ implementsRenderComplete: true,`
`34`	`35`	`schemas: new Schemas([`
`35`	`36`	`{`
`36`	`37`	`group: 'metrics',`
Original file line number	Diff line number	Diff line change
`@@ -34,6 +34,7 @@ module.controller('KbnMetricVisController', function ($scope, Private) {`
`34`	`34`	`if (resp) {`
`35`	`35`	`metrics.length = 0;`
`36`	`36`	`$scope.processTableGroups(tabifyAggResponse($scope.vis, resp));`
	`37`	`+ $scope.vis.emit('renderComplete');`
`37`	`38`	`}`
`38`	`39`	`});`
`39`	`40`	`});`
Original file line number	Diff line number	Diff line change
`@@ -43,6 +43,8 @@ module.controller('KbnTableVisController', function ($scope, Private) {`
`43`	`43`	`if (table.tables) return table.tables.some(haveRows);`
`44`	`44`	`return table.rows.length > 0;`
`45`	`45`	`});`
	`46`	`+`
	`47`	`+ $scope.vis.emit('renderComplete');`
`46`	`48`	`}`
`47`	`49`
`48`	`50`	`$scope.hasSomeRows = hasSomeRows;`
Original file line number	Diff line number	Diff line change
`@@ -24,7 +24,8 @@ define(function (require) {`
`24`	`24`	`params: {`
`25`	`25`	`editor: require('plugins/timelion/vis/timelion_vis_params.html')`
`26`	`26`	`},`
`27`		`- requiresSearch: false`
	`27`	`+ requiresSearch: false,`
	`28`	`+ implementsRenderComplete: true,`
`28`	`29`	`});`
`29`	`30`	`}`
`30`	`31`