stephenLYZ commented on code in PR #19960:
URL: https://github.com/apache/superset/pull/19960#discussion_r880117491
##########
superset-frontend/src/dashboard/components/gridComponents/Chart.jsx:
##########
@@ -154,6 +157,10 @@ export default class Chart extends React.Component {
return true;
}
+ if (!isEqual(nextProps.chart, this.props.chart)) {
Review Comment:
It makes sense to not render the chart when it is not visible, which
improves performance. The same is true for fine-grained control of chart
updates via SHOULD_UPDATE_ON_PROP_CHANGES.
In this issue, in the case of a chart that is not updated, I suggest finding
a prop that is not updated case by case and add it to
SHOULD_UPDATE_ON_PROP_CHANGES.
##########
superset-frontend/src/dashboard/components/gridComponents/Chart.jsx:
##########
@@ -175,6 +180,11 @@ export default class Chart extends React.Component {
this.resizeTimeout = setTimeout(this.resize, RESIZE_TIMEOUT);
}
+ if (nextState.isUpdated) {
+ this.setState({ isUpdated: false });
Review Comment:
It is not recommended to call `setState` in SCU, it is rather hacky. If you
want to change state when a prop change, please consider using [static
getDerivedStateFromProps](https://reactjs.org/docs/react-component.html#static-getderivedstatefromprops).
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
