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

Docs: Add dark mode #4968

Merged
merged 4 commits into from Oct 14, 2020
Merged

Docs: Add dark mode #4968

merged 4 commits into from Oct 14, 2020

Conversation

hugovk
Copy link
Member

@hugovk hugovk commented Oct 12, 2020

Dark mode is the new thing!

Not only that, it can improve accessibility, ease eye strain, and save battery power.

The prefers-color-scheme media query allows the theme to detect whether the user has requested a dark or light mode, often via an operating system toggle or time-of-day trigger.

Theme review

We're using the Read the Docs Sphinx Theme, but it doesn't support dark theme:

Checking some other themes, they don't support dark mode yet:

PdJ Sphinx Theme is a specific dark theme, but is always dark and doesn't yet toggle via prefers-color-scheme:

Alternative

As suggested at readthedocs/sphinx_rtd_theme#224 (comment), the popular Dark Reader browser extension allows export of the dark mode styles it computes for a website.

The CSS here is exported from Dark Reader, wrapped in @media (prefers-color-scheme: dark) { ... } and added to our config.

Demo

https://pillow--4968.org.readthedocs.build/en/4968/

I've checked it in Chrome, Firefox, Safari and Opera on Mac, and in Chrome and Samsung Internet on Android, not found any problems.

Would be good to check it on Windows too.

@nulano
Copy link
Contributor

nulano commented Oct 12, 2020

It looks really good!

I tested on Windows with Internet Explorer 11 (has no effect), MS Edge (based on Chromium, works), Vivaldi (based on Chromium, works), Firefox (works).

While opening the page in IE11, I noticed it doesn't even support GitHub properly, so this is probably fine.


One problem with this theme is the Text Anchors page: https://pillow--4968.org.readthedocs.build/en/4968/handbook/text-anchors.html#quick-reference

Image

image

Is it possible to use a different image based on the selected theme? I'm thinking something like adding both a bright theme image and a dark theme image, adding a class to the dark image, and using css to hide one of the images.

@pradyunsg
Copy link

Godot has a really nice theme built on top of RTD-Sphinx-Theme, but someone needs to factor it out to be reusable -- godotengine/godot-docs#3873.

@hugovk
Copy link
Member Author

hugovk commented Oct 12, 2020

Thanks for testing!

Internet Explorer 11 (has no effect),

Yeah, IE11 doesn't support it: https://caniuse.com/?search=prefers-color-scheme

Is it possible to use a different image based on the selected theme? I'm thinking something like adding both a bright theme image and a dark theme image, adding a class to the dark image, and using css to hide one of the images.

Yes, that's one way. But will both images be loaded by the browser? Alternatively, figure out how to do this in Sphinx: https://stackoverflow.com/a/56030447/724176

But from a maintenance point of view, it'll be easier to keep one set of images.

The easiest thing is just to stick a white (or off-white) background in the SVGs themselves.

Or add a background to dark.css that's lighter-than-black and darker-than-white. Let's try this, any suggestions for a colour?

@hugovk
Copy link
Member Author

hugovk commented Oct 12, 2020

Godot has a really nice theme built on top of RTD-Sphinx-Theme, but someone needs to factor it out to be reusable -- godotengine/godot-docs#3873.

Looks good!

Would be nice to get something "good enough" out as /stable in this week's release, then replace it with something else when there's something else that's ready :)

@nulano
Copy link
Contributor

nulano commented Oct 12, 2020

The easiest thing is just to stick a white background in the SVGs themselves.

Or add a background to dark.css that's lighter-than-black and darker-than-white. Let's try this, any suggestions for a colour?

I've played with it a bit, and can't find a color where both the page and image have readable text.

However, I have found that adding a style="filter: invert" looks good just by itself.
It seems to be supported almost everywhere (except for IE11).

Image

image

@hugovk
Copy link
Member Author

hugovk commented Oct 12, 2020

How does 461d936 look? In addition to the two black-on-transparent SVGs, it inverts the two black-on-white PNGs:

https://pillow--4968.org.readthedocs.build/en/4968/handbook/text-anchors.html#specifying-an-anchor

Details

image

image

image

docs/resources/css/dark.css Outdated Show resolved Hide resolved
Co-authored-by: nulano <nulano@nulano.eu>
docs/resources/css/dark.css Outdated Show resolved Hide resolved
Co-authored-by: nulano <nulano@nulano.eu>
Copy link
Contributor

@nulano nulano left a comment

Choose a reason for hiding this comment

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

There is also an image in 6.2.0 release notes, but it already uses a dark colour scheme. I can't see any other images in the docs.

image

@hugovk
Copy link
Member Author

hugovk commented Oct 14, 2020

Let's go with this! We can review other themes as and when they get dark mode.

@hugovk hugovk merged commit 8aa6a5e into python-pillow:master Oct 14, 2020
@hugovk hugovk deleted the dark-mode branch October 14, 2020 14:08
@pradyunsg
Copy link

FYI -- Furo's got a nice dark mode now. Just saying. ;)

@hugovk
Copy link
Member Author

hugovk commented Oct 15, 2020

Thanks! Here's a demo (just rebuilt with latest Furo 2020.10.15b13):

I gave it a quick test before, and there was something a bit strange on mobile with the advert always showing at the bottom of the page rather than hidden in the left menu. Of course, I didn't take a screenshot and cannot reproduce now (no ad being served), but will report to the tracker if I see it again!

@pradyunsg
Copy link

Yes please! I'd also appreciate if you could put down the html for that page somewhere, coz it's dependent on when RTD decides to serve an ad.

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.

None yet

3 participants