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

Added Dark Mode support to option tables #253

Merged
merged 12 commits into from
Mar 12, 2024

Conversation

SebastianGode
Copy link
Contributor

@SebastianGode SebastianGode commented Mar 6, 2024

I noticed that Antsibull didn't respect the prefers-color-scheme browser or system setting of the users which access the documentation website.
This will result in completely unreadable tables for Sphinx-Themes which already include support for such user settings.

To fix this I added some css code to change the colors accordingly for dark mode. The colors are chosen to get a contrast ration >7 and to pass Google's usability rating with an AAA.

If you have any suggestions feel free to mention them.

Before
image
After
image

Tip: You can quickly switch the Browser View to Dark-Mode or Light-Mode by using the Developer Options:
image
Just don't use the auomatic conversion from the Browser as that will replace all CSS.

@felixfontein
Copy link
Collaborator

@SebastianGode thanks for your contribution! I'll take a closer look soon. Can you add a changelog fragment? Thanks.

@SebastianGode
Copy link
Contributor Author

I've added a fragment. I'm not sure if it's correct, but I tried :)

@felixfontein felixfontein changed the title Added Dark-Mode Support to Option-Tables Added Dark Mode support to option tables Mar 9, 2024
Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

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

There are two big problems with this PR:

  1. This is only setting some colors. In case the theme itself does not have a dark mode, the tables are partially unreadable:
    image
  2. In low-width modes, it really looks bad since most colors are not adjusted:
    image

Maybe it's better to make dark mode support configurable, so you can enable it if the underlying theme also supports it, and disable it if not. That would fix 1. For 2., some more CSS would be needed.

@felixfontein felixfontein mentioned this pull request Mar 10, 2024
3 tasks
@felixfontein
Copy link
Collaborator

I started working around these problems in #254. That PR makes it easy to add new color schemes (this PR would translate into creating a copy of the default scheme, adding a media query with redefinitions of the colors for dark mode, and allowing to select that color scheme in the config).

That way colors can be selected (or even defined) by the extension's user (who also has control over which theme and other extensions are used).

@felixfontein
Copy link
Collaborator

I went ahead and merged #254 so you aren't blocked on that PR. Note that I changed the way the node.js dependencies (sass and postcss) are installed, now you have to run npm install or npm clean-install in src/sphinx_antsibull_ext/css/ before running ./build.sh.

@SebastianGode
Copy link
Contributor Author

Thanks for the work @felixfontein !
I will have a look, it seems like you added some variables which I can simply adjust, will need to try that stuff out.

@SebastianGode
Copy link
Contributor Author

@felixfontein I now think that I have fixed the tables in the narrow view. And adding the option to select different themes was awesome! That works like magic now :)

@felixfontein felixfontein merged commit 0768e27 into ansible-community:main Mar 12, 2024
12 checks passed
@felixfontein
Copy link
Collaborator

@SebastianGode thanks a lot for your contribution!

@SebastianGode
Copy link
Contributor Author

@felixfontein Would it be possible to get a new Release to PyPi? 😃

@felixfontein
Copy link
Collaborator

Yes, maybe later today or tomorrow. I want to add a small change before releasing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants