executablebooks / sphinx-tabs

Tabbed views for Sphinx

https://sphinx-tabs.readthedocs.io

MIT License

263 stars 67 forks source link

[fix] prefers-color-scheme: use dark theme if body[data-theme="auto"] #154

Closed return42 closed 2 years ago

return42 commented 2 years ago

Before this patch the CSS implementation assumes that "dark" is supported by the theme if the data-theme attribute of the <body> tag is unset.

Since most themes are using "light" colors (compare https://sphinx-themes.org) and do not set data-theme it is better to assume light is the default, even when the browser setting prefers-color-scheme: dark!

BTW: remove duplication of styles from dark/light theme that are already set in the common style.

To test enable dark mode in your browser (prefers-color-scheme: dark).

Current master (default theme):

After this patch applied (default theme):

I also tested furo theme which supports a dark/light mode. This is the result when browser (prefers-color-scheme: dark):

BTW: the result is the same if you toggle from "auto" to "dark".

And here is the result when you toggle into "light" mode ..

Closes: #152

return42 commented 2 years ago

@foster999 sorry for being impatient again .. can you merge and release?

Its not only my project also documentation from many other projects are suffered by this issue / e.g. all projects at RTD that are using sphinx-tabs (without pinning an old version) ..

By example, switch your browser to prefer dark mode and visit RTD / e.g.: https://sphinx-notfound-page.readthedocs.io/en/latest/installation.html

grafik

Another example is sphnix-tabs @ RTD itself: https://sphinx-tabs.readthedocs.io/en/latest/

Thanks!

codecov[bot] commented 2 years ago

Codecov Report

Merging #154 (f19ff0d) into master (53b6a63) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #154   +/-   ##
=======================================
  Coverage   92.69%   92.69%           
=======================================
  Files           2        2           
  Lines         219      219           
=======================================
  Hits          203      203           
  Misses         16       16

Flag	Coverage Δ
pytests	`92.69% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 53b6a63...f19ff0d. Read the comment docs.

foster999 commented 2 years ago

I'm sure you don't need me to tell you that not pinning your dependencies is a bad idea 😉

return42 commented 2 years ago

I'm sure you don't need me to tell you that not pinning your dependencies is a bad idea

Depends .. pinning a library at major version level is often common .. applications are often pinned at minor level (to get bug fixes) .. but it is handled different in the projects.

EDIT forgot to say / thanks for review & merge!

return42 commented 2 years ago

@foster999 could you deploy a bugfix release (3.3.1) at PyPI .. thanks!

foster999 commented 2 years ago

@return42 new version is released :)

OlafenwaMoses commented 2 years ago

Thanks very much for this fix @return42