Skip to content

.net9 dark theme #12111

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

Merged
merged 3 commits into from
Mar 23, 2025
Merged

Conversation

gerhardol
Copy link
Member

@gerhardol gerhardol commented Dec 22, 2024

Proposed changes

Theming with dotnet/winforms#11857
This is not as good looking as what @NikolayXHD did for GE 3.x, but it follows the standard Windows dark mode (there is a dark+ theme variant that stretches the design and is slightly darker).
There are many limitations and issues in dotnet/winforms#11857 still, most are supposedly planned to be fixed for .net10.
The standard theme is not modified by this PR.

This PR has a number of tweaks to the setup to make this mostly usable, there are a number of issues still, maybe some more can be handled in .net9 too.

A few noted issues - many the same in ge 3 dark mode

  • Add some kind of disclaimer for dark mode - at least that it is experimental in .net9. Win10 (and Win11 23H2?) is not expected to fully work too. Probably just in release notes.
  • tooltips and various other like disabled text boxes are bright, no dramatic issue (also reported)
  • Forms extending from GitExtensionsForm are much darker than the "modern" forms based on GitExtensionsDialog. This applies to for instance Settings and FindInFile, total of 20 forms. There is a difference for the light theme too, but the difference is bigger for dark mode. Certainly not in this PR.
  • Some adapted colors are a little too muted, add specific colors? Cleanup the adaption of colors in general?
  • Some application adopted colors do not look great, like syntax highlighting in the editor. The colors are adapted from the hardcoded highlighting per file type, there is no way to set specific colors (or use system colors?).

For .NET10?

Mostly run on Win11, a few visuals on Win10

The darksilver theme was removed, it would be the same as the dark theme as system colors cannot be changed.
highcontrast_dark was removed, not needed.
Invariant theme system colors are hardcoded, required to adjust colors.

I will appreciate if someone that is better in design and override .NET painting (.NET and GUI is not my strength, this is a hobby!) joins.

Some of the commits in this PR may be separated to individual PRs.

Screenshots

Mostly updated with the latest changes

Should this show GE 3.x/5.1/DarkMode?

For now some visual style examples and issues

General look
image

dark+
image

Ansi colors in dark, mostly readable
image

dark+
image

Example of Flat style (as in the PR) vs Standard.
image
image

White background for unreadable textboxes, ox for single line, not for commit textbox
image

image

A few usages of ListView that is not FileStatusList use ListView header
Keep as this until .net10
image

Disabled button text is hard to read
image

Panels in dark+ are darker, but tab foreground is still SystemColors.Windows (and inactive tabs is not so far from the background). Not so off though.
image

Test methodology

Used as primary interface

Test environment(s)

  • Git Extensions 33.33.33
  • Git 2.47.1.windows.1
  • Microsoft Windows NT 10.0.26100.0
  • .NET 9.0.0
  • DPI 96dpi (no scaling)
  • Portable: False
  • Microsoft.WindowsDesktop.App Versions
    Microsoft.WindowsDesktop.App 9.0.0 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

Merge strategy

Rebase merge (PR submitter must change the commit message for the last commit).


✒️ I contribute this code under The Developer Certificate of Origin.

@RussKie
Copy link
Member

RussKie commented Dec 23, 2024

Should this be GE 3.x/5.1/DarkMode?

I think this should be a dedicated branch, until it's deemed satisfactory/release-ready. Dark mode could be enabled via the settings, couldn't it?

@gerhardol
Copy link
Member Author

Should this be GE 3.x/5.1/DarkMode?

I think this should be a dedicated branch, until it's deemed satisfactory/release-ready. Dark mode could be enabled via the settings, couldn't it?

The comment was for screenshots.
As in the other comment, the invariant theme is not changed by this pr.

@gerhardol gerhardol force-pushed the feature/net9-theming branch 5 times, most recently from d69304a to a2be5d3 Compare December 26, 2024 18:41
@gerhardol gerhardol changed the title WIP net9 theming .net9 dark theme Dec 26, 2024
@gerhardol gerhardol force-pushed the feature/net9-theming branch 2 times, most recently from 6c5df92 to e95029b Compare December 27, 2024 22:47
Copy link
Member

@RussKie RussKie left a comment

Choose a reason for hiding this comment

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

🚀


public static bool IsLightTheme()
public static bool IsDarkTheme()
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

The short answer is that ThemeModule.IsDarkTheme cannot be used in Dashboard (where this is used).
I have mostly used Application.IsDarkModeEnabled for consequences of using the experimental dark mode, like overriding buttons to FlatStyle and ThemeModule.IsDarkTheme for other color relatded changes.
It is not exactly the same.

If the concept of theming is narrowed to light/dark for system colors (as WinForms supports) and custom for GE colors, then only Application.IsDarkModeEnabled could be used in all situations.
Then I would like to suppress WFO5001 globally too. I really hope that dark mode is not removed even if all quirks are not fixed for .net10 ((just two issues that I know of remains so GE could live with the current support.) Thanks to .NET engineers that fixed this in the final hours...

Copy link
Member Author

Choose a reason for hiding this comment

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

Change to use the Application API and suppress WFO5001 ?

Copy link
Member Author

Choose a reason for hiding this comment

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

@RussKie Do you have any input in if dark mode is kept in .net10 and if so if it will be experimental?
Can we assume it is going forward and suppress WFO5001, implementing as it will be kept?

Copy link
Member

Choose a reason for hiding this comment

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

I honestly don't know, I've moved from the team few years ago.

@merriemcgaw @KlausLoeffelmann are there any plans you're aware of pulling the plug on the dark theme?

Copy link
Member

Choose a reason for hiding this comment

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

Thank you, @KlausLoeffelmann. This is great to hear.

Please also note this feature for your efforts, which can be easily overlooked.

I'm finding the doc a little vague on the usefulness of this feature. Are there any samples showing the purpose of this?

The other that (so far) have affected GE are in this issue. It is definitely the ListView label that is the most important, but can rank them in the umbrella issue.

This is probably the most visible (or better say "invisible"). Other than that, the readonly colour is the most visible gap:
image

Choose a reason for hiding this comment

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

You mean the SystemColors switch, right?

The idea behind is that it switches the whole color set, albeit not configurable, for the SystemColors. We can override in the WinForms runtime quite a few things to make edge case scenarios work. But there are other cases where controls are really unchangable dependent on SystemColors. And also, if you wrote your own implementations, and used SystemColors, you want to automatically apply that set of dark-mode colors with "invers contrast" so to say, so the DarkMode-migration effort is as minimal as possible.

It not the most spectacular thing alright, and surely not helpful in all the cases. But I just did want you guys not to miss it, since it is kind of hard to notice, and maybe it can come in handy here and there.

And while we're at it:
We're forcing a control into the DarkMode scene with a certain theming option. We know that that's not a perfect solution, but we decided, before we do nothing, we do what's possible, since at this point it is more than a "fashion" thing. I know a whole series of Devs, like me, struggling with ADHD, and for me, and with my current Monitor setting, Dark Mode is a REAL A11Y improvement, but at the same time: Light Mode makes my setup no longer usable. I am getting headaches 10 minutes in. And I hear those things more and more in the community, so, to echo the approach: Let me try to make happen what's somehow possible, but we may still not end up with everything being perfect - just to set expectations right.

Also: We cannot service .NET 9, since we do not service Experimental Features. That is unfortunately true also for features, which worked and regressed - like the ToolStripStatusBar, which doesn't anymore apply the correct background color after an A11Y fix.

But ping me directly for workarounds, I might be able to help with, if you run into something, and should I not react in time, because I am way more than swamped currently, I am absolutely sure Igor has the MS-internal means to ping me in a way that I cannot not notice...

Copy link
Member

Choose a reason for hiding this comment

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

I suppose we can try to spin a .NET 10 build too...

Copy link
Member Author

Choose a reason for hiding this comment

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

@RussKie
Can I merge this before testing further?
It is a one line change to disable dark mode if needed...

Copy link
Member

Choose a reason for hiding this comment

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

Sure thing

@gerhardol
Copy link
Member Author

Dark mode could be enabled via the settings, couldn't it?

default could set darkmode if enabled for apps when this is no longer experimental. Not in this PR...

@pmiossec
Copy link
Member

I've been running the dark+ as I found it more eye-pleasing, hence my suggestion to merge it into the dark theme.

I use it also since some days on daily basis (dark theme) and I found it good enough to be merged also.
There are very few places where there are problems.

But one example ("Active" is not readable):
image

One "painful" problem is when you open an older version and you get this huge popup at startup
image
Maybe not a problem if there is no move back. But I don't know if we should release a bugfix version handling this case gracefully...

@gerhardol
Copy link
Member Author

I've been running the dark+ as I found it more eye-pleasing, hence my suggestion to merge it into the dark theme.

I much prefer dark+ too, but got some negative feedback for similar changes for light mode, so I wanted to separate the setup.
The names matches VS Code, so I hope that is understandable.
But dark could be dark-system or so too, as a reference

We can have a warning in the release notes, I guess.
I vaguely remember bouncing ideas of introducing experimental settings, but I can't recall whether anything was implemented...

Good, no popup then. Release notes is fine for me.
We had an experimental setting in Settings (no longer there), but here it is one of many themes.

I use it also since some days on daily basis (dark theme) and I found it good enough to be merged also. There are very few places where there are problems.
But one example ("Active" is not readable):

This is listed in the description - I have not found a workaround
There are more issues

One "painful" problem is when you open an older version and you get this huge popup at startup

This is probably a problem mostly for developers, switching releases.
When this PR is merged the problem will no longer be seen when working with PRs (but still when testing behavior in older releases), so not seen so much.

@pmiossec
Copy link
Member

There are more issues

I failed to remember one that I found earlier but it came back in my mind: we can't see anymore the selection when doing a blame.

Dark.css:
image

in previous version:
image

@mstv
Copy link
Member

mstv commented Mar 10, 2025

I vaguely remember bouncing ideas of introducing experimental settings

private static readonly SettingsPath ExperimentalSettingsPath = new AppSettingsPath(DetailedSettingsPath, "Experimental");

Currently not used because the regarding settings left the experimental state. Added in 12da3aa.

@mstv
Copy link
Member

mstv commented Mar 10, 2025

(I have no feedback to this PR because my monitors and my eyes work much better in light mode.)

@RussKie
Copy link
Member

RussKie commented Mar 11, 2025

There are more issues

I failed to remember one that I found earlier but it came back in my mind: we can't see anymore the selection when doing a blame.

Dark.css: image

in previous version: image

I think it is still there, just the contrast is very low. I think we'll be tuning and fine-tuning the colours for quite some time after we merge this.

gerhardol added a commit to gerhardol/gitextensions that referenced this pull request Mar 12, 2025
Basic adaption of .net9 experimental dark app mode: SystemColorMode.Dark
The themeing is adopted to use Windows dark app mode enabled in .net9.
The .net9 dark mode does not allow override of any system colors.
A number of workarounds done, but the experimental mode is still
incomplete with some text hard to read.

Add dark+, similar to VS Code Dark+ theme with darker panels and editor.

highconstrast_dark theme was removed, no longer needed

Refs: gitextensions#12111
@gerhardol gerhardol force-pushed the feature/net9-theming branch from 4447974 to 699a539 Compare March 12, 2025 22:53
@gerhardol
Copy link
Member Author

Pushed a few fixes to NBug and suppress WFO5001
#12111 (comment) is not included yet

@mstv
Copy link
Member

mstv commented Mar 13, 2025

src\app\GitUI\UserControls\RevisionGrid\RevisionDataGridView.cs(417,42): error CS0117: "ThemeModule" enthält keine Definition für "IsDarkTheme".

gerhardol added a commit to gerhardol/gitextensions that referenced this pull request Mar 19, 2025
Basic adaption of .net9 experimental dark app mode: SystemColorMode.Dark
The themeing is adopted to use Windows dark app mode enabled in .net9.
The .net9 dark mode does not allow override of any system colors.
A number of workarounds done, but the experimental mode is still
incomplete with some text hard to read.

Add dark+, similar to VS Code Dark+ theme with darker panels and editor.

highconstrast_dark theme was removed, no longer needed

Refs: gitextensions#12111
@gerhardol gerhardol force-pushed the feature/net9-theming branch from 261e47c to d40a7ff Compare March 19, 2025 22:59
@gerhardol
Copy link
Member Author

I failed to remember one that I found earlier but it came back in my mind: we can't see anymore the selection when doing a blame.

I think it is still there, just the contrast is very low. I think we'll be tuning and fine-tuning the colours for quite some time after we merge this.

value is set now, adjusted to the background
Slightly more visible in dark+

@gerhardol
Copy link
Member Author

@gerhardol I played tinkered with the dak theming, and I propose the following changes:

Adapted these suggestions some, the body text in the grid was not set correctly for instance
Also changed FileStatusList

For AuthoredHighlight you had changed the blueish color to gray. I find that still to close to selection colors and set a slightly yellow color. There is not much contrast to play with.

Plan to update screenshots tomorrow
I believe this is ready to merge, to start the finetuning

@gerhardol
Copy link
Member Author

In light mode, the subject/body for the commits in n branches not merged to master are darker, basically the same in dark mode. Will change that first. There is less contrast to play with, will be less visible though.

@gerhardol
Copy link
Member Author

Tweaked the colors some more, I believe this is good enough for usage (and more tweaks).
Will try to compile a list of issues to the Winforms team when this is merged too.

I've been running the dark+ as I found it more eye-pleasing, hence my suggestion to merge it into the dark theme.

I want to keep dark theme as a "system compliant" and dark+ as some stretches to the design that is maybe not as consistent, compare to VS Code themes.
For instance tab headers are not adjusted in dark+ (not a big problem), some popups differ to the background. But dark+ is what I use myself.

For dark mode, skip "darmode_explorer" as changes are minimal.
Some ThemeFix no longer needed for theming with .net9.
For ListBox BorderStyle.Fixed3D does not look great in dark mode but is
rendered correctly and not overridden.

# Conflicts:
#	src/app/GitExtUtils/GitUI/Theming/ThemeFix.cs
Basic adaption of .net9 experimental dark app mode: SystemColorMode.Dark
The themeing is adopted to use Windows dark app mode enabled in .net9.
The .net9 dark mode does not allow override of any system colors.
A number of adaptions of the application colors.
There are some visual issues still, some due to .net,
but also the application colors may require some tuning.

Add dark+ theme, similar to VS Code Dark+ theme with darker panels and editor.

highconstrast_dark theme was removed, no longer needed

Refs: gitextensions#12111

author highlight more subtle
@gerhardol gerhardol force-pushed the feature/net9-theming branch from 43bbeae to 212068b Compare March 23, 2025 22:12
@gerhardol gerhardol merged commit 3efac3a into gitextensions:master Mar 23, 2025
3 of 4 checks passed
@gerhardol gerhardol deleted the feature/net9-theming branch March 23, 2025 22:33
@Bush-cat
Copy link

Just wanted to send a huge THANK YOU for your work and getting the dark theme merged!

@RussKie
Copy link
Member

RussKie commented Mar 24, 2025

@gerhardol, FWIW this could be useful https://devblogs.microsoft.com/dotnet/introducing-winforms-analyzers/.

@mstv mstv added this to the v5.3 milestone Mar 25, 2025
@gerhardol
Copy link
Member Author

A short introduction and issues listed in the wiki
https://github.com/gitextensions/gitextensions/wiki/Dark-Theme

In addition, I renamed files so the repo can be cloned in Windows and applied markdownlint and did a minor cleanup

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants