Enable DarkMode Theming #10985

KlausLoeffelmann · 2024-03-03T10:19:55Z

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 · 2024-03-04T03:00:53Z

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)

src/System.Windows.Forms/src/System/Windows/Forms/Control.cs

RussKie · 2024-03-04T05:06:09Z

src/System.Windows.Forms/src/System/Windows/Forms/Control.cs

+                if (this is (Button
+                    or Label
+                    or GroupBox
+                    or Panel
+                    or SplitterPanel
+                    or ListBox
+                    or CheckedListBox
+                    or MaskedTextBox
+                    or NumericUpDown
+                    or CheckBox
+                    or RadioButton
+                    or TreeView
+                    or DataGridView
+                    or TextBox
+                    or RichTextBox
+                    or TabControl
+                    or Form))


[maintenability] please order this list as well or provide clear guidance how this list is to be maintained

I never spent thought about sorting this, since I was in the process of experimenting with what controls do have what effect.

But I think this list maybe should be sorted by inheritance level?
Does that make sense?

I'm not sure an average Igor "the Windows Forms developer" would know what right inheritance level order is, but I may be mistaken. Personally, I'd sort this alphabetically, as this is the easiest option.

@KlausLoeffelmann

You need to add apply "DarkMode_Explorer" Theme to MdiClient Control also

if (IsDarkModeEnabled) { if (this is // Controls with 4 levels of inheritance, sorted alphabetically by type name DomainUpDown // Inherits from UpDownBase, ContainerControl, ScrollableControl, Control or NumericUpDown // Inherits from UpDownBase, ContainerControl, ScrollableControl, Control // Controls with 3 levels of inheritance, sorted alphabetically by type name or CheckedListBox // Inherits from ListBox, ListControl, Control or Form // Excluded - too invasive. or FlowLayoutPanel // Inherits from Panel, ScrollableControl, Control or SplitContainer // Inherits from ContainerControl, ScrollableControl, Control or TabPage // Inherits from Panel, ScrollableControl, Control or TableLayoutPanel // Inherits from Panel, ScrollableControl, Control // Controls with 2 levels of inheritance, sorted alphabetically by type name // or ComboBox // Excluded - directly handled. or ListBox // Inherits from ListControl, Control or Button // Inherits from ButtonBase, Control or CheckBox // Inherits from ButtonBase, Control or MaskedTextBox // Inherits from TextBoxBase, Control or Panel // Inherits from ScrollableControl, Control or RadioButton // Inherits from ButtonBase, Control or RichTextBox // Inherits from TextBoxBase, Control or TextBox // Inherits from TextBoxBase, Control or HScrollBar // Inherits from ScrollBar, Control or VScrollBar // Inherits from ScrollBar, Control // Base classes and controls with direct inheritance from Control, sorted alphabetically by type name or ButtonBase // Inherits from Control or DateTimePicker // Inherits from Control // or GroupBox // Inherits from Control directly, but behaves like a container or Label // Inherits from Control or LinkLabel // Inherits from Label, Control // or ListView // Excluded - directly handled. or MonthCalendar // Inherits from Control or PictureBox // Inherits from Control or ProgressBar // Inherits from Control or ScrollableControl // Inherits from Control // or TextBoxBase // Excluded - probably too invasive. or TrackBar // Inherits from Control or TreeView // Inherits from Control or UpDownBase or MdiClient ) // Inherits from Control // Base class for all UI controls in WinForms // or Control // Excluded. { _ = PInvoke.SetWindowTheme(HWND, "DarkMode_Explorer", null); }

This will Apply Dark Theme to mdi Scrollbar that appear When mdi Child Form move out of MdiClient rectangle

before

after

RussKie · 2024-03-04T05:08:15Z

src/System.Windows.Forms/src/System/Windows/Forms/Control.cs

@@ -10853,6 +10991,17 @@ private protected void SetTopLevelInternal(bool value)
        }
    }

+    private static unsafe void PrepareDarkMode(HWND hwnd, bool darkModeEnabled)


Suggested change

private static unsafe void PrepareDarkMode(HWND hwnd, bool darkModeEnabled)

private static unsafe void PrepareDarkMode(HWND hwnd, BOOL darkModeEnabled)

...and perfrom the cast at the callsite. This makes the intent obvious.

Not sold on the method name either, it doesn't feel like it belongs to the vocabulary of this codebase (i.e., it's not very customary to call methods "PerpareXyz" here). It's probably more appropriate to call this SetDarkMode.

It's not a public method?

It is part of the codebase, and it needs to be maintained. The more cohesive the codebase is, the less mental tax is.

I'll take a look.

RussKie · 2024-03-04T05:14:32Z

src/System.Windows.Forms/src/System/Windows/Forms/ActiveX/AxHost.cs

@@ -3361,8 +3361,8 @@ private unsafe bool QuickActivate()
        }
        else
        {
-            qaContainer.colorFore = GetOleColorFromColor(SystemColors.WindowText);
-            qaContainer.colorBack = GetOleColorFromColor(SystemColors.Window);
+            qaContainer.colorFore = GetOleColorFromColor(Application.SystemColors.WindowText);


This change looks like noise, and it makes viewing the actual change much more difficult. Can this be reverted?

No, because it would change the meaning? Or am I missing something?

Oh, I totally missed that there's a new property at the Application type. This is not very good. It'd be better if the new property name wasn't clashing with the existing type name as it will introduce clashes in user code, and it that will need to be reconciled by introducing aliases. This is never pretty.

src/System.Windows.Forms/src/System/Windows/Forms/Application.cs

src/System.Windows.Forms/src/System/Windows/Forms/Controls/ListView/ListView.cs

src/System.Windows.Forms/src/System/Windows/Forms/Controls/ToolStrips/ToolStripManager.cs

src/System.Windows.Forms/src/System/Windows/Forms/DarkModeSettings.cs

src/System.Windows.Forms/src/System/Windows/Forms/Form.cs

lonitra · 2024-03-04T22:22:49Z

src/System.Windows.Forms/src/System/Windows/Forms/Application.cs

+        }
+    }
+
+    public static bool SetDefaultDarkMode(DarkMode darkMode) => darkMode switch


I think the naming here is fine. Another potential name could be to use ColorMode e.g. SetDefaultColorMode . This seems to match the language windows settings use.

I would not go with ColorSomething in the name, since my language feeling would imply theming. Per our first internal discussion, this should not go in that direction, as we would most likely overpromise.

lonitra · 2024-03-04T22:50:25Z

src/System.Windows.Forms/src/System/Windows/Forms/Theming/ForcedLightThemedSystemColors.cs

+
+namespace System.Windows.Forms
+{
+    internal class ForcedLightThemedSystemColors : ThemedSystemColors


Curious why we have both ForcedLightThemedSystemColors and LightThemedSystemColors?

Good question!
I wanted to capture the palette settings for the light mode somewhere, so, if folks want to experiment with deriving/inversing them, not each of us need to do that investigation in a silo. I'll add a comment what's this for!

kirsan31 · 2024-03-07T08:16:02Z

@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 (#3691)?

KlausLoeffelmann · 2024-03-07T10:07:30Z

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 · 2024-03-19T03:06:05Z

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 😄. 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 · 2024-03-20T23:27:39Z

Please read my comments on theming in the respective issue.

Thanks!

AraHaan · 2024-03-21T01:09:23Z

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 · 2024-04-23T16:43:04Z

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 · 2024-04-27T17:34:06Z

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 · 2024-04-28T12:33:55Z

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 · 2024-04-29T23:29:20Z

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 · 2024-04-29T23:29:53Z

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 · 2024-05-04T06:11:17Z

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 · 2024-05-05T20:51:44Z

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

* Take HighContrast settings into account for dark mode availability assessment.

…lors.

…bit.

* Enable PropertyGrid * Fix some code formatting issues * Try easy fixes for Calendar.

KlausLoeffelmann added this to the .NET 9.0 milestone Mar 3, 2024

dotnet-policy-service bot assigned KlausLoeffelmann Mar 3, 2024

dotnet-policy-service bot added the draft draft PR label Mar 3, 2024

RussKie reviewed Mar 4, 2024

View reviewed changes

lonitra reviewed Mar 4, 2024

View reviewed changes

RussKie mentioned this pull request Mar 5, 2024

Migrate theming from net461 to net50 gitextensions/gitextensions#9191

Closed

KlausLoeffelmann changed the title ~~[Proposal] Provide DarkMode Theming~~ Enable DarkMode Theming Mar 6, 2024

This was referenced Mar 6, 2024

Dark mode, the scroll bar theme is incorrect. #10665

Closed

Epic - Align with Win11 look and feel #6270

Open

KlausLoeffelmann changed the base branch from main to Feature/DarkMode March 7, 2024 18:34

KlausLoeffelmann changed the base branch from Feature/DarkMode to main March 7, 2024 19:17

KlausLoeffelmann force-pushed the DarkMode branch from 5b767df to 038bcbf Compare March 7, 2024 19:29

KlausLoeffelmann force-pushed the DarkMode branch from 038bcbf to 3701417 Compare March 18, 2024 18:48

KlausLoeffelmann force-pushed the DarkMode branch 9 times, most recently from fbc724b to 5e189d3 Compare March 27, 2024 05:26

maforget mentioned this pull request Apr 3, 2024

Dark Theme maforget/ComicRackCE#2

Open

KlausLoeffelmann force-pushed the DarkMode branch from 5e189d3 to 8d3b64e Compare April 4, 2024 19:44

xPaw mentioned this pull request Apr 17, 2024

[API Suggestion] Introduce Dark Mode for Apps targeting .NET 9 and Win 11 #7641

Open

20 tasks

KlausLoeffelmann force-pushed the DarkMode branch from 8d3b64e to aaa457e Compare April 20, 2024 19:32

KlausLoeffelmann force-pushed the DarkMode branch from aaa457e to 51a20b9 Compare April 27, 2024 16:21

KlausLoeffelmann added 12 commits May 9, 2024 16:02

First draft of DarkMode implementation.

9b050fe

First draft of Darkmode for ToolStrips.

7526a94

Fix merge issues.

e98a002

Implement Form title bar coloring and

3a90e3c

* Take HighContrast settings into account for dark mode availability assessment.

Implement DarkMode color table and replace SystemColors by dynamic co…

3d23eae

…lors.

Correct Formatting/Merge issues.

c6461e6

Fix rendering ListView ColumnHeaders in wrong text color in dark mode.

726394c

Added more XML comments and refactored the IsDarkModeEnabled logic a …

315ec56

…bit.

Update unshipped API list.

d2702e4

* Enable GroupBox...

5597d7c

* Enable PropertyGrid * Fix some code formatting issues * Try easy fixes for Calendar.

Handle DarkMode ProgressBar.

cf02292

Ensure that IsDarkModeEnabled always returns a valid result.

cd13055

KlausLoeffelmann force-pushed the DarkMode branch from 51a20b9 to cd13055 Compare May 9, 2024 23:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable DarkMode Theming #10985

Enable DarkMode Theming #10985

KlausLoeffelmann commented Mar 3, 2024 •

edited

elachlan commented Mar 4, 2024

RussKie Mar 4, 2024

KlausLoeffelmann Mar 5, 2024

RussKie Mar 6, 2024

memoarfaa May 10, 2024

RussKie Mar 4, 2024

RussKie Mar 4, 2024

KlausLoeffelmann Mar 5, 2024

RussKie Mar 6, 2024

KlausLoeffelmann Mar 7, 2024

RussKie Mar 4, 2024

KlausLoeffelmann Mar 5, 2024

RussKie Mar 6, 2024 •

edited

lonitra Mar 4, 2024

KlausLoeffelmann Mar 5, 2024

lonitra Mar 4, 2024

KlausLoeffelmann Mar 5, 2024

kirsan31 commented Mar 7, 2024 •

edited

KlausLoeffelmann commented Mar 7, 2024 •

edited

AraHaan commented Mar 19, 2024 •

edited

KlausLoeffelmann commented Mar 20, 2024

AraHaan commented Mar 21, 2024 •

edited

AraHaan commented Apr 23, 2024

AraHaan commented Apr 27, 2024 •

edited

AraHaan commented Apr 28, 2024

RussKie commented Apr 29, 2024

RussKie commented Apr 29, 2024

KlausLoeffelmann commented May 4, 2024

AraHaan commented May 5, 2024 •

edited

	private static unsafe void PrepareDarkMode(HWND hwnd, bool darkModeEnabled)
	private static unsafe void PrepareDarkMode(HWND hwnd, BOOL darkModeEnabled)

Enable DarkMode Theming #10985

Are you sure you want to change the base?

Enable DarkMode Theming #10985

Conversation

KlausLoeffelmann commented Mar 3, 2024 • edited

elachlan commented Mar 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RussKie Mar 6, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kirsan31 commented Mar 7, 2024 • edited

KlausLoeffelmann commented Mar 7, 2024 • edited

AraHaan commented Mar 19, 2024 • edited

KlausLoeffelmann commented Mar 20, 2024

AraHaan commented Mar 21, 2024 • edited

AraHaan commented Apr 23, 2024

AraHaan commented Apr 27, 2024 • edited

AraHaan commented Apr 28, 2024

RussKie commented Apr 29, 2024

RussKie commented Apr 29, 2024

KlausLoeffelmann commented May 4, 2024

AraHaan commented May 5, 2024 • edited

KlausLoeffelmann commented Mar 3, 2024 •

edited

RussKie Mar 6, 2024 •

edited

kirsan31 commented Mar 7, 2024 •

edited

KlausLoeffelmann commented Mar 7, 2024 •

edited

AraHaan commented Mar 19, 2024 •

edited

AraHaan commented Mar 21, 2024 •

edited

AraHaan commented Apr 27, 2024 •

edited

AraHaan commented May 5, 2024 •

edited