Enable DarkMode Theming

[KlausLoeffelmann]

Work in Progress. Fixes #7641.

[Draft Disclaimer]: This is NOT at all in a state for a formal review. I very much want to start a creative discussion about functionality - my overloaded ADHD brain would like to concentrate on that and postpone taking care of formatting and indention issues after we know where we're going with this. I'll be ignoring those comments for now, and address them later, should they still apply.

[elachlan]

Test failures are caused by white space analyzer errors.

##[error]src\System.Windows.Forms\src\System\Windows\Forms\Controls\ToolStrips\StatusStrip.cs(32,1): error SA1028: (NETCORE_ENGINEERING_TELEMETRY=Build) Code should not contain trailing whitespace (https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1028.md)
D:\a\_work\1\s\src\System.Windows.Forms\src\System\Windows\Forms\Controls\ToolStrips\ToolStripManager.cs(573,1): error SA1028: Code should not contain trailing whitespace (https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1028.md) [D:\a\_work\1\s\src\System.Windows.Forms\src\System.Windows.Forms.csproj]
##[error]src\System.Windows.Forms\src\System\Windows\Forms\Controls\ToolStrips\ToolStripManager.cs(573,1): error SA1028: (NETCORE_ENGINEERING_TELEMETRY=Build) Code should not contain trailing whitespace (https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1028.md)
D:\a\_work\1\s\src\System.Windows.Forms\src\System\Windows\Forms\Controls\ToolStrips\ToolStripManager.cs(575,1): error SA1028: Code should not contain trailing whitespace (https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1028.md) [D:\a\_work\1\s\src\System.Windows.Forms\src\System.Windows.Forms.csproj]
##[error]src\System.Windows.Forms\src\System\Windows\Forms\Controls\ToolStrips\ToolStripManager.cs(575,1): error SA1028: (NETCORE_ENGINEERING_TELEMETRY=Build) Code should not contain trailing whitespace (https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1028.md)

[kirsan31]

@KlausLoeffelmann First of all - thank you 🙏

Problematic controls maybe MonthCalendar, DateTimePicker and definitely TabControl at this point.

I understand that I've already annoyed everyone with this question, but still... At this stage, what can you say about Theming and MDI (https://github.com/dotnet/winforms/issues/3691)?

[KlausLoeffelmann]

I understand that I've already annoyed everyone with this question, but still... At this stage, what can you say about Theming and MDI

That it's not at all likely that it will be addressed in the underlaying layer, which WinForms "just" wraps around. I know it's a disappointing condition, and we're affected ourselves: namely the Designer document (Form) Window. We might be able to brush-up the rendering of the Window title in the WinForms Designer, but it'll only be possible with NC-custom painting. That might work in this case, because the Window is pretty static. But I wouldn't touch it, to be honest, for moving parts.

[AraHaan]

I personally do not like the extensive usage of SystemColor in this change. I would prefer the ability to instead pass in (optionally) a type derived from ProfessionalColorTable instead of that. This way not only does it integrate the the existing Renderer types in the API, but also could be used to theme everything as well. Say for example one wants to be able to have a switch from Light, Dark, and Pink mode in their application. With that change they can and besides if we are already exploring theming support might as well invest a little bit extra work to allow custom app defined themes as well :smile:. I have such an application where such a change would eliminate thousands of lines of code where I have to manually draw everything or make "fake" versions of controls within winforms today using picture boxes and svg images that I manipulate the colors on using System.Drawing.Common apis. Of which is a pain in the butt to maintain.

Also if by default the non-client area used said type derived from ProfessionalColorTable to draw itself within winforms I would be happy as well as it is then LESS work that I would need to do for my application as well. Same for scrollbars and other not so friendly controls as well that I would need to "fake" in my application that lies inside of any component in winforms that derive from ScrollableControl (just about everything basically).

Also having it like this would promote people to ship special nuget packages that people can install and pass in to their application to use a theme they created using a type derived from ProfessionalColorTable that said packages make public in order to be used with this type of API to then properly render it's theme everywhere that is possible.

[KlausLoeffelmann]

Please read my comments on theming in the respective issue.

Thanks!

[AraHaan]

Please read my comments on theming in the respective issue.

Thanks!

The SystemColor palette could use the passed in ProfessionalColorTable derived type to provide the colors to use. Doing that could simplify things a bit more.

[AraHaan]

I still would prefer if the changes were ONLY made to ProfessionalColorTable and for that to detect if it should use dark mod or not with a subclass of it that is meant for applications who want to always use the dark colors in their application for everything (including the scrollbars and non-client area stuffs. Also with it using a user provided ProfessionalColorTable instance theey could also define their own themes with their own colors as well and pass it in to say Application.Theme.set(ProfessionalColorTable) (and internally inside of Control.WndProc it will always check Application.Theme.get to obtain the current ProfessionalColorTable instance that is then used to update it's Paint.

[AraHaan]

Please migrate from Application.SystemColors in this change to prefer application registered and provided ProfessionalColorTable subclasses. I also have existing code that provides dark version colors of the items inside of that as well.

Also it would allow for me to have very minimal changes to my code to allow using it globally as well as remove any code that needed to be provided to manually dark theme my program (most of it copied from ShareX btw).

https://github.com/Elskom/Els_kom_new/blob/main/Els_kom/Themes/DarkColorTable.cs https://github.com/Elskom/Els_kom_new/blob/main/Els_kom/Themes/Theme.cs

Basically applications can then do this and register it using Application.SetColorTable(ProfessionalColorTable) and then winforms would use that color table globally from then on until the user tells it to change color tables which would cause a repaint on everything according to the new color table. Also would need a new public method of Application.GetColorTable (or expose it as a property) and it would do the same thing if not BETTER than the SystemColors option. Besides they get used for special coloring on ContextMenuStrips, etc anyways so why not extend it to work with all of Windows Forms for free (along with non-client area painting).

[AraHaan]

I am starting to think that with dark theme support, it might be better to have it mature a bit more and have it be slated for .NET 10, also with more of that time being invested would mean a much deeper integrated dark theme support and with it perhaps support for all controls + the default scrollbars in the ScrollableControls all based on the user's provided ProfessionalColorTable that gets registered into the Application class in WinForms.

[RussKie]

I am starting to think that with dark theme support, it might be better to have it mature a bit more and have it be slated for .NET 10, also with more of that time being invested would mean a much deeper integrated dark theme support and with it perhaps support for all controls + the default scrollbars in the ScrollableControls all based on the user's provided ProfessionalColorTable that gets registered into the Application class in WinForms.

I don't think there's a reason to delay the API, if those are believed to be usable. Those API, however, must be marked as experimental (spec). This would allow to collect valuable feedback, and make any changed in the following release (including the options to completely rewrie or remove the API).

[RussKie]

Please migrate from Application.SystemColors in this change to prefer application registered and provided ProfessionalColorTable subclasses. I also have existing code that provides dark version colors of the items inside of that as well.

Also it would allow for me to have very minimal changes to my code to allow using it globally as well as remove any code that needed to be provided to manually dark theme my program (most of it copied from ShareX btw).

Elskom/Els_kom_new@main/Els_kom/Themes/DarkColorTable.cs Elskom/Els_kom_new@main/Els_kom/Themes/Theme.cs

Basically applications can then do this and register it using Application.SetColorTable(ProfessionalColorTable) and then winforms would use that color table globally from then on until the user tells it to change color tables which would cause a repaint on everything according to the new color table. Also would need a new public method of Application.GetColorTable (or expose it as a property) and it would do the same thing if not BETTER than the SystemColors option. Besides they get used for special coloring on ContextMenuStrips, etc anyways so why not extend it to work with all of Windows Forms for free (along with non-client area painting).

💯

[KlausLoeffelmann]

I agree with Igor here.

And again: we have no plans to go down the theming route. That has way too much potential to mess with existing APIs and screw up existing theming concepts.

With Application.SystemColors you have an easy way to implement DarkMode, but you can also easily break the Ambient Ancestor DarkMode chain at any point and take over manually with your own palette.

Or just opt out of it at the beginning, if you think your third party's theming is doing a visually more compatible job for your target audience.

Thanks

Klaus

[AraHaan]

I agree with Igor here.

And again: we have no plans to go down the theming route. That has way too much potential to mess with existing APIs and screw up existing theming concepts.

With Application.SystemColors you have an easy way to implement DarkMode, but you can also easily break the Ambient Ancestor DarkMode chain at any point and take over manually with your own palette.

Or just opt out of it at the beginning, if you think your third party's theming is doing a visually more compatible job for your target audience.

Thanks

Klaus

I am referring to very simple "themes" provided only by specific colors using the existing ProfessionalColorTable class and any subclasses. As it would be basically the same as the SystemColors option, however more compatible with existing API as then they would need to call 1 or 2 new functions in Application (or a single property with get; set; to set the color table) and it being much like how it would be to go into Windows settings itself and "creating your own color scheme" by changing all of the system colors inside of a "theme" (not really a theme much as it is just a collection of colors).

[KlausLoeffelmann]

I am referring to very simple "themes" provided only by specific colors using the existing ProfessionalColorTable class and any subclasses.

I get what you mean and feel you. But there are more implications if we did that, namely: Accessibility. So, we need to make sure for now, we can adhere without any risks to the requirements for colors and color schemes around A11Y. And for that reason, for this first version of the feature, we cannot open up the theming at this point.

[AraHaan]

I did not mean to do the approve changes there 😄.

[hyperquantum]

Is this PR still going to target .NET 9 or will it be .NET 10? Asking because of issue #11731.

[MagicAndre1981]

1 question. does this PR also handle dark/light theme change in settings app and changes the colors on the fly without closing/reopening the window to take effect?

[RussKie]

1 question. does this PR also handle dark/light theme change in settings app and changes the colors on the fly without closing/reopening the window to take effect?

I don't believe it does. It was called out during the API review.

[AngeloCresta]

1 question. does this PR also handle dark/light theme change in settings app and changes the colors on the fly without closing/reopening the window to take effect?

I don't believe it does. It was called out during the API review.

Is it not possible to add an event when the ColorMode changes? In this way a component that will loop through controls and force repainting in something easy for us to implement and let the 3party controls producers to style the "stock controls" while theming their own ...

[RussKie]

I've gone through 89/139 of files and I don't see how the new "colour system" is expected to work. That is, I see ControlSystemColors being an adapter for the SystemColors. The issue with this - as was pointed during the the API Review - all consumers will have to essentially rewrite their apps to use ControlSystemColors instead of SystemColors. And, if consumers happen to use some 3P controls, which in turn, use SystemColors, those 3P controls will become "unusable" because those controls won't match the colour scheme...

[KlausLoeffelmann]

@RussKie: https://github.com/dotnet/runtime/pull/105525

[KlausLoeffelmann]

Locked the discussion, since we need to separate this out into different PRs. More background info to come soon.

[merriemcgaw]

Important!

I've made the extremely hard decision to bump this feature's release to 10. This was coming in awfully hot for .NET 9 RC1 and wasn't looking like everything we needed to do internally (t's crossed and i's dotted) could be addressed in time to meet code complete, which is in just a couple of days. Rather than try to force it into RC1 I decided the best course of action will be to move it to 10 and have it ready to go well before the first preview. Given the bake time it will have in the next release, I think we can be confident of a successful release in 10.

Those of you who are invested in the Dark Mode and VisualStyles work - we want all of the feedback that we can get. You will be able to start seeing builds with this feature work in a few weeks at most when .NET repos move Main to target .NET 10. We have every confidence in this feature and are looking forward to having it ready for you with the very first builds of .NET 10 for your evaluation.

If you have any questions, please don't hesitate to ask on the main issue #7641.

[RussKie]

Looks like the dark-mode landed in .NET 9 afterall --> https://github.com/dotnet/winforms/pull/11857 🎉

[merriemcgaw]

Indeed it did! We were able to separate it from the bigger Visual Styles work and get it in for .NET 9. Huge Kudos to @KlausLoeffelmann and @JeremyKuhne for the heroics over the weekend to make sure we had this for .NET 9. Without their above and beyond efforts we would not have been able to make code-complete. Please use and provide feedback!