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

fix: correctly invert colors of alerts in dark mode #591

Merged
merged 2 commits into from
Jul 10, 2024

Conversation

BelKed
Copy link
Contributor

@BelKed BelKed commented Jun 14, 2024

Before:

Screenshot


After:

Screenshot


🚀 This description was created by Ellipsis for commit 67db254

Summary:

Exclude the close button from color changes in dark mode by updating static/dark.css.

Key points:

  • Update static/dark.css to exclude .close button from color changes
  • Modify body, button, html, table selector to body, button:not(.close), html, table

Generated with ❤️ by ellipsis.dev

Copy link

@ellipsis-dev ellipsis-dev bot left a 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! Reviewed everything up to 67db254 in 21 seconds

More details
  • Looked at 21 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. static/dark.css:11
  • Draft comment:
    The change to exclude .close buttons from the color styling in dark mode is correctly implemented using the :not() CSS pseudo-class. This ensures that the close buttons retain their original styling, making them visually distinct, which is important for usability.
  • Reason this comment was not posted:
    Confidence changes required: 0%
    The PR aims to exclude the close button from the color change applied to other buttons in dark mode. The selector used is button:not(.close), which is intended to apply the color #e9ebf0 to all buttons except those with the class .close. This seems to be a targeted change to maintain the original color of the close button in alerts or modals, likely because the dark mode color makes it less visible or aesthetically pleasing. The change is straightforward and appears to be correctly implemented based on the CSS selector used.

Workflow ID: wflow_W3ZsZlsLBORxmJHq


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link

codecov bot commented Jun 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 26.13%. Comparing base (2f3d1e8) to head (275a2e6).
Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #591   +/-   ##
=======================================
  Coverage   26.13%   26.13%           
=======================================
  Files          27       27           
  Lines        1630     1630           
  Branches      281      286    +5     
=======================================
  Hits          426      426           
- Misses       1145     1178   +33     
+ Partials       59       26   -33     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@BelKed BelKed force-pushed the dark-mode-alert branch from 4e02035 to 11ea327 Compare June 14, 2024 11:53
@ErikBjare
Copy link
Member

ErikBjare commented Jul 9, 2024

This is great, but I wonder if we should instead make the popup less bright/a darker red (and a brighter text).

@BelKed BelKed changed the title [Dark mode] Don't change the color of the close button in the alert [Dark mode] Make alerts less bright Jul 10, 2024
@BelKed
Copy link
Contributor Author

BelKed commented Jul 10, 2024

Good point, "inverting" the colors makes it look amazing 😍

Screenshot 2024-07-10 at 13 26 26 Screenshot 2024-07-10 at 13 26 46 Screenshot 2024-07-10 at 13 26 08

@ErikBjare ErikBjare changed the title [Dark mode] Make alerts less bright fix: correctly invert colors of alerts in dark mode Jul 10, 2024
@ErikBjare
Copy link
Member

Very nice! Merging 🎉

@ErikBjare ErikBjare merged commit e6ad4cd into ActivityWatch:master Jul 10, 2024
8 checks passed
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