Skip to content

DOC: Inheritance diagram for dark mode#15783

Merged
pllim merged 2 commits into
astropy:mainfrom
pllim:doc-inheritance-dark-mode
Jan 9, 2024
Merged

DOC: Inheritance diagram for dark mode#15783
pllim merged 2 commits into
astropy:mainfrom
pllim:doc-inheritance-dark-mode

Conversation

@pllim

@pllim pllim commented Dec 22, 2023

Copy link
Copy Markdown
Member

Description

This pull request is to add CSS to handle inheritance diagram for dark mode. I don't know how to do it in a larger scale from upstream like in sphinx-astropy etc.

Fixes astropy/sphinx-automodapi#167

Example inheritance diagram: https://astropy--15783.org.readthedocs.build/en/15783/modeling/reference_api.html#id6

Coordinates frames: https://astropy--15783.org.readthedocs.build/en/15783/coordinates/index.html#module-astropy.coordinates.builtin_frames

  • By checking this box, the PR author has requested that maintainers do NOT use the "Squash and Merge" button. Maintainers should respect this when possible; however, the final decision is at the discretion of the maintainer that merges the PR.

@github-actions

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.

  • Do the proposed changes actually accomplish desired goals?
  • Do the proposed changes follow the Astropy coding guidelines?
  • Are tests added/updated as required? If so, do they follow the Astropy testing guidelines?
  • Are docs added/updated as required? If so, do they follow the Astropy documentation guidelines?
  • Is rebase and/or squash necessary? If so, please provide the author with appropriate instructions. Also see instructions for rebase and squash.
  • Did the CI pass? If no, are the failures related? If you need to run daily and weekly cron jobs as part of the PR, please apply the "Extra CI" label. Codestyle issues can be fixed by the bot.
  • Is a change log needed? If yes, did the change log check pass? If no, add the "no-changelog-entry-needed" label. If this is a manual backport, use the "skip-changelog-checks" label unless special changelog handling is necessary.
  • Is this a big PR that makes a "What's new?" entry worthwhile and if so, is (1) a "what's new" entry included in this PR and (2) the "whatsnew-needed" label applied?
  • Is a milestone set? Milestone must be set but we cannot check for it on Actions; do not let the green checkmark fool you.
  • At the time of adding the milestone, if the milestone set requires a backport to release branch(es), apply the appropriate "backport-X.Y.x" label(s) before merge.

@github-actions

This comment was marked as resolved.

@pllim pllim force-pushed the doc-inheritance-dark-mode branch from 2da445a to f4eb31c Compare December 22, 2023 20:00
@pllim

This comment was marked as resolved.

@pllim

This comment was marked as outdated.

@pllim

This comment was marked as resolved.

@pllim pllim force-pushed the doc-inheritance-dark-mode branch 3 times, most recently from f62bb0f to d329990 Compare December 23, 2023 02:38
@pllim pllim force-pushed the doc-inheritance-dark-mode branch from d329990 to 7bf3572 Compare December 23, 2023 03:00
@pllim pllim modified the milestones: v6.0.1, v6.1.0 Dec 23, 2023
@pllim pllim marked this pull request as ready for review December 23, 2023 06:35
@pllim pllim requested review from Cadair and saimn December 23, 2023 06:36
Comment thread docs/_static/astropy.css Outdated

/* Flip the colours on graphviz graphs on dark mode */
html[data-theme="dark"] div.graphviz > object.inheritance {
filter: invert(1);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the background of the site isn't pure black you might want to make this 0.9 or something.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, thanks! I copied over https://github.com/scipy/scipy/blob/c1b803b66a3e9f0416f3feafc6d4cf24d2ec98e2/doc/source/_static/scipy.css#L133 that was mentioned in matplotlib/matplotlib#25983 .

Couldn't figure out how to make the .. plot: directive plot be dark mode friendly but that is out of scope here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Cadair , did lowering the invert stuff make it better or worse? I cannot tell.

The color of an edge in the graph (i.e., the transformations between two
frames) is set by the type of transformation; the legend box defines the
mapping from transform class name to color.
mapping from transform class name to color (in light mode).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like that the colors in the graph and in the legend don't match in dark mode.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can also just not touch this one. So that means, normal inheritance diagrams everywhere else will look dark, but this one will remain light mode no matter what. Is that fine by you?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If dark mode can change colors on the plot then surely it can change the colors in the legend too.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The colors are hardcoded for light mode. Dark mode uses the "invert" magic. If you know how to grab the color code for inverted value, please suggest changes. Thanks!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eerovaher , for the sake of fixing the rest of the diagrams but this one, I am just going to revert the change here and chuck this for future follow-up.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing it should be possible to invert the colors on the legend similarly to how it is done in the graphs, but I don't know CSS well enough to actually make the change immediately.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK lemme try one thing but if that doesn't work, I'll exclude this file from dark mode.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried injection class= at ul level and also in a new div container I wrap around ul but neither seems to have any effect. In fact, they disappear in the final rendered HTML when I inspect page source of the RTD preview. Not sure what is going on here. At least I tried.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You know what... I forgot to refresh the browser. OMG LOL. Anyways, I can try as follow-up PR. Been a long day.

@pllim pllim force-pushed the doc-inheritance-dark-mode branch from fd8e4b5 to c4a84dd Compare January 9, 2024 19:13
Comment thread docs/_static/astropy.css Outdated
Co-authored-by: Stuart Mumford <stuart@cadair.com>
@pllim pllim force-pushed the doc-inheritance-dark-mode branch from c4a84dd to dbcc25d Compare January 9, 2024 21:44
attempted but only got the graph to work
so it is commented out for now.

DOC: Use invert values from scipy CSS
by tupui. Although their CSS was for matplotlib but maybe still works here.

[ci skip]
@pllim pllim force-pushed the doc-inheritance-dark-mode branch from dbcc25d to c197a4a Compare January 9, 2024 22:18
@pllim pllim modified the milestones: v6.1.0, v6.0.1 Jan 9, 2024
@pllim pllim merged commit 4e456bc into astropy:main Jan 9, 2024
@pllim pllim deleted the doc-inheritance-dark-mode branch January 9, 2024 22:50
meeseeksmachine pushed a commit to meeseeksmachine/astropy that referenced this pull request Jan 9, 2024
@pllim pllim mentioned this pull request Jan 9, 2024
1 task
pllim added a commit that referenced this pull request Jan 9, 2024
…783-on-v6.0.x

Backport PR #15783 on branch v6.0.x (DOC: Inheritance diagram for dark mode)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Class inheritance diagram hard to read in dark mode

3 participants