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.

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

preference-sensing persistent dark mode toggle #2

Merged
merged 7 commits into from
Nov 6, 2023
Merged

Conversation

finrod09
Copy link
Collaborator

@finrod09 finrod09 commented Nov 3, 2023

Both light and dark stylesheets are loaded.
If the local storage item is present, that theme is applied.
If it is not present, one style is applied based on browser-supplied preference by means of css @media and light or dark is saved in local storage.
The button toggles the style immediately and saves the preference in local storage, overriding the browser-supplied preference.

@finrod09 finrod09 requested a review from bstiq November 3, 2023 11:11
@finrod09 finrod09 added the enhancement New feature or request label Nov 3, 2023
@finrod09 finrod09 linked an issue Nov 3, 2023 that may be closed by this pull request
@finrod09
Copy link
Collaborator Author

finrod09 commented Nov 3, 2023

The callout colors need to be adjusted for the dark theme. I've made them explicit for your tweaking pleasure, see assets/css/just-the-docs-bkb-dark.scss.

@finrod09
Copy link
Collaborator Author

finrod09 commented Nov 3, 2023

A little prettier: The color scheme toggle and the github aux link are now images.

@bstiq
Copy link
Member

bstiq commented Nov 5, 2023

Great work here.
As far as we can see visually everything looks good.
Let me know if you're also happy with the current code. If no more background work is needed I'll merge the PR.

@finrod09
Copy link
Collaborator Author

finrod09 commented Nov 5, 2023

Only the issue that the toggle is not shown in mobile view remains, which I've broken out into #3.

Since we also respect the preference supplied by the browser, the selected view should be correct most of the time. Imho, the benefits of merging this now outweigh the drawback of possibly locking mobile users into a theme they didn't want.

@bstiq bstiq merged commit 6b22560 into master Nov 6, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dark theme
2 participants