coderefinery / sphinx-lesson

Sphinx extension for creating CodeRefinery lessons
https://coderefinery.github.io/sphinx-lesson/
MIT License
15 stars 20 forks source link

Use a custom selection background and foreground color #106

Closed ashwinvis closed 1 month ago

ashwinvis commented 1 month ago

Closes #105

ashwinvis commented 1 month ago

cc: @rkdarst and @bast Since the text color is implemented in https://github.com/AaltoSciComp/sphinx_rtd_theme_ext_color_contrast/, now I wonder if this is best added there. On the other hand if sphinx_rtd_theme_ext_color_contrast is a temporary package then this change is best added here.

rkdarst commented 1 month ago

This is good! I assume you know more about colors and design than me, if you think this is good then it's good.

There is one thing though: sphinx-lesson could use different themes and this might interfere with others. Most of my work with improving sphinx-rtd-theme's design has gone into here: https://github.com/AaltoSciComp/sphinx_rtd_theme_ext_color_contrast/tree/master/sphinx_rtd_theme_ext_color_contrast . I made it as a way to do accessibility improvements that were too slow in sphinx_rtd_theme. You could look there and simultaneously submit it there, if there isn't already something. But there are many issues and handling is slow, so I wouldn't count on much.

Here are some thoughts you can probably figure out better:

Thanks for looking into this stuff. It's very useful stuff that the rest of us aren't likely to ever get to...

ashwinvis commented 1 month ago

This is good! I assume you know more about colors and design than me, if you think this is good then it's good.

There is one thing though: sphinx-lesson could use different themes and this might interfere with others. Most of my work with improving sphinx-rtd-theme's design has gone into here: https://github.com/AaltoSciComp/sphinx_rtd_theme_ext_color_contrast/tree/master/sphinx_rtd_theme_ext_color_contrast . I made it as a way to do accessibility improvements that were too slow in sphinx_rtd_theme. You could look there and simultaneously submit it there, if there isn't already something. But there are many issues and handling is slow, so I wouldn't count on much.

It could be added to upstream, but let's test it out here. Then it would be easy to change once we deploy it and if people have issues with it. Moreover this is very useful as a teaching aid, but less useful for a regular user reading the docs.

Here are some thoughts you can probably figure out better:

* I like the highlight of the yellow background: it's seems good for highlighting things when teaching.

Yes! That's the idea!

* When things are on a variety of different screens (screenshare, projected on a room screen, etc), do you know if the color choice is still visible?  (it doesn't have to be perfect, we can improve later)

Honestly, I haven't checked, but there are WCAG checkers online and here is one such for the pair of colours: https://webaim.org/resources/contrastchecker/?fcolor=242424&bcolor=FCE762

We could make the foreground colour one shade darker just in case #121212 or go all in with #000000. To my eye, both are fine.

* Do you know if it works when it's in dark theme?  (if dark theme is even supported - I thought it was but don't seem to know enough about firefox to test that...)

Then we would need some other shade of dark yellow and white. We can try to address that using https://developer.mozilla.org/en-US/docs/Web/CSS/@media/prefers-color-scheme

There are 2 kinds of dark theme, btw

  1. that is natively implemented using CSS prefers-color-scheme queries (you can try that by changing Firefox's theme to Dark)
  2. dynamically created using Dark Reader add-on.

Dark Reader can also sense the prefers-color-scheme query and use it, but I need to try it out.

sphinx-rtd-theme does not have a native CSS dark theme, but there are themes such as:

Which support this

Thanks for looking into this stuff. It's very useful stuff that the rest of us aren't likely to ever get to...

You're welcome!

ashwinvis commented 1 month ago

Here's how it works with Furo:

image

image

ashwinvis commented 1 month ago

I confirm that this PR also works with sphinx v8 and sphinx-rtd-theme v3 - ofc it should, it is pure CSS after all.

rkdarst commented 1 month ago

Thanks! I will assume you know this better than me and can fix stuff if it breaks.

But I really think that sphinx_rtd_theme_ext_color_contrast is a better place to put this. It's hardly an "upstream", I also maintain it and develop it as a companion to almost every Sphinx project, including non-teaching ones.

Still... maybe this is mainly useful for teaching where it's seen on a screen? Sounds good to merge and we can improve. Send PRs when you want to move around.