dotnet / wpf

WPF is a .NET Core UI framework for building Windows desktop applications.
MIT License
7.08k stars 1.17k forks source link

[API Proposal]: Simplify Grid Column and Row Definitions #9802

Open himgoyalmicro opened 1 month ago

himgoyalmicro commented 1 month ago

Background and motivation

Following up on discussion in #5612 and #1739 this proposal is to simplify the definition syntax of RowDefinitions and ColumnDefinitions by:

Goal

The goal of this feature is to make Grid markup less verbose, allowing developers to create grids with simpler syntax.

Example

Current Syntax

Defining columns and rows is overly tedious, repetitive and not very productive, as is shown below:

<Grid>
    <Grid.ColumnDefinitions>
          <ColumnDefinition Width="1*" />
          <ColumnDefinition Width="2*" />
          <ColumnDefinition Width="Auto" />
          <ColumnDefinition Width="*" />
          <ColumnDefinition Width="300" />
    </Grid.ColumnDefinitions>
    <Grid.RowDefinitions>
          <RowDefinition Height="1*" />
          <RowDefinition Height="Auto" />
          <RowDefinition Height="25" />
          <RowDefinition Height= "14" />
          <RowDefinition Height="20" />
    </Grid.RowDefinitions>
</Grid>

Proposed Syntax

The same functionality as above with the proposed succinct syntax is shown below:

<Grid ColumnDefinitions="1*, 2*, Auto, *, 300" RowDefinitions="1*, Auto, 25, 14, 20"> </Grid>
<!-- or -->
<Grid ColumnDefinitions="1* 2* Auto * 300" RowDefinitions="1* Auto 25 14 20"> </Grid>

Scope

Feature Priority
Ability for rows and columns to be defined as a collection or list of values Must
Content properties of RowDefinition and ColumnDefinition are set to Height and Width, respectively Must
Original syntax will still be fully supported and functional Must
Include support for min/max height/width Won't
Include support for data binding within definitions Won't

API Proposal

Provide new typeconverters for ColumnDefinitionCollection and RowDefinitionCollection. This will allow us to convert string input into the corresponding collection.

namespace System.Windows.Controls
{

+    public class ColumnDefinitionCollectionConverter : TypeConverter
+    {
+        ...
+    }

+    public class RowDefinitionCollectionConverter : TypeConverter
+    {
+       ...
+    }

}

Introduce Setter properties for ColumnDefinitons and RowDefinitions of Grid.

+[TypeConverter(typeof(ColumnDefinitionCollectionConverter))]
[DesignerSerializationVisibility(DesignerSerializationVisibility.Content)]
public ColumnDefinitionCollection ColumnDefinitions
{
    get { ... }
+    set { ... }
}

+[TypeConverter(typeof(RowDefinitionCollectionConverter))]
[DesignerSerializationVisibility(DesignerSerializationVisibility.Content)]
public RowDefinitionCollection RowDefinitions
{
    get { ... }
+    set { ... }
}

API Usage

<Grid ColumnDefinitions="1*, 2*, Auto, *, 300" RowDefinitions="1*, Auto, 25, 14, 20"> </Grid>
<Grid ColumnDefinitions="1* 2* Auto * 300" RowDefinitions="1* Auto 25 14 20"> </Grid>
<Grid RowDefinitions="1*, Auto, 25, 14, 20">
    <Grid.ColumnDefinitions>
          <ColumnDefinition Width="1*" />
          <ColumnDefinition Width="2*" />
          <ColumnDefinition Width="Auto" />
          <ColumnDefinition Width="*" />
          <ColumnDefinition Width="300" />
    </Grid.ColumnDefinitions>
</Grid>
<Grid ColumnDefinitions="1*, 2*, Auto, *, 300">
    <Grid.RowDefinitions>
          <RowDefinition Height="1*" />
          <RowDefinition Height="Auto" />
          <RowDefinition Height="25" />
          <RowDefinition Height= "14" />
          <RowDefinition Height="20" />
    </Grid.RowDefinitions>
</Grid>

Alternative Designs

Introduce new public dependency properties Columns and Rows that provide a dedicated place to update the row and column definitions.

+public string Columns
+{
+    get { ... }
+    set { ... }
+}

+public string Rows
+{
+    get { ... }
+    set { ... }
+}

+public static readonly DependencyProperty ColumnsProperty =
+    DependencyProperty.Register(
+        nameof(Columns),
+        typeof(string),
+        typeof(Grid),
+        new FrameworkPropertyMetadata(null, OnColumnsChanged));

+public static readonly DependencyProperty RowsProperty =
+    DependencyProperty.Register(
+        nameof(Rows),
+        typeof(string),
+        typeof(Grid),
+        new FrameworkPropertyMetadata(null, OnRowsChanged));

Remarks

However, this leaves us with two similar properties to set the same things, which is not a clean approach.

Risks

No response

miloush commented 1 month ago

This looks like a reasonably conservative approach, I support the proposed syntax (where the commas are optional like in other collections). The Alternative Design is obviously not viable, not only because of duplication but also because of the ability to change the value once set, which brings challenges of preserving instances of individual definitions etc.

The setter does not need to be public I believe, internal would do. Introducing a public setter brings complexity in terms of managing changes to the value and ownership.

Creating the collections is not that easy though, as they do not have a public constructor and require an instance of the owning Grid. During XAML parsing, the type converter could get the instance of the Grid from ITypeDescriptorContext resp. IProvideValueTarget and could use the internal constructor. However, that means the converters cannot be used on its own without context. Alternatively, we could add public parameter-less constructors and deal with ownership on assignment.

h3xds1nz commented 1 month ago

I agree with miloush here; and yeah, it should be possible to specify both comma-separated and space-separated values as it is with other elements, e.g. Thickness.

himgoyalmicro commented 1 month ago

I agree with miloush here; and yeah, it should be possible to specify both comma-separated and space-separated values as it is with other elements, e.g. Thickness.

@h3xds1nz, @miloush this suggestion is reasonable and I have updated the API Proposal to show both comma-separated and space-separated values.

The setter does not need to be public I believe, internal would do. Introducing a public setter brings complexity in terms of managing changes to the value and ownership.

I am yet to try this one out. Once done I will update the proposal accordingly