kgabryje commented on PR #21438: URL: https://github.com/apache/superset/pull/21438#issuecomment-1245394309
Thank you for the feedback! @hushaoqing @villebro Removing charts after scrolling them out of view was a conscious decision in order to reduce the number of nodes in DOM tree. However, I do agree that seeing the loading spinner all the time might be annoying, especially for the charts that take longer to render. It's very easy to change that behaviour, I'll do it later today. If we get some opposite feedback (charts should be removed), I can introduce another feature flag to make it configurable. @michael-s-molina I considered using `react-window`, but I don't think it's applicable in this case. `react-window` expects a clear list of elements to render, with their heights and widths. The logic of rendering the dashboard is more complex - here we have `DashboardComponent`s recursively rendering other `DashboardComponent`s. Moreover, even if we managed to restructure dashboard rendering logic to use `react-window`, that would mean that we not only delay drawing the chart, but also running chart queries - as it's the `src/components/Chart.jsx` component that handles running queries and we'd not render it until it's in view. I think in this case the "manual" implementation of virtualization with `IntersectionObserver` is a better choice -- 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]
