Darken TabControl in dark mode #12471
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #12471 +/- ##
===================================================
- Coverage 76.13468% 76.13105% -0.00364%
===================================================
Files 3242 3242
Lines 642363 642370 +7
Branches 47271 47272 +1
===================================================
- Hits 489061 489043 -18
- Misses 149751 149782 +31
+ Partials 3551 3545 -6
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
This is not an area I specialize in and could use some guidance. My plan is to help tackle the remaining dark mode help wanted issues if this approach is acceptable. I have done my best to glean from the work that has already been done and have been experimenting with the dark mode themes that I see in .msstyles. Am I correct that in an ideal world these controls would not require this additional styling? |
|
These are also tackled in #11985 |
To me it looks like that PR could be the start of a new API proposal. In this PR, my goal is to fix the issue without introducing any new public APIs. |
| @@ -1771,6 +1779,9 @@ private bool ShouldSerializeItemSize() | |||
| return !_padding.Equals(s_defaultPaddingPoint); | |||
| } | |||
|
|
|||
| private BOOL StyleUpDown(HWND handle) | |||
| => PInvoke.SetWindowTheme(handle, $"{DarkModeIdentifier}_{ExplorerThemeIdentifier}", null).Succeeded; | |||
There was a problem hiding this comment.
We are adding line breaks after the => - https://github.com/dotnet/winforms/blob/main/docs/coding-style.md#line-breaks
There was a problem hiding this comment.
Maybe we should theme only immediate child windows, the ones whose .Parent in this TabControl?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
|
@willibrandon - had you tested tab controls with various child controls in the pages? Including nested tab controls? |
Tanya-Solyanik
added
the
waiting-author-feedback
|
@Tanya-Solyanik - I need 24 to 48 hours before I can get back to this but I am eager to do so. I've been studying quite a lot about color science and WCAG compliant contrast ratios. |
@Tanya-Solyanik - Just getting back to this now. I will manually test the tab control with various child controls in the pages now. In the meantime, do you have suggestions on how I might test this change in an automated way? Does it make sense to put this in an automated test? And lastly, is there already an established way of testing dark mode support? Please let me know, and if you can describe it or show me an example, then I will automate the test and iterate until everyone is confident in this change. I assume that the assertion would be on the color or background of the child controls, as well as any contrast ratio requirements that should be met. Is there anything else that I should consider? |
Fixes #11953
Proposed changes
Customer Impact
Regression?
Risk
Screenshots
Before
After
Test methodology
Test environment(s)
Microsoft Reviewers: Open in CodeFlow