-
Notifications
You must be signed in to change notification settings - Fork 243
dark mode #381
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
dark mode #381
Conversation
Codecov Report
@@ Coverage Diff @@
## master #381 +/- ##
=======================================
Coverage 97.63% 97.63%
=======================================
Files 4 4
Lines 423 423
=======================================
Hits 413 413
Misses 10 10
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
Hey @jkowalleck , Thanks for the PR! 🙏 Would you mind adding the report or screenshot of the report as a file attached to this PR? So that the link doesn't break in the future if it's removed. 😊 Cheers |
@@ -28,6 +29,25 @@ table { | |||
border-collapse: collapse; | |||
} | |||
|
|||
@media (prefers-color-scheme: dark) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does the browser support look like for this annotation? Will this break older browsers?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems pretty recent: https://developer.mozilla.org/en-US/docs/Web/CSS/@media/prefers-color-scheme
How will older browsers handle this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
general browser support: https://caniuse.com/mdn-css_at-rules_media
special browser support: https://caniuse.com/prefers-color-scheme
old browsers will not be broken. they will simply ignore it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Thanks!
Hello @BeyondEvil
resulting HTML is this: https://jkowalleck.github.io/pytest-html/report.html here is a screenshot, for those that dont have dark mode enabled: https://jkowalleck.github.io/pytest-html/report.png |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, but I'm no CSS ace. @gnikonorov @ssbarnea any comments?
Is it possible to (unit) test this somehow? 🤔 |
thanks :-) i loved to have the colors written differently. but since there was no sass/scss and colors were all over the place, i did what i could. i would suggest to strip the colors from where they are and have default- and dark-theme coloring in an extra section ... |
Is it a lot of work to unify the colors in it's own (I guess reusable?) section? @jkowalleck |
not much. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this @jkowalleck!
Can you also add a changelog entry? You can pretty much copy the changes to CHANGES.rst
in #380, just replace the description and authors with a description of this change and yourself.
I can take a thorough look at this tonight/tomorrow @BeyondEvil |
done that. |
closed in favor of #383. |
users that have their OS/Browsers set to dark mode expect to see web sites in a darker tone.
so this PR will add a darker tone to re resulting report.
resulting HTML is this: https://jkowalleck.github.io/pytest-html/report.html
used
.tox/devel/log/report.html
aftertox -e devel
image as shown here: #381 (comment)