kgabryje commented on a change in pull request #16444:
URL: https://github.com/apache/superset/pull/16444#discussion_r696403065
##########
File path: superset-frontend/src/dashboard/components/gridComponents/Chart.jsx
##########
@@ -87,7 +87,8 @@ const defaultProps = {
// resizing across all slices on a dashboard on every update
const RESIZE_TIMEOUT = 350;
const SHOULD_UPDATE_ON_PROP_CHANGES = Object.keys(propTypes).filter(
- prop => prop !== 'width' && prop !== 'height',
+ prop =>
+ prop !== 'width' && prop !== 'height' && prop !== 'isComponentVisible',
Review comment:
In `shouldComponentUpdate` we have a `if(isComponentVisible)` statement
followed by additional checks if component should render. The point is that we
run those only when component is visible.
In other words, `isComponentVisible` should be used to decide if we should
run checks to determine if component should be rerendered. However, the change
of `isComponentVisible` alone shouldn't trigger a rerender, because it's
already rendered. You can check out that behaviour by switching tabs back and
forth. On the first change of tab, `isComponentVisible` changes, but so do
other props, so the chart is rerendered. But if you go back to previous tab,
only `isComponentVisible` changes, so there's no point in rerendering a chart
because there are no changes that would affect that chart.
Sorry for rambling, I hope that it makes sense 😆
--
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]
