rusackas commented on PR #35777: URL: https://github.com/apache/superset/pull/35777#issuecomment-3577246051
Heya! This is MUCH cleaner! Might you (and your team) have time to actually sync up on this? I spoke with some other reviewers on zoom, and the consenus thought is that this does fix the accute issue with minimal code/impact, but is not the right generalized approach for the same sort of issue with other charts. For example, there are other chart interactions that can be done (scrolling in a table, searching/sorting or rearranging columns in a table), adjusting the zoom/scrubber on bar/line charts, zooming on maps, etc. I hope to find time to test this (you're welcome to, too!) but probably ALL of these "chart state interaction" details are lost when scrolling through a dashboard and charts get de-rendered and re-rendered. What might be the right way to do this is to generalize the approach. I'm not sure where the right place is to grab "all chart state" - maybe it's to clone the props for a chart as an object, whatever they may be (legend, search, column order, whatever). Then that entire object could be saved as state when it's de-rendered by dashboard virtualization (de-rendering/re-rendering). It could be saved as Dashboard state, or in sessionStorage, so long as that part of sessionStorage is cleared when moving away from the dashboard, with a React Router hook. Hope that all makes sense? Again, if not, happy to talk about this more realtime (zoom, a Slack channel for the project, etc). -- 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]
