Skip to content

Conversation

gerhardol
Copy link
Member

@gerhardol gerhardol commented Feb 22, 2025

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

image

Unchanged
image

After

image

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).

image

image

As a reference, the adapted colors in dark mode

image

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.

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

@mstv mstv left a comment

Choose a reason for hiding this comment

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

LGTM

@RussKie
Copy link
Member

RussKie commented Feb 23, 2025

As a reference, the adapted colors in dark mode

In the dark mode the text should be light, not dark though.

@RussKie
Copy link
Member

RussKie commented Feb 23, 2025

Other than that, the screenshots look good

@gerhardol
Copy link
Member Author

As a reference, the adapted colors in dark mode

In the dark mode the text should be light, not dark though.

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).

image
image

    public static readonly Color BrightGreen = Color.FromArgb(106, 209, 106);
    public static readonly Color BrightYellow = Color.FromArgb(209, 209, 106);
    public static readonly Color BrightRed = Color.FromArgb(209, 106, 106);

All details will not be fixed in #12111, there are too many issues in .net9 implementation anyway, it will be experimental.

@RussKie
Copy link
Member

RussKie commented Feb 23, 2025

My comment isn't blocking, more like thinking out loud. This can certainly be tinkered with later.
Good job.

@gerhardol
Copy link
Member Author

My comment isn't blocking, more like thinking out loud. This can certainly be tinkered with later. Good job.

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).
Dark mode will be experimental in GE (as Winforms could even remove the feature in .net10) and everything will not be perfect.

@RussKie
Copy link
Member

RussKie commented Feb 24, 2025

I just made a private build of your .net9 theming branch, so it'll take me some time to form tangible feedback. But it looks 🔥🔥🔥🔥
image

@gerhardol
Copy link
Member Author

In this case it was just feedback for the reset/settings colors in dark mode, if more adjustments are needed.

@gerhardol gerhardol mentioned this pull request Feb 24, 2025
@RussKie
Copy link
Member

RussKie commented Feb 25, 2025

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.

@gerhardol gerhardol merged commit 01ed600 into gitextensions:master Feb 25, 2025
4 checks passed
@gerhardol gerhardol deleted the feature/setting-check-colors branch February 25, 2025 21:17
@mstv mstv added this to the v5.3 milestone Mar 20, 2025
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.

3 participants