Fix dark mode text visibility in Collection Editor dialogs #14129
Conversation
Co-authored-by: SimonZhao888 <[email protected]>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #14129 +/- ##
===================================================
- Coverage 77.15337% 77.15258% -0.00079%
===================================================
Files 3279 3279
Lines 645325 645329 +4
Branches 47720 47721 +1
===================================================
- Hits 497890 497888 -2
- Misses 143748 143750 +2
- Partials 3687 3691 +4
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
SimonZhao888
requested review from
KlausLoeffelmann,
LeafShi1,
Shyam-Gupta and
ricardobossan
There was a problem hiding this comment.
Pull request overview
This PR fixes dark mode text visibility issues in Collection Editor dialogs by changing the selected item text color from SystemColors.HighlightText to SystemColors.ControlText when dark mode is enabled, following the established pattern from CheckedListBox.
Key changes:
- Modified text color selection logic to use
SystemColors.ControlTextin dark mode for better contrast - Applied WFO5001 pragma suppression consistent with other dark mode implementations
...s.Design/src/System/ComponentModel/Design/CollectionEditor.CollectionEditorCollectionForm.cs
Outdated
Show resolved
Hide resolved
| using (SolidBrush backBrush = new(backColor)) | ||
| { | ||
| g.FillRectangle(backBrush, button); | ||
| } |
There was a problem hiding this comment.
Is this necessary? In most cases, adjusting the text color alone is enough to ensure proper contrast in dark mode.
There was a problem hiding this comment.
Yes, actually the text color is highlight, but backcolor of the button is light color，this caused the text of button can't see clearly, so I adjust backcolor of the button
| { | ||
| backColor = SystemColors.Highlight; | ||
| textColor = SystemColors.HighlightText; | ||
| #pragma warning disable WFO5001 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed. |
There was a problem hiding this comment.
The "Disable warning" option here should no longer be necessary.
...s.Design/src/System/ComponentModel/Design/CollectionEditor.CollectionEditorCollectionForm.cs
Show resolved
Hide resolved
| using (SolidBrush backBrush = new(SystemColors.Window)) | ||
| { | ||
| g.FillRectangle(backBrush, button); | ||
| } |
There was a problem hiding this comment.
Should use modern 'using var' declaration style instead of the traditional using statement block. This reduces nesting and improves readability per the coding guidelines. Change to 'using SolidBrush backBrush = new(SystemColors.Window);' and remove the braces, moving the FillRectangle call outside.
Proposed changes
CollectionEditor.CollectionEditorCollectionFormto useSystemColors.ControlTextin dark mode instead ofSystemColors.HighlightTextCheckedListBoxdark mode handlingCustomer Impact
#2C62B2on near-black#000C39) to compliant contrastRegression?
No
Risk
Low - single line change following established pattern, affects only dark mode rendering
Screenshots
Before
Text invisible due to poor contrast in dark mode:
After
Text visible with proper contrast (pending manual verification)
Test methodology
CheckedListBox.csimplementationTabControl+PropertyGrid, setApplication.SetColorMode(SystemColorMode.Dark), verify text visibility in Collection EditorAccessibility testing
Manual accessibility testing required post-build to verify WCAG AA contrast compliance.
Test environment(s)
Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
rcxvsblobprodcus328.vsblob.vsassets.io/home/REDACTED/work/winforms/winforms/.dotnet/dotnet /home/REDACTED/work/winforms/winforms/.dotnet/dotnet msbuild /m /nologo /clp:Summary /v:minimal /nr:true /warnaserror /p:TreatWarningsAsErrors=true /p:ContinuousIntegrationBuild=false /home/REDACTED/work/winforms/winforms/artifacts/toolset/restore.proj /t:__WriteToolsetLocation /clp:ErrorsOnly;NoSummary /p:__ToolsetLocationOutputFile=/home/REDACTED/work/winforms/winforms/artifacts/toolset/11.0.0-beta.25609.108.txt(dns block)If you need me to access, download, or install something from one of these locations, you can either:
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.
Microsoft Reviewers: Open in CodeFlow