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

Grid Shorthand Syntax Implementation #6

Open
wants to merge 8 commits into
base: release/9.0
Choose a base branch
from
Open

Grid Shorthand Syntax Implementation #6

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
7a5aa07
Grid Shorthand Syntax Implementation
himgoyalmicro Nov 25, 2024
61376b0
Creating an internal property Owner for CDC
himgoyalmicro Nov 29, 2024
5cab537
Removing ConvertTo Method in CDCConverter
himgoyalmicro Dec 3, 2024
f14d5e3
Adding Owner Check
himgoyalmicro Dec 3, 2024
56975ef
Nit change
himgoyalmicro Dec 3, 2024
f4f03c8
Updated to allow the 'owner' property to be set to null
himgoyalmicro Dec 4, 2024
3af704d
Adding the changes to RowDefinitions
himgoyalmicro Dec 4, 2024
bd9984b
1. Adding Public Constructor to RowDefinitionCollection and ColumnDef…
himgoyalmicro Jan 6, 2025
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: 3 additions & 1 deletion src/Microsoft.DotNet.Wpf/ApiCompat/Baselines/PresentationFramework-ref.baseline.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,9 @@ CannotChangeAttribute : Attribute 'System.Windows.LocalizabilityAttribute' on 'S
CannotChangeAttribute : Attribute 'System.Windows.LocalizabilityAttribute' on 'System.Windows.Controls.Expander' changed from '[LocalizabilityAttribute(0, Readability=0)]' in the contract to '[LocalizabilityAttribute(LocalizationCategory.None, Readability=Readability.Unreadable)]' in the implementation.
CannotChangeAttribute : Attribute 'System.Windows.LocalizabilityAttribute' on 'System.Windows.Controls.Frame' changed from '[LocalizabilityAttribute(16)]' in the contract to '[LocalizabilityAttribute(LocalizationCategory.Ignore)]' in the implementation.
CannotChangeAttribute : Attribute 'System.ComponentModel.DesignerSerializationVisibilityAttribute' on 'System.Windows.Controls.Grid.ColumnDefinitions' changed from '[DesignerSerializationVisibilityAttribute(2)]' in the contract to '[DesignerSerializationVisibilityAttribute(DesignerSerializationVisibility.Content)]' in the implementation.
CannotRemoveAttribute : Attribute 'System.ComponentModel.TypeConverterAttribute' exists on 'System.Windows.Controls.Grid.ColumnDefinitions' in the contract but not the implementation.
CannotChangeAttribute : Attribute 'System.ComponentModel.DesignerSerializationVisibilityAttribute' on 'System.Windows.Controls.Grid.RowDefinitions' changed from '[DesignerSerializationVisibilityAttribute(2)]' in the contract to '[DesignerSerializationVisibilityAttribute(DesignerSerializationVisibility.Content)]' in the implementation.
CannotRemoveAttribute : Attribute 'System.ComponentModel.TypeConverterAttribute' exists on 'System.Windows.Controls.Grid.RowDefinitions' in the contract but not the implementation.
CannotChangeAttribute : Attribute 'System.ComponentModel.DesignerSerializationVisibilityAttribute' on 'System.Windows.Controls.GridView.ColumnHeaderTemplateSelector' changed from '[DesignerSerializationVisibilityAttribute(0)]' in the contract to '[DesignerSerializationVisibilityAttribute(DesignerSerializationVisibility.Hidden)]' in the implementation.
CannotChangeAttribute : Attribute 'System.ComponentModel.DesignerSerializationVisibilityAttribute' on 'System.Windows.Controls.GridView.Columns' changed from '[DesignerSerializationVisibilityAttribute(2)]' in the contract to '[DesignerSerializationVisibilityAttribute(DesignerSerializationVisibility.Content)]' in the implementation.
CannotChangeAttribute : Attribute 'System.Windows.LocalizabilityAttribute' on 'System.Windows.Controls.GridViewColumn' changed from '[LocalizabilityAttribute(0, Readability=0)]' in the contract to '[LocalizabilityAttribute(LocalizationCategory.None, Readability=Readability.Unreadable)]' in the implementation.
Expand Down Expand Up @@ -261,4 +263,4 @@ CannotChangeAttribute : Attribute 'System.ComponentModel.DesignerSerializationVi
CannotChangeAttribute : Attribute 'System.Windows.Markup.DesignerSerializationOptionsAttribute' on 'System.Windows.Markup.XmlAttributeProperties.GetXmlSpace(System.Windows.DependencyObject)' changed from '[DesignerSerializationOptionsAttribute(1)]' in the contract to '[DesignerSerializationOptionsAttribute(DesignerSerializationOptions.SerializeAsAttribute)]' in the implementation.
CannotChangeAttribute : Attribute 'System.ComponentModel.DesignerSerializationVisibilityAttribute' on 'System.Windows.Media.Animation.Storyboard.GetTarget(System.Windows.DependencyObject)' changed from '[DesignerSerializationVisibilityAttribute(0)]' in the contract to '[DesignerSerializationVisibilityAttribute(DesignerSerializationVisibility.Hidden)]' in the implementation.
CannotChangeAttribute : Attribute 'System.Windows.LocalizabilityAttribute' on 'System.Windows.Shapes.Shape' changed from '[LocalizabilityAttribute(0, Readability=0)]' in the contract to '[LocalizabilityAttribute(LocalizationCategory.None, Readability=Readability.Unreadable)]' in the implementation.
Total Issues: 262
Total Issues: 264
2 changes: 2 additions & 0 deletions src/Microsoft.DotNet.Wpf/src/PresentationFramework/PresentationFramework.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -548,6 +548,7 @@
<Compile Include="System\Windows\Controls\CleanUpVirtualizedItemEventArgs.cs" />
<Compile Include="System\Windows\Controls\ClickMode.cs" />
<Compile Include="System\Windows\Controls\ColumnDefinition.cs" />
<Compile Include="System\Windows\Controls\ColumnDefinitionCollectionConverter.cs" />
<Compile Include="System\Windows\Controls\ComboBox.cs" />
<Compile Include="System\Windows\Controls\ComboBoxItem.cs" />
<Compile Include="System\Windows\Controls\ContainerTracking.cs" />
Expand Down Expand Up @@ -740,6 +741,7 @@
<Compile Include="System\Windows\Controls\RealizedColumnsBlock.cs" />
<Compile Include="System\Windows\Controls\RichTextBox.cs" />
<Compile Include="System\Windows\Controls\RowDefinition.cs" />
<Compile Include="System\Windows\Controls\RowDefinitionCollectionConverter.cs" />
<Compile Include="System\Windows\Controls\ScrollChangedEventArgs.cs" />
<Compile Include="System\Windows\Controls\ScrollUnit.cs" />
<Compile Include="System\Windows\Controls\ScrollViewer.cs" />
Expand Down
48 changes: 42 additions & 6 deletions ...icrosoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Controls/ColumnDefinition.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public sealed class ColumnDefinitionCollection : IList<ColumnDefinition> , IList
/// <summary>
/// Default ctor.
/// </summary>
internal ColumnDefinitionCollection(Grid owner)
internal ColumnDefinitionCollection(Grid owner = null)
{
_owner = owner;
PrivateOnModified();
Expand Down Expand Up @@ -393,6 +393,10 @@ bool IList.IsFixedSize
{
get
{
if (_owner == null)
{
return false;
}
return ( _owner.MeasureOverrideInProgress
|| _owner.ArrangeOverrideInProgress );
}
Expand All @@ -406,6 +410,10 @@ bool IList.IsFixedSize
{
get
{
if (_owner == null)
{
return false;
}
return ( _owner.MeasureOverrideInProgress
|| _owner.ArrangeOverrideInProgress );
}
Expand Down Expand Up @@ -511,6 +519,31 @@ internal void InternalTrimToSize()

#region Internal Properties

/// <summary>
/// Owner property.
/// </summary>
internal Grid Owner
himgoyalmicro marked this conversation as resolved.
Show resolved Hide resolved
{
get { return (_owner); }
set
{
if(_owner != null && _owner != value)
{
throw new ArgumentException(SR.Format(SR.GridCollection_InOtherCollection, "value", "ColumnDefinitionCollection"));
}
if (value == null || _owner == value)
{
return;
himgoyalmicro marked this conversation as resolved.
Show resolved Hide resolved
}
_owner = value;
for (int i = 0; i < _size; i++)
{
_owner.AddLogicalChild(_items[i]);
_items[i].OnEnterParentTree();
}
}
}

/// <summary>
/// Internal version of Count.
/// </summary>
Expand Down Expand Up @@ -604,7 +637,7 @@ private void PrivateConnectChild(int index, DefinitionBase value)
_items[index] = value;
value.Index = index;

_owner.AddLogicalChild(value);
_owner?.AddLogicalChild(value);
value.OnEnterParentTree();
}

Expand All @@ -623,7 +656,7 @@ private void PrivateDisconnectChild(DefinitionBase value)
_items[value.Index] = null;
value.Index = -1;

_owner.RemoveLogicalChild(value);
_owner?.RemoveLogicalChild(value);
}

/// <summary>
Expand Down Expand Up @@ -696,8 +729,11 @@ private void PrivateRemove(DefinitionBase value)
private void PrivateOnModified()
{
_version++;
_owner.ColumnDefinitionCollectionDirty = true;
_owner.Invalidate();
if (_owner != null)
{
_owner.ColumnDefinitionCollectionDirty = true;
_owner.Invalidate();
}
}

/// <summary>
Expand Down Expand Up @@ -731,7 +767,7 @@ private void PrivateSetCapacity(int value)
//------------------------------------------------------

#region Private Fields
private readonly Grid _owner; // owner of the collection
private Grid _owner; // owner of the collection
private DefinitionBase[] _items; // storage of items
private int _size; // size of the collection
private int _version; // version tracks updates in the collection
Expand Down
62 changes: 62 additions & 0 deletions .../src/PresentationFramework/System/Windows/Controls/ColumnDefinitionCollectionConverter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.ComponentModel;
using System.Windows.Controls;
using System.Windows.Markup;
using System.Globalization;
using System.Text;
using MS.Internal;

namespace System.Windows.Controls
{
internal sealed class ColumnDefinitionCollectionConverter : TypeConverter
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it makes sense to have create base type converter for code shared between Rows and Columns

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They don't really have any properties in common, the code would have to be split anyway even if it was a single converter. And I would rather have two standalone converters than one relying on type descriptor to figure out what the converter should produce.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @miloush here, there is very less commonality between the two properties and we would need to bifurcate the code to handle the properties.

{
#region Public Methods

/// <summary>
/// CanConvertFrom - Returns whether or not this class can convert from a given type.
/// </summary>
/// <returns>
/// bool - True if this converter can convert from the provided type, false if not.
/// </returns>
/// <param name="typeDescriptorContext"> The ITypeDescriptorContext for this call. </param>
/// <param name="sourceType"> The Type being queried for support. </param>
public override bool CanConvertFrom(ITypeDescriptorContext typeDescriptorContext, Type sourceType)
{
return sourceType == typeof(string);
}

/// <summary>
/// ConvertFrom - Attempt to convert to a ColumnDefinitionCollection from the given object.
/// </summary>
/// <returns>
/// The object which was constructed.
/// </returns>
/// <exception cref="ArgumentNullException">
/// An ArgumentNullException is thrown if the example object is null.
/// </exception>
/// <param name="typeDescriptorContext"> The ITypeDescriptorContext for this call. </param>
/// <param name="cultureInfo"> The CultureInfo which is respected when converting. </param>
/// <param name="value"> The Thickness to convert. </param>
public override object ConvertFrom(ITypeDescriptorContext typeDescriptorContext, CultureInfo cultureInfo, object value)
{
if (value is string input)
{
ColumnDefinitionCollection collection = new ColumnDefinitionCollection();

TokenizerHelper th = new TokenizerHelper(input, cultureInfo);
while (th.NextToken())
{
collection.Add(new ColumnDefinition { Width = GridLengthConverter.FromString(th.GetCurrentToken(), cultureInfo) });
}

return collection;
}
throw GetConvertFromException(value);
}

#endregion Public Methods
}
}
39 changes: 39 additions & 0 deletions src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Controls/Grid.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,7 @@ public bool ShowGridLines
/// <summary>
/// Returns a ColumnDefinitionCollection of column definitions.
/// </summary>
[TypeConverter(typeof(ColumnDefinitionCollectionConverter))]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason why type converter is applied to ColumnDefinitions property rather than class ColumnDefinitionCollection?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would find that a higher risk, as it would affect uses outside of the Grid (and would get returned by TypeDescriptor). The main reason I would be reluctant to do so is because the converter does not really support converting the whole collection. It supports only quite a narrow albeit common case where properties like MinWidth, MaxWidth, Offset or SharedSizeGroup are not set.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is also a discussion that for that reason, the converter might only support conversion from string, not to string.

(And type converter applied to a property was an important distinguishing factor for the XAML parser to allow manipulating collections using attribute, though with public setter we have departed from that requirement.)

[DesignerSerializationVisibility(DesignerSerializationVisibility.Content)]
public ColumnDefinitionCollection ColumnDefinitions
{
Expand All @@ -282,11 +283,30 @@ public ColumnDefinitionCollection ColumnDefinitions

return (_data.ColumnDefinitions);
}
set
{
if (value.Owner == this)
himgoyalmicro marked this conversation as resolved.
Show resolved Hide resolved
{
return;
}
if (value?.Owner is not null)
himgoyalmicro marked this conversation as resolved.
Show resolved Hide resolved
{
throw new ArgumentException(SR.Format(SR.GridCollection_InOtherCollection, "value", "ColumnDefinitionCollection"));
}
_data ??= new ExtendedData();
_data.ColumnDefinitions?.Clear();
dipeshmsft marked this conversation as resolved.
Show resolved Hide resolved
if (value is not null)
{
value.Owner = this;
_data.ColumnDefinitions = value;
}
}
}

/// <summary>
/// Returns a RowDefinitionCollection of row definitions.
/// </summary>
[TypeConverter(typeof(RowDefinitionCollectionConverter))]
[DesignerSerializationVisibility(DesignerSerializationVisibility.Content)]
public RowDefinitionCollection RowDefinitions
{
Expand All @@ -297,6 +317,25 @@ public RowDefinitionCollection RowDefinitions

return (_data.RowDefinitions);
}
set
{
_data ??= new ExtendedData();
_data.RowDefinitions ??= new RowDefinitionCollection(this);
_data.RowDefinitions.Clear();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need to remove the definitions from the parent?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind, Clear() does that internally.

if (value == null)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why initialize _data if value is null

{
return;
}
foreach (RowDefinition rowDef in value)
{
if (rowDef.Parent != null)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this throws when the definition is already the correct owner/parent

e.g. grid.ColumnDefinitions = grid.ColumnDefinitions would throw and shouldn't

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah except you clear everything first and then readd it.

{
throw new ArgumentException(SR.Format(SR.GridCollection_InOtherCollection, "value", "RowDefinitionCollection"));
}
rowDef.Index = -1;
_data.RowDefinitions.Add(rowDef);
}
}
}

#endregion Public Properties
Expand Down
21 changes: 16 additions & 5 deletions src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Controls/RowDefinition.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public sealed class RowDefinitionCollection : IList<RowDefinition> , IList
/// <summary>
/// Default ctor.
/// </summary>
internal RowDefinitionCollection(Grid owner)
internal RowDefinitionCollection(Grid owner = null)
{
_owner = owner;
PrivateOnModified();
Expand Down Expand Up @@ -393,6 +393,10 @@ bool IList.IsFixedSize
{
get
{
if (_owner == null)
{
return false;
}
return ( _owner.MeasureOverrideInProgress
|| _owner.ArrangeOverrideInProgress );
}
Expand All @@ -406,6 +410,10 @@ bool IList.IsFixedSize
{
get
{
if (_owner == null)
{
return false;
}
return ( _owner.MeasureOverrideInProgress
|| _owner.ArrangeOverrideInProgress );
}
Expand Down Expand Up @@ -604,7 +612,7 @@ private void PrivateConnectChild(int index, DefinitionBase value)
_items[index] = value;
value.Index = index;

_owner.AddLogicalChild(value);
_owner?.AddLogicalChild(value);
value.OnEnterParentTree();
}

Expand All @@ -623,7 +631,7 @@ private void PrivateDisconnectChild(DefinitionBase value)
_items[value.Index] = null;
value.Index = -1;

_owner.RemoveLogicalChild(value);
_owner?.RemoveLogicalChild(value);
}

/// <summary>
Expand Down Expand Up @@ -696,8 +704,11 @@ private void PrivateRemove(DefinitionBase value)
private void PrivateOnModified()
{
_version++;
_owner.RowDefinitionCollectionDirty = true;
_owner.Invalidate();
if (_owner != null)
{
_owner.RowDefinitionCollectionDirty = true;
_owner.Invalidate();
}
}

/// <summary>
Expand Down
Loading