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

Add support for themes + GitHub dark dimmed theme #1416

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

LordBaryhobal
Copy link
Contributor

Added css for a Github dark dimmed theme
To enable it add this to the config file:

[html]
dark_theme = True

@nedbat
Copy link
Owner

nedbat commented Jul 1, 2022

Hi, we should have a discussion about how to support more themes before you get too much further into this.

  • GitHub has a number of themes, so I'm concerned that a boolean dark_theme = True is limiting.
  • Why are you editing a .css file? Shouldn't this be done in a .scss file instead?

@LordBaryhobal
Copy link
Contributor Author

Hi, thank you for taking the time to look into my PR.
I totally agree on the fact that a boolean is quite limiting, I just used that since I only had one custom theme. I chose this theme because I like it but it would be nice to have the other GitHub themes as a choice.

I recently used Sphinx to document one of my projects and found the theme gallery very useful, so that could be a thing too for coveragepy

I wrote a .css file because I'm not used to .scss but that is of course the better choice. I'll try to convert it and make a new commit.

@LordBaryhobal
Copy link
Contributor Author

LordBaryhobal commented Jul 2, 2022

check-manifest seems to fail because lists of files in version control and sdist do not match but it runs fine on my computer
How can I fix this ?

@LordBaryhobal
Copy link
Contributor Author

LordBaryhobal commented Jul 2, 2022

I changed setup.py to include htmlfiles/**/*.* and not just htmlfiles/*.*
I also added the themes folder to the do_check_eof files list in igor.py

The Makefile should also be modified to compile all .scss themes but I don't know how to do that

@ProsperousHeart
Copy link
Contributor

Is this still valid? Dark theme looks fine for me

@LordBaryhobal
Copy link
Contributor Author

Hello,
I completely forgot about this PR...
Yes I think this is still valid ?
I haven't used coverage.py recently but looking at the documentation, I can't see any implementation of themes

Let me know if I should do something

@LordBaryhobal LordBaryhobal changed the title Github dark dimmed theme Add support for themes + GitHub dark dimmed theme Oct 3, 2023
@nedbat
Copy link
Owner

nedbat commented Oct 4, 2023

I'm sure people would appreciate other themes. I'm concerned about the duplication in the .scss though: many aspects of the styling that are the same between the two themes (default and dark dimmed) are expressed separately in the two themes, but sometimes in different ways. Can we refactor so that the coloring differences are isolated from other layout concerns?

@vsalvino can you help?

@153957
Copy link

153957 commented Jan 17, 2024

Would it make more sense to simply use the CSS media query prefers-color-scheme to automatically use a dark theme when the user prefers the dark color scheme (automatically detected by system/browser)? That way it is up to the reader/viewer what style they prefer instead of some setting.

@vsalvino
Copy link
Contributor

vsalvino commented Jan 17, 2024

The default CSS already uses prefer-color-scheme to switch between dark/light. This is based on browser preference, which is user toggleable in most browsers or operating systems. This is especially nice because during the daylight one might have the light theme on, and at night the dark theme, and the coveragepy report will automatically adapt without having to be re-generated.

If you're looking to implement custom user-made themes, Sphinx has a configurable theme system which might serve as inspiration. I would highly recommend making these themes 3rd party packages, unless the maintainer of this package is willing to commit to maintaining these new themes indefinitely.

@153957
Copy link

153957 commented Jan 17, 2024

Ah I did not check that, good to know. In that case I would agree it makes more sense to let these be 3rd party package if there is a demand for more themes.

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.

5 participants