-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Improve inheritance diagrams #22273
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
Improve inheritance diagrams #22273
Conversation
I'm not convinced of scrollbars. This significantly clips the contents and reduces usability. See the screenshots below (please ignore resolution or use the links - screenshots taken on a 4K display with 1.3x don't render well). Maybe the current diagrams don't look as nice, but they are more usable. One can be a better rendering using SVG, which we already had, but reverted in #17913 because of an sphinx issue with broken links in SVG. My preference:
New: Old: |
SVG may fix the URL map, but I'm not so sure it will look better for consistency in the cases where everything gets scaled down (such as |
Not sure if it's my 4K screen which makes it look worse or better one way or the other. Appearance is mediocre either way. What counts for me is usability and there needing to scroll the diagram is a no-go. If the size is really not managable, it would even better to only have a link and show the diagram somewhere else rather than show a small viewport. |
8594bd3
to
75c685d
Compare
Discussed on the call, we are going to for ugly but working, over broken. |
When Graphviz is given a page size (which Sphinx does by default), it will layout the whole graph, and then scale the _entire_ thing down to fit. This means that text and lines on large diagrams are inconsistent with ones that fit. Additionally, the default size is larger than the content width which means the browser will also scale the image down a bit. But the URL map is _not_ scaled down, so the links will appear in the wrong spot. To fix this, set the page size in Graphviz very large, and make the diagram scrollable with CSS. Plus a few minor tweaks to font and line sizes now that images are a consistent scale.
75c685d
to
aa1ae8d
Compare
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.
This works on the three diagrams I spot checked...
PR Summary
When Graphviz is given a page size (which Sphinx does by default), it will layout the whole graph, and then scale the entire thing down to fit. This means that text and lines on large diagrams are inconsistent with ones that fit.
Additionally, the default size is larger than the content width which means the browser will also scale the image down a bit. But the URL map is not scaled down, so the links will appear in the wrong spot.
To fix this, set the page size in Graphviz very large, and make the diagram scrollable with CSS.
Plus a few minor tweaks to font and line sizes now that images are a consistent scale.
PR Checklist
Tests and Styling
pytest
passes).flake8-docstrings
and runflake8 --docstring-convention=all
).Documentation
doc/users/next_whats_new/
(follow instructions in README.rst there).doc/api/next_api_changes/
(follow instructions in README.rst there).