Skip to content

Commit 6d4ea1a

Browse files
Address review suggestions.
1 parent e079671 commit 6d4ea1a

25 files changed

+1091
-1156
lines changed

src/System.Windows.Forms/System/Windows/Forms/Controls/Buttons/Button.cs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -75,13 +75,11 @@ public AutoSizeMode AutoSizeMode
7575

7676
protected override AccessibleObject CreateAccessibilityInstance() => new ButtonAccessibleObject(this);
7777

78-
internal override ButtonBaseAdapter CreateFlatAdapter() => new ButtonFlatAdapter(this);
78+
internal override ButtonBaseAdapter CreateFlatAdapter() => DarkModeAdapterFactory.CreateFlatAdapter(this);
7979

80-
internal override ButtonBaseAdapter CreatePopupAdapter() => new ButtonPopupAdapter(this);
80+
internal override ButtonBaseAdapter CreatePopupAdapter() => DarkModeAdapterFactory.CreatePopupAdapter(this);
8181

82-
internal override ButtonBaseAdapter CreateStandardAdapter() => new ButtonStandardAdapter(this);
83-
84-
internal override ButtonBaseAdapter CreateDarkModeAdapter() => new ButtonDarkModeAdapter(this);
82+
internal override ButtonBaseAdapter CreateStandardAdapter() => DarkModeAdapterFactory.CreateStandardAdapter(this);
8583

8684
internal override Size GetPreferredSizeCore(Size proposedConstraints)
8785
{
@@ -123,6 +121,7 @@ protected override CreateParams CreateParams
123121
else
124122
{
125123
cp.Style |= PInvoke.BS_PUSHBUTTON;
124+
126125
if (IsDefault)
127126
{
128127
cp.Style |= PInvoke.BS_DEFPUSHBUTTON;

src/System.Windows.Forms/System/Windows/Forms/Controls/Buttons/ButtonBase.cs

Lines changed: 76 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,44 @@ protected internal bool IsDefault
337337
/// <summary>
338338
/// Gets or sets the flat style appearance of the button control.
339339
/// </summary>
340+
/// <remarks>
341+
/// <para>
342+
/// The <see cref="FlatStyle"/> property determines how the button is rendered. The following values are supported:
343+
/// </para>
344+
/// <list type="bullet">
345+
/// <item>
346+
/// <term><see cref="FlatStyle.Standard"/></term>
347+
/// <description>
348+
/// The default style. The button is not wrapping the system button. It is rendered using the StandardButton adapter.
349+
/// VisualStyleRenderer from the OS is used for certain parts, which may have issues in high-resolution scenarios.
350+
/// Dark mode works to some extent, but improvements are needed.
351+
/// </description>
352+
/// </item>
353+
/// <item>
354+
/// <term><see cref="FlatStyle.Popup"/></term>
355+
/// <description>
356+
/// The button is fully owner-drawn. No rendering is delegated to the OS, not even VisualStyleRenderer.
357+
/// This style works well in dark mode and is fully controlled by the application.
358+
/// 3D effects are expected but may not be rendered; consider revisiting for meaningful styling.
359+
/// </description>
360+
/// </item>
361+
/// <item>
362+
/// <term><see cref="FlatStyle.Flat"/></term>
363+
/// <description>
364+
/// The button is fully owner-drawn, with no OS calls or VisualStyleRenderer usage.
365+
/// This fits modern design language and works well in dark mode.
366+
/// </description>
367+
/// </item>
368+
/// <item>
369+
/// <term><see cref="FlatStyle.System"/></term>
370+
/// <description>
371+
/// The button wraps the system button and is not owner-drawn.
372+
/// No <c>OnPaint</c>, <c>OnPaintBackground</c>, or adapter is involved.
373+
/// In dark mode, this style is used as a fallback for Standard-style buttons.
374+
/// </description>
375+
/// </item>
376+
/// </list>
377+
/// </remarks>
340378
[SRCategory(nameof(SR.CatAppearance))]
341379
[DefaultValue(FlatStyle.Standard)]
342380
[Localizable(true)]
@@ -356,6 +394,7 @@ public FlatStyle FlatStyle
356394
_flatStyle = value;
357395
LayoutTransaction.DoLayoutIf(AutoSize, ParentInternal, this, PropertyNames.FlatStyle);
358396
Invalidate();
397+
359398
UpdateOwnerDraw();
360399
}
361400
}
@@ -409,12 +448,18 @@ public Image? Image
409448
StopAnimate();
410449

411450
_image = value;
451+
412452
if (_image is not null)
413453
{
414454
ImageIndex = ImageList.Indexer.DefaultIndex;
415455
ImageList = null;
416456
}
417457

458+
// If we have an Image, for some flat styles we need to change the rendering approach from
459+
// being a wrapper around the Win32 control to being owner-drawn. The Win32 control does not
460+
// support images in the same flexible way as we need it.
461+
UpdateOwnerDraw();
462+
418463
LayoutTransaction.DoLayoutIf(AutoSize, ParentInternal, this, PropertyNames.Image);
419464
Animate();
420465
Invalidate();
@@ -620,7 +665,14 @@ internal virtual Rectangle OverChangeRectangle
620665
}
621666
}
622667

623-
private protected virtual bool OwnerDraw => FlatStyle != FlatStyle.System;
668+
/// <summary>
669+
/// OwnerDraw ultimately determines, if we're wrapping the respective Win32 control
670+
/// (Button, CheckBox, RadioButton - OwnerDraw == false) or not. When we're not OwnerDraw,
671+
/// both Light- and DarkMode are (and can be) rendered by the System, but then there is
672+
/// no image rendering, and no OnPaint. This is the original behavior of the Win32 controls.
673+
/// </summary>
674+
private protected virtual bool OwnerDraw =>
675+
FlatStyle != FlatStyle.System;
624676

625677
bool? ICommandBindingTargetProvider.PreviousEnabledStatus { get; set; }
626678

@@ -964,40 +1016,35 @@ internal override Size GetPreferredSizeCore(Size proposedConstraints)
9641016
return LayoutUtils.UnionSizes(preferredSize + Padding.Size, MinimumSize);
9651017
}
9661018

967-
#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.
1019+
/// <summary>
1020+
/// Returns an adapter for Rendering one of the FlatStyles. Note, that we always render
1021+
/// buttons ourselves, except when the User explicitly requests FlatStyle.System rendering!
1022+
/// </summary>
9681023
internal ButtonBaseAdapter Adapter
9691024
{
9701025
get
9711026
{
9721027
if (_adapter is null
9731028
|| FlatStyle != _cachedAdapterType)
9741029
{
975-
if (Application.IsDarkModeEnabled && this is Button)
1030+
switch (FlatStyle)
9761031
{
977-
_adapter = CreateDarkModeAdapter();
1032+
case FlatStyle.Standard:
1033+
_adapter = CreateStandardAdapter();
1034+
break;
1035+
case FlatStyle.Popup:
1036+
_adapter = CreatePopupAdapter();
1037+
break;
1038+
case FlatStyle.Flat:
1039+
_adapter = CreateFlatAdapter();
1040+
break;
1041+
default:
1042+
Debug.Fail($"Unsupported FlatStyle: \"{FlatStyle}\"");
1043+
break;
9781044
}
979-
else
980-
{
981-
switch (FlatStyle)
982-
{
983-
case FlatStyle.Standard:
984-
_adapter = CreateStandardAdapter();
985-
break;
986-
case FlatStyle.Popup:
987-
_adapter = CreatePopupAdapter();
988-
break;
989-
case FlatStyle.Flat:
990-
_adapter = CreateFlatAdapter();
991-
break;
992-
default:
993-
Debug.Fail($"Unsupported FlatStyle: \"{FlatStyle}\"");
994-
break;
995-
}
9961045

997-
_cachedAdapterType = FlatStyle;
998-
}
1046+
_cachedAdapterType = FlatStyle;
9991047
}
1000-
#pragma warning restore WFO5001 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.
10011048

10021049
return _adapter;
10031050
}
@@ -1021,14 +1068,6 @@ internal virtual ButtonBaseAdapter CreateStandardAdapter()
10211068
return null;
10221069
}
10231070

1024-
internal virtual ButtonBaseAdapter CreateDarkModeAdapter()
1025-
{
1026-
// When a button-derived class does not have a dedicated DarkMode adapter implementation,
1027-
// we're falling back to the standard adapter, to not _force_ the derived class to implement
1028-
// a dark mode adapter.
1029-
return CreateStandardAdapter();
1030-
}
1031-
10321071
internal virtual StringFormat CreateStringFormat()
10331072
{
10341073
if (Adapter is null)
@@ -1255,7 +1294,11 @@ private void SetFlag(int flag, bool value)
12551294

12561295
private bool ShouldSerializeImage() => _image is not null;
12571296

1258-
private void UpdateOwnerDraw()
1297+
// Indicates whether this control uses owner drawing, enabling UserPaint and determining
1298+
// if we wrap the native Win32 control (OwnerDraw == false) or render it ourselves.
1299+
// Also needed to detect a Dark Mode opt-out for FlatStyle.Standard when system painting
1300+
// cannot be forced.
1301+
private protected void UpdateOwnerDraw()
12591302
{
12601303
if (OwnerDraw != GetStyle(ControlStyles.UserPaint))
12611304
{

src/System.Windows.Forms/System/Windows/Forms/Controls/Buttons/ButtonInternal/ButtonBaseAdapter.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -645,8 +645,7 @@ private ColorOptions CommonRender(IDeviceContext deviceContext) =>
645645
Enabled = Control.Enabled
646646
};
647647

648-
protected ColorOptions PaintRender(IDeviceContext deviceContext)
649-
=> CommonRender(deviceContext);
648+
protected ColorOptions PaintRender(IDeviceContext deviceContext) => CommonRender(deviceContext);
650649

651650
internal static ColorOptions PaintFlatRender(Graphics g, Color foreColor, Color backColor, bool enabled) =>
652651
CommonRender(g, foreColor, backColor, enabled);

src/System.Windows.Forms/System/Windows/Forms/Controls/Buttons/ButtonInternal/ButtonDarkModeAdapter.cs

Lines changed: 0 additions & 128 deletions
This file was deleted.

0 commit comments

Comments
 (0)