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  ##### After: fixed  ### Questions: * Does the licenses files need update? - NO * Is there breaking changes for older versions? - NO * Does this needs documentation? - NO Author: 1ambda <1am...@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 <1am...@gmail.com> Authored: Fri Mar 10 15:33:19 2017 +0900 Committer: Lee moon soo <m...@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>
