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
- in `componentDidUpdate`, set descriptionHeight state like
```
const descriptionHeight =
this.props.isExpanded && this.descriptionRef
? this.descriptionRef.offsetHeight
: 0;
```
- 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;
}
```
- for `getChartHeight()` function , get 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]
