-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Update AlternateSystemColors of the KnownColorTable to match expected value #123169
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
Update AlternateSystemColors of the KnownColorTable to match expected value #123169
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes incorrect color values in the AlternateSystemColors array used for dark mode in Windows Forms. Five system colors were returning incorrect values (black or near-black/gray) when they should have been returning white (0xFFFFFFFF) according to their documentation.
Changes:
- Fixed HighlightText, MenuText, WindowFrame, WindowText, and ButtonHighlight to return white (0xFFFFFFFF) instead of black/dark gray values in dark mode
// These values were based on manual investigation of dark mode themes in the
// Win32 Common Controls and WinUI. There aren't direct mappings published by
// Windows, these may change slightly when this feature is finalized to make
// sure we have the best experience in hybrid dark mode scenarios (mixing
// WPF, WinForms, and WinUI).I’m curious about how the error occurred. Why are there 5 incorrect values and all related to white color? Is there anything else that needs to be fixed? |
|
Tagging subscribers to this area: @dotnet/area-system-drawing |
|
Based on the historical records, this enumeration class has been in its current state since its initial addition, and no one has modified it. Furthermore, according to the original comment, Windows does not provide a direct mapping, so these values are not the final or optimal mapping values. The current changes are only due to customer feedback – in dark mode, Regarding the other items, I'm not sure if they are reasonable. @JeremyKuhne @KlausLoeffelmann How can we determine their reasonableness? Do you have any good suggestions on this? |
|
All fair points. As per our discussions, the right choice of color in terms of what "counts" as the correct complementary @JeremyKuhne and I had a longer discussing at the time about this, and if I remember correctly, on this, we were pretty quickly in an agreement that
We have done quite some research in different UIs stacks to find a good and accessible compromise. Have we done it correctly? I don't know if there is a real "correct" per se. The moment we would really run into accessibility issues with a common color combination, which worked in classic mode but not in dark mode, we would probably discuss changing it. I agree that that is not the case here. If @JeremyKuhne would also agree, I am seconding closing this PR and leave it as it is. |
|
@KlausLoeffelmann agreed. |
Fixes dotnet/winforms#14140
Update AlternateSystemColors of the KnownColorTable to match expected value