graceguo-supercat commented on a change in pull request #14467:
URL: https://github.com/apache/superset/pull/14467#discussion_r626173442



##########
File path: superset-frontend/src/dashboard/components/gridComponents/Chart.jsx
##########
@@ -164,6 +164,14 @@ export default class Chart extends React.Component {
     clearTimeout(this.resizeTimeout);
   }
 
+  componentDidUpdate(prevProps) {
+    // refresh chart when user toggles the description.
+    // fix for https://github.com/apache/superset/issues/12643
+    if (prevProps.isExpanded !== this.props.isExpanded) {
+      this.forceUpdate();
+    }

Review comment:
       forceUpdate() is a solution but we should try to avoid all uses of 
forceUpdate() and only read from this.props and this.state in render().
   
   So i suggest we do this:
   - add `descriptionHeight` in component state, initial value = 0
   - add `componentDidUpdate` so that we can set descriptionHeight state 
whenever `isExpanded` prop is changed: 
   ```
     componentDidUpdate(prevProps, prevState, snapshot) {
       if (this.props.isExpanded !== prevProps.isExpanded) {
         const descriptionHeight =
           this.props.isExpanded && this.descriptionRef
             ? this.descriptionRef.offsetHeight
             : 0;
   
         this.setState({
           descriptionHeight,
         })
       }
     }
   ```
   - in `shouldComponentUpdate`, if descriptionHeight state is changed, just 
like height state is change,  should return true:
   ```
   if (
         nextState.width !== this.state.width ||
         nextState.height !== this.state.height ||
         nextState.descriptionHeight !== this.state.descriptionHeight
       ) {
         return true;
       }
   ```
   - in the end, `getChartHeight()` function should return chart content height 
by height and descriptionHeight:
   ```
     getChartHeight() {
       const headerHeight = this.getHeaderHeight();
       return this.state.height - headerHeight - this.state.descriptionHeight;
     }
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to