Skip to content
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.

Sign up for GitHub

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

Darken TabControl in dark mode #12471

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Darken TabControl in dark mode #12471

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ff8cd62
Darken TabControl in dark mode
willibrandon Nov 13, 2024
d32e423
Merge branch 'main' into darken-tab-control-in-dark-mode
willibrandon Jan 27, 2025
4be2d6d
Break the expression body after the =>
willibrandon Jan 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/System.Windows.Forms/src/System/Windows/Forms/Control.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ public unsafe partial class Control :
internal const string ExplorerThemeIdentifier = "Explorer";
internal const string ItemsViewThemeIdentifier = "ItemsView";
internal const string ComboBoxButtonThemeIdentifier = "CFD";
internal const string BannerContainerThemeIdentifier = "FileExplorerBannerContainer";

private const short PaintLayerBackground = 1;
private const short PaintLayerForeground = 2;
Expand Down
11 changes: 11 additions & 0 deletions src/System.Windows.Forms/src/System/Windows/Forms/Controls/TabControl/TabControl.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1289,6 +1289,14 @@ protected override void OnHandleCreated(EventArgs e)
}

UpdateTabSelection(false);

#pragma warning disable WFO5001
if (Application.IsDarkModeEnabled)
{
PInvoke.SetWindowTheme(HWND, null, $"{DarkModeIdentifier}::{BannerContainerThemeIdentifier}");
PInvokeCore.EnumChildWindows(this, StyleUpDown);
}
#pragma warning restore WFO5001
}

protected override void OnHandleDestroyed(EventArgs e)
Expand Down Expand Up @@ -1771,6 +1779,9 @@ private bool ShouldSerializeItemSize()
return !_padding.Equals(s_defaultPaddingPoint);
}

private BOOL StyleUpDown(HWND handle)
=> PInvoke.SetWindowTheme(handle, $"{DarkModeIdentifier}_{ExplorerThemeIdentifier}", null).Succeeded;
Copy link
Member

Choose a reason for hiding this comment

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

We are adding line breaks after the => - https://github.com/dotnet/winforms/blob/main/docs/coding-style.md#line-breaks

Copy link
Member

@Tanya-Solyanik Tanya-Solyanik Jan 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe we should theme only immediate child windows, the ones whose .Parent in this TabControl?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe we should theme only immediate child windows, the ones whose .Parent in this TabControl?

Is this covered in the design document somewhere? 😄

Let me know what you would like me to do, but my thought is if a user enables dark mode and the control is ultimately not dark, not dark enough, or not consistently dark, then the user might object. However, like you eluded to, some users might expect for immediate and child controls to be styled dark, but not any of the grandchildren. It's a little harder for me to envision why users would expect this scenario but my knowledge, experience, and use cases are more limited than yours. If I were the user I would expect a sensible default with an option to override.

Copy link
Member

Choose a reason for hiding this comment

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

I thought that goal of our PR was to theme page headers and the spinner control, page bodies and their children are already themed. Is {DarkModeIdentifier}_{ExplorerThemeIdentifier} an appropriate theme identifier for other windows?


/// <summary>
/// Returns a string representation for this control.
/// </summary>
Expand Down