DOC use sphinx-gallery css variable to adapt light/dark theme#27659
DOC use sphinx-gallery css variable to adapt light/dark theme#27659GaelVaroquaux merged 6 commits intoscikit-learn:mainfrom
Conversation
| --sklearn-color-text-on-default-background: var(--theme-code-foreground, var(--jp-content-font-color1, black)); | ||
| --sklearn-color-background: var(--theme-background, var(--jp-layout-color0, white)); | ||
| --sklearn-color-border-box: var(--theme-code-foreground, var(--jp-content-font-color1, black)); | ||
| --sklearn-color-text-on-default-background: var(--sg-text-color, var(--theme-code-foreground, var(--jp-content-font-color1, black))); |
There was a problem hiding this comment.
Just to double check that I understand this CSS (in which case TIL CSS vars!): this is going to set --sklearn-color-text-on-default-background to the value of --sg-text-color if it exists, falling back to --theme-code-foreground, falling back to --jp-content-font-color1 and if none of those exists black?
There was a problem hiding this comment.
yes exactly. So checking the rendering, it seems that does not solve the issue because sphinx-gallery is detecting that I am in a dark mode while we don't have dark mode in scikit-learn. Maybe it uses the OS then.
There was a problem hiding this comment.
It means that switching to a proper light/dark mode as the one offered by the pydata-sphinx-theme would be the way to go.
There was a problem hiding this comment.
So sphinx-gallery sets the value of --sg-text-color based on the OS setting?
There was a problem hiding this comment.
So to make it work, I need to add data-theme="light" in the <html> tag since this is the selector imposed by sphinx-gallery.
betatim
left a comment
There was a problem hiding this comment.
Assuming my understanding in the comment is correct, I think we should merge this.
It feels like a better solution would be to "invert" this. So that sphinx-gallery or a notebook viewer could set the colours used. But this is something for a new PR (if it is even a good idea).
|
OK it works now. |
|
@GaelVaroquaux do you want to merge this one. Just to explain visually what it changes on my OS.
|
|
Taking a step back to see if I understand what happens/this PR does: when the OS (or browser?) is set to dark mode currently the gallery is unchanged (white background) but the estimator representation changes to "dark mode". This is because the gallery doesn't automatically switch and the estimator does. Is that right? I think we should merge this and maybe also think about some way to solve this "generically", without having to extend the |
This is right. |
GaelVaroquaux
left a comment
There was a problem hiding this comment.
LGTM, but it would be good to add a note somewhere in the docs.
Do we have docs for downstream libraries? We should
|
Once the note in the doc is added, this is ready to merge (and I can do the merge) |
The website rendering uses the CSS variable defined by sphinx-gallery. Therefore, we use the default OS preference while sphinx-gallery already provide some CSS variable.
Here, we intend to use these variables for a better integration (and it should make it compatible with pydata-theme for the future).