From 7a5bb947542fdc20e2ec70e18b1cf418b8d1dba8 Mon Sep 17 00:00:00 2001
From: vera-liu <vera.liu@airbnb.com>
Date: Thu, 15 Dec 2016 14:06:54 -0800
Subject: [PATCH] Stop ChartContainer from rendering twice on chartStatus
 change (#1828)

* Stop ChartContainer from rendering twice on chartStatus change

* Make spinner overlay and dim chart while laoding

* Added timeout to make render more stable on resize

* Put viz in state and call resize at browser size change

* add render after height change since resize depends on render called on the same object

* Change name of renderVis to renderVisOnChange

* Only call resize at height change, persist viz in state

* Call resize wihout render at window size change
---
 .../explorev2/components/ChartContainer.jsx   | 52 ++++++++++++-------
 .../components/ExploreViewContainer.jsx       |  6 ++-
 superset/assets/visualizations/nvd3_vis.js    |  3 +-
 3 files changed, 38 insertions(+), 23 deletions(-)

diff --git a/superset/assets/javascripts/explorev2/components/ChartContainer.jsx b/superset/assets/javascripts/explorev2/components/ChartContainer.jsx
index ff59c35b9c0..30ad9091038 100644
--- a/superset/assets/javascripts/explorev2/components/ChartContainer.jsx
+++ b/superset/assets/javascripts/explorev2/components/ChartContainer.jsx
@@ -43,31 +43,39 @@ class ChartContainer extends React.Component {
     this.state = {
       selector: `#${props.containerId}`,
       mockSlice: {},
+      viz: {},
     };
   }
 
   componentWillMount() {
-    this.setState({ mockSlice: this.getMockedSliceObject(this.props) });
+    const mockSlice = this.getMockedSliceObject(this.props);
+    this.setState({
+      mockSlice,
+      viz: visMap[this.props.viz_type](mockSlice),
+    });
   }
 
   componentDidMount() {
-    this.renderVis();
+    this.state.viz.render();
   }
 
   componentWillReceiveProps(nextProps) {
-    if (nextProps.chartStatus !== this.props.chartStatus
-      || nextProps.height !== this.props.height) {
-      this.setState({ mockSlice: this.getMockedSliceObject(nextProps) });
+    if (nextProps.chartStatus === 'loading') {
+      const mockSlice = this.getMockedSliceObject(nextProps);
+      this.setState({
+        mockSlice,
+        viz: visMap[nextProps.viz_type](mockSlice),
+      });
     }
   }
 
-  shouldComponentUpdate(nextProps) {
-    return (nextProps.chartStatus !== this.props.chartStatus
-      || nextProps.height !== this.props.height);
-  }
-
-  componentDidUpdate() {
-    this.renderVis();
+  componentDidUpdate(prevProps) {
+    if (this.props.chartStatus === 'loading') {
+      this.state.viz.render();
+    }
+    if (prevProps.height !== this.props.height) {
+      this.state.viz.resize();
+    }
   }
 
   getMockedSliceObject(props) {
@@ -96,7 +104,7 @@ class ChartContainer extends React.Component {
           // should call callback to adjust height of chart
           $(this.state.selector).css(dim, size);
         },
-        height: () => parseInt(props.height, 10) - 100,
+        height: () => parseInt(this.props.height, 10) - 100,
 
         show: () => { this.render(); },
 
@@ -108,7 +116,7 @@ class ChartContainer extends React.Component {
 
       width: () => this.chartContainerRef.getBoundingClientRect().width,
 
-      height: () => parseInt(props.height, 10) - 100,
+      height: () => parseInt(this.props.height, 10) - 100,
 
       selector: this.state.selector,
 
@@ -149,10 +157,6 @@ class ChartContainer extends React.Component {
     this.props.actions.removeChartAlert();
   }
 
-  renderVis() {
-    visMap[this.props.viz_type](this.state.mockSlice).render();
-  }
-
   renderChartTitle() {
     let title;
     if (this.props.slice_name) {
@@ -180,13 +184,21 @@ class ChartContainer extends React.Component {
     return (
       <div>
         {loading &&
-          <img alt="loading" width="25" src="/static/assets/images/loading.gif" />
+          <img
+            alt="loading"
+            width="25"
+            src="/static/assets/images/loading.gif"
+            style={{ position: 'absolute' }}
+          />
         }
         <div
           id={this.props.containerId}
           ref={(ref) => { this.chartContainerRef = ref; }}
           className={this.props.viz_type}
-          style={{ overflowX: 'auto' }}
+          style={{
+            overflowX: 'auto',
+            opacity: loading ? '0.25' : '1',
+          }}
         />
       </div>
     );
diff --git a/superset/assets/javascripts/explorev2/components/ExploreViewContainer.jsx b/superset/assets/javascripts/explorev2/components/ExploreViewContainer.jsx
index 7e4d210e453..a568b9ebead 100644
--- a/superset/assets/javascripts/explorev2/components/ExploreViewContainer.jsx
+++ b/superset/assets/javascripts/explorev2/components/ExploreViewContainer.jsx
@@ -29,7 +29,6 @@ class ExploreViewContainer extends React.Component {
 
   componentDidMount() {
     window.addEventListener('resize', this.handleResize.bind(this));
-    this.props.actions.updateChartStatus('success');
   }
 
   componentWillReceiveProps(nextProps) {
@@ -64,7 +63,10 @@ class ExploreViewContainer extends React.Component {
   }
 
   handleResize() {
-    this.setState({ height: this.getHeight() });
+    clearTimeout(this.resizeTimer);
+    this.resizeTimer = setTimeout(() => {
+      this.setState({ height: this.getHeight() });
+    }, 250);
   }
 
   toggleModal() {
diff --git a/superset/assets/visualizations/nvd3_vis.js b/superset/assets/visualizations/nvd3_vis.js
index ba7bbfcab0a..d4db0fb340b 100644
--- a/superset/assets/visualizations/nvd3_vis.js
+++ b/superset/assets/visualizations/nvd3_vis.js
@@ -57,7 +57,6 @@ function nvd3Vis(slice) {
   let chart;
   let colorKey = 'key';
 
-
   const render = function () {
     d3.json(slice.jsonEndpoint(), function (error, payload) {
       slice.container.html('');
@@ -373,6 +372,8 @@ function nvd3Vis(slice) {
 
   const update = function () {
     if (chart && chart.update) {
+      chart.height(slice.height());
+      chart.width(slice.width());
       chart.update();
     }
   };