[Enhancement] Grid definitions in controls

[proinfocus]

Summary

Simplify the grid definitions in controls

API Changes / Additions

eg:
Existing

<Grid>
  <Grid.ColumnDefinitions>
    <ColumnDefinition Width="*" />
    <ColumnDefinition Width="Auto" />
    <ColumnDefinition Width="*" />
  </Grid.ColumnDefinitions>
  <Grid.RowDefinitions>
    <RowDefinition Height="*" />
    <RowDefinition Height="Auto" />
    <RowDefinition Height="*" />
  </Grid.RowDefinitions>
    <Label Grid.Column="0" Grid.Row="0" Text="ABC" />
    <Label Grid.Column="0" Grid.Row="0" Grid.Colspan="2" Text="ABC" />
    <Label Grid.Column="0" Grid.Row="0" Grid.Colspan="2" Grid.Rowspan="2" Text="ABC" />
</Grid>

Proposed

<Grid>
  <Grid.ColumnDefinitions>
    <ColumnDefinition Width="*" />
    <ColumnDefinition Width="Auto" />
    <ColumnDefinition Width="*" />
  </Grid.ColumnDefinitions>
  <Grid.RowDefinitions>
    <RowDefinition Height="*" />
    <RowDefinition Height="Auto" />
    <RowDefinition Height="*" />
  </Grid.RowDefinitions>
    <Label Grid.Cell="0,0" Text="ABC" />
    <Label Grid.Cell="0,0,2" Text="ABC" />
    <Label Grid.Cell="0,0,2,2" Text="ABC" />
</Grid>

Replacing Column, Row, Colspan and Rowspan with Cell would make the code elegant and compact within grids. This will also help in changing the position more swiftly with less code.

Intended Use Case

Grid is used almost in every page/view in Xamarin.Forms and the controls used within the grid gets too complex just with the addition of column, row, colspan and rowspans. The proposed change is quite similar to what the recent definition compacting has been done by the team and this will extend to the children within the grid and make the code far more simple and compact.

[proinfocus]

or like this:

<Label Grid.At="0,0" Text="ABC" />
<Label Grid.At="0,0,2" Text="ABC" />
<Label Grid.At="0,0,2,2" Text="ABC" />

[davidortinau]

I dig this. Cell might be more appropriate than At. Just my gut reaction.

So I'm clear, what's the proposed order? <Label Grid.Cell="Col,Row,ColSpan,RowSpan"/> as you have listed?

[mattleibow]

It will be a bit confusing to have all in the same place. What about splitting it into:

<Label Grid.Cell="0,0" Grid.Span="2,2" Text="ABC" />

The reason I say this is because you have to know the order. Not hard, but what does things like Margin and Padding do when you only do 3? They are different because 1, 2 and 4 mean things. 3 is weird and I think it might be an error?

I don't know, just throwing some thoughts out.

[proinfocus]

It can be forced to provide either Col,Row or Col,Row,Colspan,Rowspan so it can be pretty much clear and simple in just under a single attribute. If in case either Colspan or Rowspan is not required the user can enter 0 or 1 to indicate that its not spanning...

Grid.Cell="1,1" or Grid.Cell="1,1,0,2"

What you think?

[proinfocus]

I dig this. Cell might be more appropriate than At. Just my gut reaction.

So I'm clear, what's the proposed order? <Label Grid.Cell="Col,Row,ColSpan,RowSpan"/> as you have listed?

Yes, that's perfect!

[StephaneDelcroix]

you know that you can already express Col and RowDefs in a more elegant way than what you're using, right ?

[proinfocus]

you know that you can already express Col and RowDefs in a more elegant way than what you're using, right ?

Yes, I know that. I am proposing compacting of attached properties of Grid, like Grid.Column, Grid.Row, Grid.Colspan and Grid.Rowspan to Grid.Cell or Grid.At

[davidortinau]

Adding this related discussion about adding names microsoft/microsoft-ui-xaml#2094

<Grid ColumnDefinitions="left:100, main:*, right:100"
        RowDefinitions="header:60, body:*, footer:100">
    <Label Grid.Cell="left, header" Text="ABC" />
    <Label Grid.Cell="left, header, 2" Text="ABC" />
    <Label Grid.Cell="main, body" Text="ABC" />
</Grid>

[VincentH-Net]

Adding this related discussion about adding names

Note that if you support names, you can also use begin and end names instead of a span count.
In C# Markup these overloads exist for enums:

Label ("ABC")
      .Row (Rows.Header, Rows.Footer)
      .Column (Columns.A, Columns.D)

The thought behind adding these overloads was that since names mean you don't have to count to specify a position, devs might appreciate if they also don't have to count for spans if they don't prefer that. The layout becomes more functional than calculative

The downside is that it is more verbose. But as an alternative overload that would be the developer's choice

[bmacombe]

I would vote for the cell syntax and think the named would be awesome as well!

[dotMorten]

And what happens if both cell and grid/column/spans are set? This creates a really weird object model with things fighting each other and mysterious precedence

[proinfocus]

If grid/column/spans are set they can take precedent to cell as they are already matured attributes and are used widely now. Grid.Cell can be ignored if individual attributes are set.

[dotMorten]

And now you have a non-functional property. There's really no good way to know something hasn't been set unless you do it programmatically (could be set by control constructor, implicit style etc)

[mattleibow]

The implicit styles is a good call. This is going to make things harder.

Also, what if some "attached" behaviour/effect/style/anything, changes a column/row? Weird things will happen. Libraries probably shouldn't be setting positions, but people do crazy things.

[bmacombe]

Since in the end, it's all setting the row, column and spans anyway I think it's the user's choice if they mix them, between tags, though I wouldn't myself. I would think you should get a compile-time error if you mixed them on the same tag. You can either do the classic attributes, cell, or name...but not a combo of multiple.

[VincentH-Net]

I like it. Cell seems intuitive and is more concise.

It would be simple to add to C# Markup too:

Grid (
    Label ("Hi")
          .Cell (0,0),

    Label ("Hi")
          .Cell (0,0,2),

    Label ("Hi")
          .Cell (0,0,2,2)
)

We already have enums for Grid row and column names in C# Markup, it would be nice if XAML also supported names. Names could then be included in a conversion tool.

[StephaneDelcroix]

just like @dotMorten , I'm torn on this one. I like the simplicity of it, but I hate having multiple (non-deprecated) ways of setting the same property

[proinfocus]

But this one is very much required to make the code look simple. The amount of code required will be much lesser considering Grid is almost always used in every view and it bothers many IMHO to make changes in cols and rows in case the layout changes.

[bmacombe]

@StephaneDelcroix I get where you are coming from. I would be ok with deprecating the old attributes in favor of cell personally. But how is this different then new column and row definitions?

If cell and names are two much, which I could see it being...I'd favor cell.

[dotMorten]

Why not instead create a new grid type where you don't need to set row/col at all, but the order means the cell they go to (and the option to set the span if you need). That's really what we mostly want to do anyway.

[dotMorten]

My definitions proposal didn't add any new properties or types. It just defines a shorthand syntax for that very property. You can't set it again with the old syntax since you can't set a property twice in xaml

[proinfocus]

@mattleibow I highly doubt any developer would confuse with it. Once they get Grid.Cell why would they type 2 extra properties for the same purpose? Of course if the developer hasn't come across Grid.Cell would never add it unknowningly. What you think?

[bmacombe]

What about adding this to the Community Toolkit? Much more opt-in that way?

[proinfocus]

@dotMorten you can set definitions both the old and new ways (I tried just to confirm - I am using Xamarin.Forms (4.7.0.1142)) and it accepts without any issue. So it makes the same case as what we were discussing here...

[dotMorten]

@proinfocus not sure what you're getting at? I'm not saying you can't. I'm saying it's IMHO poor API design to have both and it creates confusion on how each thing work, presedence issues, styling confusion, properties non-functional due to presedence, some values would return the wrong thing on inspection etc. IMHO the ship on this design sailed years ago. If you need this create a new type of grid (see my suggestion earlier with an auto flow option so you don't even have to set the cell property)

[proinfocus]

If anyone is interested, here is the code which adds an attached property - Grid.Cell

using System;
using Xamarin.Forms;

namespace XamarinPlayground.CustomControls
{
    /// <summary>
    /// Custom Grid control having Cell as attached property for
    /// compact definition of Column, Row, Colspan and Rowspan
    /// for any given child controls.
    /// </summary>
    public class Grid : Xamarin.Forms.Grid
    {
        /// <summary>
        /// Define Column, Row, Colspan and/or Rowspan.
        /// </summary>
        public static readonly BindableProperty CellProperty =
            BindableProperty.CreateAttached("Cell", typeof(string), typeof(Grid), null,
                propertyChanged: (BindableObject bindable, object oldValue, object newValue) =>
                {
                    if (!(bindable is VisualElement ve))
                        return;

                    try
                    {
                        if (string.IsNullOrEmpty(newValue.ToString()))
                            return;

                        var values = newValue.ToString().Split(',');

                        if (values.Length >= 1)
                            SetColumn(ve, Convert.ToInt32(values[0].Trim()));
                        else
                            return;

                        if (values.Length >= 2)
                            SetRow(ve, Convert.ToInt32(values[1].Trim()));
                        else
                            return;

                        if (values.Length >= 3)
                            SetColumnSpan(ve, Convert.ToInt32(values[2].Trim()));
                        else
                            return;

                        if (values.Length >= 4)
                            SetRowSpan(ve, Convert.ToInt32(values[3].Trim()));
                    }
                    catch (Exception ex)
                    {
                        Console.WriteLine("Error: " + ex.Message);
                    }
                });

        public static string GetCell(BindableObject view) => (string)view.GetValue(CellProperty);
        public static void SetCell(BindableObject view, string value) => view.SetValue(CellProperty, value);        
    }

    public static class GridMarkup
    {
        public static string Cell<TView>(this TView view, string definitions) where TView : VisualElement
        {
            if (!string.IsNullOrEmpty(definitions))
                Grid.SetCell(view, definitions);

            return Grid.GetCell(view);
        }
    }
}

[bmacombe]

ah, I'm tracking now, that makes sense...Thanks for explaining.

This is probably an unconventional getter on a bindable property. But could

public static string GetCell(BindableObject view) => (string)view.GetValue(CellProperty);

modified to return the current state instead of just calling GetValue?

[bmacombe]

oh, duh...yeah that will mess with the binding system, wouldn't it?

Because the actual property isn't always where the value gets pulled from.

[dotMorten]

This is probably an unconventional getter on a bindable property.

But that won't work when you use the more generic object.GetValue(CellProperty). Don't ever override the default implementation of attached and bindable properties.

[bmacombe]

yep, thought of that about 3 secs after I posted that!

I suppose all the various existing attached properties could update "Cell" if they are changed...but I'm sure there are issues with that also...APIs are hard!

Thanks for the discussion though, helping me put together a lot of random things I already knew, but hadn't put together!

[proinfocus]

Can you not just make this a pure attached property on Grid? No need to create a new control... I think...

@mattleibow How can I make it happen? Can you share an example of it?

[bmacombe]

Could the same feature mentioned in this issue be used to create a compiler error if Cell was added and both Cell and Row, Column, Spans where set?

#10847

[proinfocus]

@bmacombe I am sure it can be. When you can check for errors when a property is used more than once, then it can also be checked conditionally for combination of properties...

[SeRgI1982]

What about SharedSizeGroup for columns and rows ?
It would be very helpful in cases where you have CollectionView/ListView and Grid inside ItemTemplate.
Currently, everything flows when you use Auto or * (at least on XF for MacOS).

[bmacombe]

I put up a spec for a grid that implies the row and column of its children from the order they are defined in XAML or added in code to the community toolkit. Check it out and leave any thoughts or comments you have. Thanks!

xamarin/XamarinCommunityToolkit#196

[bmacombe]

There is a PR open to add Implied Order Grid Behavior to the Xamarin Community Toolkit. Hopefully will ship with V1.

xamarin/XamarinCommunityToolkit#274