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

Use a custom selection background and foreground color #106

Merged
merged 4 commits into from
Oct 13, 2024

Conversation

ashwinvis
Copy link
Contributor

Closes #105

@ashwinvis ashwinvis changed the title Use a custom selection bg color Use a custom selection background and foreground color Oct 1, 2024
@ashwinvis
Copy link
Contributor Author

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
Copy link
Member

rkdarst commented Oct 2, 2024

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:

  • I like the highlight of the yellow background: it's seems good for highlighting things when teaching.
  • 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)
  • 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...)

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

@ashwinvis
Copy link
Contributor Author

ashwinvis commented Oct 3, 2024

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!

The themes which support light/dark themes
for example, furo and pydata-sphinx-theme
tend to set an attribute in either
<html> or <body> tag.

We can opt for a darker selection background
colour. It could be
#0c6100 - dark green
#745315   dark yellow: we use this!
Comment on lines +9 to +21
/* https://webaim.org/resources/contrastchecker/?fcolor=FFFFFF&bcolor=745315
* when dark theme is selected the some themes use this attirbute
*/
html[data-theme='dark'], body[data-theme='dark'] {
--sphinx-lesson-selection-bg-color: #745315;
--sphinx-lesson-selection-fg-color: #ffffff;
}

/* when browser/system theme is dark and no theme is selected */
@media (prefers-color-scheme: dark) {
html[data-theme='auto'], body[data-theme='auto'] {
--sphinx-lesson-selection-bg-color: #745315;
--sphinx-lesson-selection-fg-color: #ffffff;
Copy link
Contributor Author

@ashwinvis ashwinvis Oct 4, 2024

Choose a reason for hiding this comment

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

Now I have added support for the Sphinx themes furo and pydata_sphinx_theme with their dark variant. It works for 2 scenarios explained in the comments.

In my testing, it turns out sphinx-rtd-theme-ext-contrast 's following directive

color: #242424

breaks pydata_sphinx_theme's dark theme variant, but works for furo's dark one. However that is another issue which could be easily fixed by using the above approach.

@ashwinvis
Copy link
Contributor Author

Here's how it works with Furo:

image

image

@ashwinvis
Copy link
Contributor Author

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
Copy link
Member

rkdarst commented Oct 13, 2024

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.

@rkdarst rkdarst merged commit fbdf2a3 into coderefinery:master Oct 13, 2024
11 checks passed
@ashwinvis ashwinvis deleted the selection-bg-color branch October 13, 2024 21:25
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.

Change highlighter colour to preserve black text
2 participants