-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
fix: adjust settings check colors #12218
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: adjust settings check colors #12218
Conversation
Use the same colors in the settings check as in reset branch. These colors also look OK in dark mode. Use a common definition of the bright colors.
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.
LGTM
In the dark mode the text should be light, not dark though. |
Other than that, the screenshots look good |
In this PR, the dark mode colors are not adapted, the dark mode colors are very bright, same as in light mode. This is slightly to light to me, but acceptable. It takes forever to adjust all colors. The "automatic" adaption will try to change the very bright colors close to the white background to dark colors close to the dark mode grayish background. This worked better with the 3.x dark mode as the background was closer to black. It is much harder to adjust colors with the .net9 dark mode, it looks worse and less contrast to play with. (Most dark modes are darker too.) But custom colors would in this case require theme colors (or that the OtherColors are reversed and the colors are hardcoded again). An example below, also in light mode (not great).
All details will not be fixed in #12111, there are too many issues in .net9 implementation anyway, it will be experimental. |
My comment isn't blocking, more like thinking out loud. This can certainly be tinkered with later. |
If you have comments on the dark mode related to this, lets resolve it here: If the colors need to be adjusted for dark mode, then they must be themeable and that is could be done outside that PR (even if it is not changing light mode it is big anyway). |
In this case it was just feedback for the reset/settings colors in dark mode, if more adjustments are needed. |
I haven't used it enough to form an opinion. Yet. |
Proposed changes
Use the same colors in the settings check as in reset branch. These colors also look OK in dark mode.
Use a common definition of the bright colors.
Screenshots
Before
Unchanged

After
Good enough for me in dark mode (can be tuned to be slightly darker but better than adapting current colors, there are less colors to customize if that is done).
As a reference, the adapted colors in dark mode
Test methodology
Manual
Merge strategy
I agree that the maintainer squash merge this PR (if the commit message is clear).
✒️ I contribute this code under The Developer Certificate of Origin.