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

Property Store Refactor #11779

Merged
merged 9 commits into from
Aug 1, 2024
Merged

Property Store Refactor #11779

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
5eb33f2
Refactor PropertyStore GetSize/SetSize
elachlan Jul 30, 2024
5d30cba
Remove GetColor/SetColor
elachlan Jul 30, 2024
96f178e
Refactor ContainsInteger usage
elachlan Jul 31, 2024
460e0cf
Refactor RemoveInteger usage
elachlan Jul 31, 2024
da14311
Refactor PropertyStore.RemoveObject usage
elachlan Jul 31, 2024
588fa23
Refactor PropertyStore.GetInteger
elachlan Jul 31, 2024
f09a072
Changes from review
elachlan Aug 1, 2024
e955b5b
fix GetWrapContents
elachlan Aug 1, 2024
bcddb08
Fix GetWrapContents and simplify ShouldSerializeSelectionForeColor
elachlan Aug 1, 2024
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
4 changes: 2 additions & 2 deletions src/System.Windows.Forms/src/System/Windows/Forms/Control.Ime.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,14 +230,14 @@ internal int ImeWmCharsToIgnore
// after all messages are sent, corresponding WM_CHAR messages are also sent. (in non-unicode
// windows two WM_CHAR messages are sent per char in the IME). We need to keep a counter
// not to process each character twice or more.
get => Properties.GetInteger(s_imeWmCharsToIgnoreProperty);
get => Properties.GetValueOrDefault<int>(s_imeWmCharsToIgnoreProperty);
set
{
// WM_CHAR is not send after WM_IME_CHAR when the composition has been closed by either, changing the conversion mode or
// dissociating the IME (for instance when loosing focus and conversion is forced to complete).
if (ImeWmCharsToIgnore != ImeCharsToIgnoreDisabled)
{
Properties.SetInteger(s_imeWmCharsToIgnoreProperty, value);
Properties.AddValue(s_imeWmCharsToIgnoreProperty, value);
}
}
}
Expand Down
42 changes: 21 additions & 21 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 @@ -758,14 +758,14 @@ public virtual object? DataContext

// When DataContext was different than its parent before, but now it is about to become the same,
// we're removing it altogether, so it can inherit the value from its parent.
if (Properties.ContainsObject(s_dataContextProperty) && Equals(ParentInternal?.DataContext, value))
if (Properties.ContainsKey(s_dataContextProperty) && Equals(ParentInternal?.DataContext, value))
{
Properties.RemoveObject(s_dataContextProperty);
Properties.RemoveValue(s_dataContextProperty);
OnDataContextChanged(EventArgs.Empty);
return;
}

Properties.SetObject(s_dataContextProperty, value);
Properties.AddOrRemoveValue(s_dataContextProperty, value);
JeremyKuhne marked this conversation as resolved.
Show resolved Hide resolved
OnDataContextChanged(EventArgs.Empty);
}
}
Expand All @@ -774,7 +774,7 @@ private bool ShouldSerializeDataContext()
=> Properties.ContainsObject(s_dataContextProperty);

private void ResetDataContext()
=> Properties.RemoveObject(s_dataContextProperty);
=> Properties.RemoveValue(s_dataContextProperty);

/// <summary>
/// The background color of this control. This is an ambient property and
Expand Down Expand Up @@ -834,9 +834,9 @@ public virtual Color BackColor
}

Color c = BackColor;
if (!value.IsEmpty || Properties.ContainsObject(s_backColorProperty))
if (!value.IsEmpty || Properties.ContainsKey(s_backColorProperty))
{
Properties.SetColor(s_backColorProperty, value);
Properties.AddValue(s_backColorProperty, value);
}

if (!c.Equals(BackColor))
Expand Down Expand Up @@ -2095,7 +2095,7 @@ public virtual Color ForeColor
{
get
{
Color color = Properties.GetColor(s_foreColorProperty);
Properties.TryGetValue(s_foreColorProperty, out Color color);
if (!color.IsEmpty)
{
return color;
Expand Down Expand Up @@ -2125,9 +2125,9 @@ public virtual Color ForeColor
set
{
Color color = ForeColor;
if (!value.IsEmpty || Properties.ContainsObject(s_foreColorProperty))
if (!value.IsEmpty || Properties.ContainsKey(s_foreColorProperty))
{
Properties.SetColor(s_foreColorProperty, value);
Properties.AddValue(s_foreColorProperty, value);
}

if (!color.Equals(ForeColor))
Expand Down Expand Up @@ -2794,7 +2794,7 @@ internal virtual Control? ParentInternal
internal PropertyStore Properties { get; }

// Returns the value of the backColor field -- no asking the parent with its color is, etc.
internal Color RawBackColor => Properties.GetColor(s_backColorProperty);
internal Color RawBackColor => Properties.GetValueOrDefault<Color>(s_backColorProperty);

/// <summary>
/// Indicates whether the control is currently recreating its handle. This
Expand Down Expand Up @@ -6976,9 +6976,9 @@ protected virtual void OnFontChanged(EventArgs e)

Invalidate();

if (Properties.ContainsInteger(s_fontHeightProperty))
if (Properties.ContainsKey(s_fontHeightProperty))
{
Properties.SetInteger(s_fontHeightProperty, -1);
Properties.AddValue(s_fontHeightProperty, -1);
}

// Cleanup any font handle wrapper.
Expand Down Expand Up @@ -7086,7 +7086,7 @@ protected virtual void OnNotifyMessage(Message m)
[EditorBrowsable(EditorBrowsableState.Advanced)]
protected virtual void OnParentBackColorChanged(EventArgs e)
{
Color backColor = Properties.GetColor(s_backColorProperty);
Color backColor = Properties.GetValueOrDefault<Color>(s_backColorProperty);
if (backColor.IsEmpty)
{
OnBackColorChanged(e);
Expand Down Expand Up @@ -7123,10 +7123,10 @@ protected virtual void OnParentDataContextChanged(EventArgs e)
if (Properties.ContainsObject(s_dataContextProperty))
{
// If this DataContext was the same as the Parent's just became,
if (Equals(Properties.GetObject(s_dataContextProperty), Parent?.DataContext))
if (Equals(Properties.GetValueOrDefault<object>(s_dataContextProperty), Parent?.DataContext))
{
// we need to make it ambient again by removing it.
Properties.RemoveObject(s_dataContextProperty);
Properties.RemoveValue(s_dataContextProperty);

// Even though internally we don't store it any longer, and the
// value we had stored therefore changed, technically the value
Expand Down Expand Up @@ -7209,7 +7209,7 @@ internal virtual void OnParentHandleRecreating()
// Move this control over to the parking window.

// If we left it parented to the parent control, DestroyWindow would force us to destroy our handle as well.
// Temporarilty parenting to the parking window avoids having to recreate our handle from scratch.
// Temporarily parenting to the parking window avoids having to recreate our handle from scratch.

if (IsHandleCreated)
{
Expand All @@ -7220,7 +7220,7 @@ internal virtual void OnParentHandleRecreating()
[EditorBrowsable(EditorBrowsableState.Advanced)]
protected virtual void OnParentForeColorChanged(EventArgs e)
{
Color foreColor = Properties.GetColor(s_foreColorProperty);
Color foreColor = Properties.GetValueOrDefault<Color>(s_foreColorProperty);
if (foreColor.IsEmpty)
{
OnForeColorChanged(e);
Expand Down Expand Up @@ -10686,7 +10686,7 @@ internal virtual bool ShouldPerformContainerValidation()
[EditorBrowsable(EditorBrowsableState.Never)]
internal virtual bool ShouldSerializeBackColor()
{
Color backColor = Properties.GetColor(s_backColorProperty);
Color backColor = Properties.GetValueOrDefault<Color>(s_backColorProperty);
return !backColor.IsEmpty;
}

Expand Down Expand Up @@ -10714,7 +10714,7 @@ private bool ShouldSerializeEnabled()
[EditorBrowsable(EditorBrowsableState.Never)]
internal virtual bool ShouldSerializeForeColor()
{
Color foreColor = Properties.GetColor(s_foreColorProperty);
Color foreColor = Properties.GetValueOrDefault<Color>(s_foreColorProperty);
return !foreColor.IsEmpty;
}

Expand Down Expand Up @@ -10936,9 +10936,9 @@ private protected void SetScaledFont(Font scaledFont, bool raiseOnFontChangedEve
// Dispose old FontHandle.
DisposeFontHandle();

if (Properties.ContainsInteger(s_fontHeightProperty))
if (Properties.ContainsKey(s_fontHeightProperty))
{
Properties.SetInteger(s_fontHeightProperty, scaledFont.Height);
Properties.AddValue(s_fontHeightProperty, scaledFont.Height);
}

if (!raiseOnFontChangedEvent)
Expand Down
63 changes: 20 additions & 43 deletions src/System.Windows.Forms/src/System/Windows/Forms/Controls/ComboBox/ComboBox.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -447,18 +447,14 @@ public DrawMode DrawMode
[SRDescription(nameof(SR.ComboBoxDropDownWidthDescr))]
public int DropDownWidth
{
get
{
int dropDownWidth = Properties.GetInteger(s_propDropDownWidth, out bool found);
return found ? dropDownWidth : Width;
}
get => Properties.TryGetValue(s_propDropDownWidth, out int dropDownWidth) ? dropDownWidth : Width;
set
{
ArgumentOutOfRangeException.ThrowIfNegativeOrZero(value);

if (Properties.GetInteger(s_propDropDownWidth) != value)
if (Properties.GetValueOrDefault<int>(s_propDropDownWidth) != value)
{
Properties.SetInteger(s_propDropDownWidth, value);
Properties.AddValue(s_propDropDownWidth, value);
if (IsHandleCreated)
{
PInvoke.SendMessage(this, PInvoke.CB_SETDROPPEDWIDTH, (WPARAM)value);
Expand All @@ -477,35 +473,24 @@ public int DropDownWidth
[DefaultValue(106)]
public int DropDownHeight
{
get
{
int dropDownHeight = Properties.GetInteger(s_propDropDownHeight, out bool found);
if (found)
{
return dropDownHeight;
}
else
{
return DefaultDropDownHeight;
}
}
get => Properties.TryGetValue(s_propDropDownHeight, out int dropDownHeight) ? dropDownHeight : DefaultDropDownHeight;
set
{
ArgumentOutOfRangeException.ThrowIfNegativeOrZero(value);

if (Properties.GetInteger(s_propDropDownHeight) != value)
if (Properties.GetValueOrDefault<int>(s_propDropDownHeight) != value)
{
Properties.SetInteger(s_propDropDownHeight, value);
Properties.AddValue(s_propDropDownHeight, value);

// The dropDownHeight is not reflected unless the
// combobox integralHeight == false..
// ComboBox integralHeight == false..
IntegralHeight = false;
}
}
}

/// <summary>
/// Indicates whether the DropDown of the combo is currently dropped down.
/// Indicates whether the DropDown of the combo is currently dropped down.
/// </summary>
[Browsable(false)]
[DesignerSerializationVisibility(DesignerSerializationVisibility.Hidden)]
Expand Down Expand Up @@ -636,37 +621,29 @@ public int ItemHeight
drawMode == DrawMode.OwnerDrawVariable ||
!IsHandleCreated)
{
int itemHeight = Properties.GetInteger(s_propItemHeight, out bool found);
if (found)
{
return itemHeight;
}
else
{
return FontHeight + 2;
}
return Properties.TryGetValue(s_propItemHeight, out int itemHeight) ? itemHeight : FontHeight + 2;
}

// Note that the above if clause deals with the case when the handle has not yet been created
Debug.Assert(IsHandleCreated, "Handle should be created at this point");

int h = (int)PInvoke.SendMessage(this, PInvoke.CB_GETITEMHEIGHT);
if (h == -1)
int height = (int)PInvoke.SendMessage(this, PInvoke.CB_GETITEMHEIGHT);
if (height == -1)
{
throw new Win32Exception();
}

return h;
return height;
}
set
{
ArgumentOutOfRangeException.ThrowIfNegativeOrZero(value);

ResetHeightCache();

if (Properties.GetInteger(s_propItemHeight) != value)
if (Properties.GetValueOrDefault<int>(s_propItemHeight) != value)
{
Properties.SetInteger(s_propItemHeight, value);
Properties.AddValue(s_propItemHeight, value);
if (DrawMode != DrawMode.Normal)
{
UpdateItemHeight();
Expand Down Expand Up @@ -759,7 +736,7 @@ public int MaxLength
{
get
{
return Properties.GetInteger(s_propMaxLength);
return Properties.GetValueOrDefault<int>(s_propMaxLength);
}
set
{
Expand All @@ -770,7 +747,7 @@ public int MaxLength

if (MaxLength != value)
{
Properties.SetInteger(s_propMaxLength, value);
Properties.AddValue(s_propMaxLength, value);
if (IsHandleCreated)
{
PInvoke.SendMessage(this, PInvoke.CB_LIMITTEXT, (WPARAM)value);
Expand Down Expand Up @@ -3201,12 +3178,12 @@ private void ResetAutoCompleteCustomSource()

private void ResetDropDownWidth()
{
Properties.RemoveInteger(s_propDropDownWidth);
Properties.RemoveValue(s_propDropDownWidth);
}

private void ResetItemHeight()
{
Properties.RemoveInteger(s_propItemHeight);
Properties.RemoveValue(s_propItemHeight);
}

public override void ResetText()
Expand Down Expand Up @@ -3433,15 +3410,15 @@ private bool ShouldSerializeAutoCompleteCustomSource()

internal bool ShouldSerializeDropDownWidth()
{
return (Properties.ContainsInteger(s_propDropDownWidth));
return (Properties.ContainsKey(s_propDropDownWidth));
}

/// <summary>
/// Indicates whether the itemHeight property should be persisted.
/// </summary>
internal bool ShouldSerializeItemHeight()
{
return (Properties.ContainsInteger(s_propItemHeight));
return (Properties.ContainsKey(s_propItemHeight));
}

/// <summary>
Expand Down
Loading