Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Since we don't have a Dark theme (yet?) use Light theme only #352

Closed
wants to merge 2 commits into from
Closed

Since we don't have a Dark theme (yet?) use Light theme only #352

wants to merge 2 commits into from

Conversation

psobolewskiPhD
Copy link
Member

@psobolewskiPhD psobolewskiPhD commented Feb 6, 2024 •
edited
Loading

Depends on #423

References and relevant issues

Closes: napari/napari-sphinx-theme#124
See napari/napari-sphinx-theme#124 (comment)
For the docs see: https://pydata-sphinx-theme.readthedocs.io/en/stable/user_guide/light-dark.html#light-and-dark-themes

Description

The default setting is to auto-swap the theme between Light and Dark with system settings, resulting in this in Safari dev console:
[Log] [PST]: Changed to auto mode using the dark theme. (pydata-sphinx-theme.js, line 1)
But we don't actually have a different Dark theme, so it just uses Light, while preventing my extension from darkening it -- but shows Light.

With this change, the theme will not auto change and will just use Light. Once we have a Dark theme we can reconsider.

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Feb 6, 2024
@psobolewskiPhD
Copy link
Member Author

psobolewskiPhD commented Feb 6, 2024 •
edited
Loading

Hmm, when I open the CircleCI preview, I still get:
[Log] [PST]: Changed to auto mode using the light theme. (pydata-sphinx-theme.js, line 1) in the console.
So I must have misunderstood the docs somewhere.

edit: yeah checking on my personal mac, CircleCI artifact I get:
image

So I'm missing something.

@psobolewskiPhD
Copy link
Member Author

So when I pull up the CircleCI artifact, index.html (and other pages) are correct:
<html lang="en" data-content_root="./" data-theme="light">
But when inspecting, I do see an element that is using auto:
html.fontawesome-i2svg-active.fontawesome-i2svg-complete

<html lang="en" data-content_root="./" data-theme="light" data-mode="auto" class="fontawesome-i2svg-active fontawesome-i2svg-complete"><head>

and this element covers the whole page, so I suspect it's the one triggering the javascript.

@melissawm melissawm force-pushed the psobolewskiPhD-patch-8 branch from 2754ef7 to 2076e50 Compare March 21, 2024 20:22
@melissawm
Copy link
Member

OK, I checked that this element indeed comes from the pydata sphinx theme. Your change looks good but I'm not sure it does much as you observed. We can probably raise this upstream but since I don't use this extension I'm not sure I can help much.

@psobolewskiPhD
Copy link
Member Author

I can poke around over there maybe I can find a fix upstream.
Either way, low priority

@psobolewskiPhD psobolewskiPhD marked this pull request as draft March 21, 2024 21:36
@psobolewskiPhD
Copy link
Member Author

psobolewskiPhD commented Mar 21, 2024 •
edited
Loading

Actually, looks like it was raised over there pydata/pydata-sphinx-theme#1661
and fixed? pydata/pydata-sphinx-theme#1663
Looking at that PR it does look like it's the trick.
So when that is released (it's not in 15.2 as far as I can tell) and we up our pin, then the setting in this PR should work.

@melissawm
Copy link
Member

In that case I'll leave as approved and we can even merge, just can't forget to check when the release is out. Thanks!

@psobolewskiPhD psobolewskiPhD marked this pull request as ready for review May 30, 2024 00:45
@psobolewskiPhD psobolewskiPhD added this to the 0.5.0 milestone May 30, 2024
@psobolewskiPhD
Copy link
Member Author

This should work in 15.3, so un-drafting!

@psobolewskiPhD psobolewskiPhD mentioned this pull request May 31, 2024
Merged
@psobolewskiPhD
Copy link
Member Author

Merging main which should rebuild using the new theme (due to changes upstream and in constraints).
Hopefully fixes:
#423 (comment)

@psobolewskiPhD
Copy link
Member Author

psobolewskiPhD commented May 31, 2024 •
edited
Loading

Boo, the site builds with 15.3 and 0.4.0 but I still get:
[PST]: Changed to auto mode using the light theme.
And changing my system from light to dark:
[Log] [PST]: Changed to auto mode using the dark theme. (pydata-sphinx-theme.js, line 1)
which makes the bug from #423 (comment) happen.

@psobolewskiPhD
Copy link
Member Author

psobolewskiPhD commented May 31, 2024 •
edited
Loading

@melissawm with a little help from ChatGPT4o I figured it out!
Here in pydata:
https://github.com/pydata/pydata-sphinx-theme/blob/b45af6cc66e25399880183484eab70baf1767b96/src/pydata_sphinx_theme/theme/pydata_sphinx_theme/layout.html#L16-L19

The mode is set using localStorage.getItem("mode") or if that is null, the default_mode.
The localStorage will persist! So using icognito or removing the cookie did the trick:
image
So it would appear that anyone that previously visited, got the auto mode--because it was default. So now they're stuck with it till their cookie expires.

@psobolewskiPhD
Copy link
Member Author

Closing because this was picked over to #423

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@melissawm melissawm melissawm approved these changes

Assignees

@psobolewskiPhD psobolewskiPhD

Labels
documentation Improvements or additions to documentation
Projects
None yet
Milestone
0.5.0
Development

Successfully merging this pull request may close these issues.

Either provide a Dark theme or don't have advertise a dark theme
2 participants
@psobolewskiPhD @melissawm