-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
DOC Improve resizing of plotly parallel coord plots #30778
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DOC Improve resizing of plotly parallel coord plots #30778
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, I pushed a commit try to trigger a CircleCI run on an example using parallel coordinate plot to double-check this works.
Rendered documentation looks fine, compared to 1.5 doc which has the rendering issue (colorbar of the parallel coordinate plot is only partly visible) |
This reverts commit 20b97f8.
I reverted my temporary change and set auto-merge |
@jeromedockes out of curiosity, it feels like the fix would belong in pydata-sphinx-theme, that would need to trigger a resize event once it is loaded. Would you agree? |
Loïc Estève ***@***.***> writes:
@jeromedockes out of curiosity, it feels like the fix would belong in pydata-sphinx-theme, that would need to trigger a resize event once it is loaded. Would you agree?
I'm not sure because it is kind of a hack which may only be needed for plotly figures: we pretend the window is being resized, but actually the window hasn't changed just the center column due to the insertion of the side bar.
So one could argue it is rather the plotly figures that could change their behavior, by using the ResizeObserver API to react to changes in the size of their container `div` rather than just listening to window resize events, and maybe they should wait for the DOM to load before drawing the figure. Maybe they do but it doesn't work here or maybe they cannot for some reason -- I really don't know enough about plotly to venture a guess.
So to move this upstream I think I would look at plotly rather than the sphinx theme, in general there shouldn't be a need to dispatch a resize event on the window after changing the layout of the page it's really more of a workaround for the figure's limited responsiveness. I'll try to look if there are any open plotly issues about this.
|
@lesteve I think we can follow this issue: plotly/plotly.js#7059 it is also suggesting to watch parent element size changes with |
OTOH I guess many pydata-themed docs use plotly so it might make sense to have the quickfix in the theme until the issue is fixed in plotly as you suggested 🤔 |
Thanks a lot for the investigation, I have added a comment in plotly: plotly/plotly.js#7059 (comment). Let's see what they say! |
Improvement of #30297
As discussed with @glemaitre and @lesteve :
I tried to use the strategy used to resize plotly figures in the example gallery for the skrub library, however it was failing when I had several parallel plots in the same example
in skrub I changed the script as shown in this PR:
the figures are already responsive by default, so we don't need to resize them manually using
Plotly.Plots.resize
(nor to listen to resize events). It seems to be enough to trigger a resize event once the DOM has loaded and the figures will adjust themselves.But I haven't tried to reproduce the issue I had in a scikit-learn example so actually I don't know if the problem would occur or if this change would help 🤷