zeppelin-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From m...@apache.org
Subject zeppelin git commit: [ZEPPELIN-2234] Can't display the same chart again (master)
Date Sat, 11 Mar 2017 03:26:54 GMT
Repository: zeppelin
Updated Branches:
  refs/heads/master 44b395f90 -> ce97b5343


[ZEPPELIN-2234] Can't display the same chart again (master)

### What is this PR for?

Can't display the same chart again. I attached a screenshot.

- This should be backported into 0.7.0 as well.

#### Implementation Details

After https://github.com/apache/zeppelin/pull/2092,

- result.html will draw chart every time since we use `ng-if` instead of `ng-show`
- that means DOM is deleted, and created too
- so we have to create visualization instance every time which requires a newly created DOM.

```js
builtInViz.instance = new Visualization(loadedElem, config); // `loadedElem` is the newly
created DOM.
```

### What type of PR is it?
[Bug Fix]

### Todos

NONE

### What is the Jira issue?
* Open an issue on Jira https://issues.apache.org/jira/browse/ZEPPELIN/
* Put link here, and add [ZEPPELIN-*Jira number*] in PR title, eg. [ZEPPELIN-533]

### How should this be tested?

I attached a screenshot

### Screenshots (if appropriate)

##### Before: buggy

![2234](https://cloud.githubusercontent.com/assets/4968473/23694278/4451594e-041c-11e7-9971-f0bb5945a1be.gif)

##### After: fixed

![2234-2](https://cloud.githubusercontent.com/assets/4968473/23694270/34866ba8-041c-11e7-83a8-693a93646fa4.gif)

### Questions:
* Does the licenses files need update? - NO
* Is there breaking changes for older versions? - NO
* Does this needs documentation? - NO

Author: 1ambda <1amb4a@gmail.com>

Closes #2110 from 1ambda/ZEPPELIN-2234/cant-display-same-chart-again and squashes the following
commits:

14ee617 [1ambda] fix: wait until DOM is loaded in renderDefaultDisplay func
42b5529 [1ambda] fix: Revert #2092


Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo
Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/ce97b534
Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/ce97b534
Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/ce97b534

Branch: refs/heads/master
Commit: ce97b53431b1b372475ebfbb330ce147f37bfef2
Parents: 44b395f
Author: 1ambda <1amb4a@gmail.com>
Authored: Fri Mar 10 15:33:19 2017 +0900
Committer: Lee moon soo <moon@apache.org>
Committed: Fri Mar 10 19:26:50 2017 -0800

----------------------------------------------------------------------
 .../paragraph/result/result.controller.js       | 142 +++++++++----------
 .../app/notebook/paragraph/result/result.html   |   6 +-
 2 files changed, 68 insertions(+), 80 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/zeppelin/blob/ce97b534/zeppelin-web/src/app/notebook/paragraph/result/result.controller.js
----------------------------------------------------------------------
diff --git a/zeppelin-web/src/app/notebook/paragraph/result/result.controller.js b/zeppelin-web/src/app/notebook/paragraph/result/result.controller.js
index d3a81d4..1c452c4 100644
--- a/zeppelin-web/src/app/notebook/paragraph/result/result.controller.js
+++ b/zeppelin-web/src/app/notebook/paragraph/result/result.controller.js
@@ -266,20 +266,24 @@ function ResultCtrl($scope, $rootScope, $route, $window, $routeParams,
$location
   };
 
   $scope.renderDefaultDisplay = function(targetElemId, type, data, refresh) {
-    if (type === DefaultDisplayType.TABLE) {
-      $scope.renderGraph(targetElemId, $scope.graphMode, refresh);
-    } else if (type === DefaultDisplayType.HTML) {
-      renderHtml(targetElemId, data);
-    } else if (type === DefaultDisplayType.ANGULAR) {
-      renderAngular(targetElemId, data);
-    } else if (type === DefaultDisplayType.TEXT) {
-      renderText(targetElemId, data);
-    } else if (type === DefaultDisplayType.ELEMENT) {
-      renderElem(targetElemId, data);
-    } else {
-      console.error(`Unknown Display Type: ${type}`);
+    const afterLoaded = () => {
+      if (type === DefaultDisplayType.TABLE) {
+        renderGraph(targetElemId, $scope.graphMode, refresh);
+      } else if (type === DefaultDisplayType.HTML) {
+        renderHtml(targetElemId, data);
+      } else if (type === DefaultDisplayType.ANGULAR) {
+        renderAngular(targetElemId, data);
+      } else if (type === DefaultDisplayType.TEXT) {
+        renderText(targetElemId, data);
+      } else if (type === DefaultDisplayType.ELEMENT) {
+        renderElem(targetElemId, data);
+      } else {
+        console.error(`Unknown Display Type: ${type}`);
+      }
     }
 
+    retryUntilElemIsLoaded(targetElemId, afterLoaded);
+
     // send message to parent that this result is rendered
     const paragraphId = $scope.$parent.paragraph.id;
     $scope.$emit('resultRendered', paragraphId);
@@ -377,50 +381,38 @@ function ResultCtrl($scope, $rootScope, $route, $window, $routeParams,
$location
   };
 
   const renderElem = function(targetElemId, data) {
-    const afterLoaded = () => {
-      const elem = angular.element(`#${targetElemId}`);
-      handleData(() => { data(targetElemId) }, DefaultDisplayType.ELEMENT,
-        () => {}, /** HTML element will be filled with data. thus pass empty success callback
*/
-        (error) => { elem.html(`${error.stack}`); }
-      );
-    };
-
-    retryUntilElemIsLoaded(targetElemId, afterLoaded);
+    const elem = angular.element(`#${targetElemId}`);
+    handleData(() => { data(targetElemId) }, DefaultDisplayType.ELEMENT,
+      () => {}, /** HTML element will be filled with data. thus pass empty success callback
*/
+      (error) => { elem.html(`${error.stack}`); }
+    );
   };
 
   const renderHtml = function(targetElemId, data) {
-    const afterLoaded = () => {
-      const elem = angular.element(`#${targetElemId}`);
-      handleData(data, DefaultDisplayType.HTML,
-        (generated) => {
-          elem.html(generated);
-          elem.find('pre code').each(function(i, e) {
-            hljs.highlightBlock(e);
-          });
-          /*eslint new-cap: [2, {"capIsNewExceptions": ["MathJax.Hub.Queue"]}]*/
-          MathJax.Hub.Queue(['Typeset', MathJax.Hub, elem[0]]);
-        },
-        (error) => {  elem.html(`${error.stack}`); }
-      );
-    };
-
-    retryUntilElemIsLoaded(targetElemId, afterLoaded);
+    const elem = angular.element(`#${targetElemId}`);
+    handleData(data, DefaultDisplayType.HTML,
+      (generated) => {
+        elem.html(generated);
+        elem.find('pre code').each(function(i, e) {
+          hljs.highlightBlock(e);
+        });
+        /*eslint new-cap: [2, {"capIsNewExceptions": ["MathJax.Hub.Queue"]}]*/
+        MathJax.Hub.Queue(['Typeset', MathJax.Hub, elem[0]]);
+      },
+      (error) => {  elem.html(`${error.stack}`); }
+    );
   };
 
   const renderAngular = function(targetElemId, data) {
-    const afterLoaded = () => {
-      const elem = angular.element(`#${targetElemId}`);
-      const paragraphScope = noteVarShareService.get(`${paragraph.id}_paragraphScope`);
-      handleData(data, DefaultDisplayType.ANGULAR,
-        (generated) => {
-          elem.html(generated);
-          $compile(elem.contents())(paragraphScope);
-        },
-        (error) => {  elem.html(`${error.stack}`); }
-      );
-    };
-
-    retryUntilElemIsLoaded(targetElemId, afterLoaded);
+    const elem = angular.element(`#${targetElemId}`);
+    const paragraphScope = noteVarShareService.get(`${paragraph.id}_paragraphScope`);
+    handleData(data, DefaultDisplayType.ANGULAR,
+      (generated) => {
+        elem.html(generated);
+        $compile(elem.contents())(paragraphScope);
+      },
+      (error) => {  elem.html(`${error.stack}`); }
+    );
   };
 
   const getTextResultElemId = function (resultId) {
@@ -428,25 +420,21 @@ function ResultCtrl($scope, $rootScope, $route, $window, $routeParams,
$location
   };
 
   const renderText = function(targetElemId, data) {
-    const afterLoaded = () => {
-      const elem = angular.element(`#${targetElemId}`);
-      handleData(data, DefaultDisplayType.TEXT,
-        (generated) => {
-          // clear all lines before render
-          removeChildrenDOM(targetElemId);
-
-          if (generated) {
-            const divDOM = angular.element('<div></div>').text(generated);
-            elem.append(divDOM);
-          }
-
-          elem.bind('mousewheel', (e) => { $scope.keepScrollDown = false; });
-        },
-        (error) => {  elem.html(`${error.stack}`); }
-      );
-    };
+    const elem = angular.element(`#${targetElemId}`);
+    handleData(data, DefaultDisplayType.TEXT,
+      (generated) => {
+        // clear all lines before render
+        removeChildrenDOM(targetElemId);
+
+        if (generated) {
+          const divDOM = angular.element('<div></div>').text(generated);
+          elem.append(divDOM);
+        }
 
-    retryUntilElemIsLoaded(targetElemId, afterLoaded);
+        elem.bind('mousewheel', (e) => { $scope.keepScrollDown = false; });
+      },
+      (error) => {  elem.html(`${error.stack}`); }
+    );
   };
 
   const removeChildrenDOM = function(targetElemId) {
@@ -479,14 +467,13 @@ function ResultCtrl($scope, $rootScope, $route, $window, $routeParams,
$location
     }
   }
 
-  $scope.renderGraph = function(graphElemId, graphMode, refresh) {
+  const renderGraph = function(graphElemId, graphMode, refresh) {
     // set graph height
     const height = $scope.config.graph.height;
     const graphElem = angular.element(`#${graphElemId}`);
     graphElem.height(height);
 
     if (!graphMode) { graphMode = 'table'; }
-    const tableElemId = `p${$scope.id}_${graphMode}`;
 
     const builtInViz = builtInVisualizations[graphMode];
     if (!builtInViz) { return; }
@@ -501,9 +488,11 @@ function ResultCtrl($scope, $rootScope, $route, $window, $routeParams,
$location
       }
     }
 
+    let afterLoaded = function() { /** will be overwritten */ };
+
     if (!builtInViz.instance) { // not instantiated yet
       // render when targetEl is available
-      const afterLoaded = (loadedElem) => {
+      afterLoaded = function(loadedElem) {
         try {
           const transformationSettingTargetEl = angular.element('#trsetting' + $scope.id
+ '_' + graphMode);
           const visualizationSettingTargetEl = angular.element('#trsetting' + $scope.id +
'_' + graphMode);
@@ -542,12 +531,11 @@ function ResultCtrl($scope, $rootScope, $route, $window, $routeParams,
$location
         }
       };
 
-      retryUntilElemIsLoaded(tableElemId, afterLoaded);
     } else if (refresh) {
       // when graph options or data are changed
       console.log('Refresh data %o', tableData);
 
-      const afterLoaded = (loadedElem) => {
+      afterLoaded = function(loadedElem) {
         const transformationSettingTargetEl = angular.element('#trsetting' + $scope.id +
'_' + graphMode);
         const visualizationSettingTargetEl = angular.element('#trsetting' + $scope.id + '_'
+ graphMode);
         const config = getVizConfig(graphMode);
@@ -561,15 +549,15 @@ function ResultCtrl($scope, $rootScope, $route, $window, $routeParams,
$location
         builtInViz.instance.renderSetting(visualizationSettingTargetEl);
       };
 
-      retryUntilElemIsLoaded(tableElemId, afterLoaded);
     } else {
-      const afterLoaded = (loadedElem) => {
+      afterLoaded = function(loadedElem) {
         loadedElem.height(height);
         builtInViz.instance.activate();
       };
-
-      retryUntilElemIsLoaded(tableElemId, afterLoaded);
     }
+
+    const tableElemId = `p${$scope.id}_${graphMode}`;
+    retryUntilElemIsLoaded(tableElemId, afterLoaded);
   };
 
   $scope.switchViz = function(newMode) {

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/ce97b534/zeppelin-web/src/app/notebook/paragraph/result/result.html
----------------------------------------------------------------------
diff --git a/zeppelin-web/src/app/notebook/paragraph/result/result.html b/zeppelin-web/src/app/notebook/paragraph/result/result.html
index 5a05eb7..5b251e5 100644
--- a/zeppelin-web/src/app/notebook/paragraph/result/result.html
+++ b/zeppelin-web/src/app/notebook/paragraph/result/result.html
@@ -28,10 +28,10 @@ limitations under the License.
                     && config.graph.optionOpen && !asIframe && !viewOnly">
         <div ng-repeat="viz in builtInTableDataVisualizationList track by $index"
              id="trsetting{{id}}_{{viz.id}}"
-             ng-if="graphMode == viz.id"></div>
+             ng-show="graphMode == viz.id"></div>
         <div ng-repeat="viz in builtInTableDataVisualizationList track by $index"
              id="vizsetting{{id}}_{{viz.id}}"
-             ng-if="graphMode == viz.id"></div>
+             ng-show="graphMode == viz.id"></div>
       </div>
 
       <!-- graph -->
@@ -40,7 +40,7 @@ limitations under the License.
            ng-class="{'noOverflow': graphMode=='table'}">
         <div ng-repeat="viz in builtInTableDataVisualizationList track by $index"
              id="p{{id}}_{{viz.id}}"
-             ng-if="graphMode == viz.id">
+             ng-show="graphMode == viz.id">
         </div>
       </div>
 


Mime
View raw message