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.

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

Proposal: Naming Grid Rows/Columns on Simplified Inline Syntax and Grid.RowColumn #2094

Open
rizamarhaban opened this issue Mar 10, 2020 · 25 comments
Labels
area-Layouts feature proposal New feature proposal needs-winui-3 Indicates that feature can only be done in WinUI 3.0 or beyond. (needs winui 3) team-Controls Issue for the Controls team

Comments

@rizamarhaban
Copy link

rizamarhaban commented Mar 10, 2020

Proposal: Naming Grid Rows/Columns on Simplified Inline Syntax and Grid.RowColumn

Summary

This proposal is to add feature to simplify the syntax of Grid Rows, Columns and its implementation:

  • Define Grid Rows and Columns definitions inline. Can use comma or space as the delimeter.
  • Allowing to add variable naming for each row and column index definition value. I propose to use a colon simply [variable name]:[value]. See example below on this.
  • Adding new attribute Grid.RowColumn besides Grid.Row and Grid.Column.
  • Using the variable name as reference as well as index number to assign rows and columns in implementation.

Rationale

  • Currently it is difficult to refactor Grid rows and column. Imagine having so many rows and columns and we want to insert a row or column, we usually do this:
    • Add RowDefinition and ColumnDefinition.
    • Reassign the indexes of the implementation.
  • Switching index is difficult and require to reassign the indexes and we usually make mistake because of the index number.
  • This will add productivity by easy to refactor and easy to trace rows and columns.

Scope

Capability Priority
Original syntax not changed Must
Implementation using indexes not changed (can use indexes or naming) Must
Row/column definitions naming feature to simplify and easy refactoring of Grid Should
Allow inline RowDefinitions and ColumnDefinitions of Grid Should
New attribute Grid.RowColumn for the Row/Column of Grid Should
This model can be used for other similar cases on any element if any Could

Example

Define Grid Rows and Columns for Current Syntax

<Grid>
    <Grid.ColumnDefinitions>
        <ColumnDefinition x:Name="a" Width="1*" />
        <ColumnDefinition x:Name="b" Width="1*" />
        <ColumnDefinition x:Name="c" Width="Auto" />
        <ColumnDefinition x:Name="d" Width="*" />
        <ColumnDefinition x:Name="e" Width="300" />
    </Grid.ColumnDefinitions>
    <Grid.RowDefinitions>
        <RowDefinition x:Name="a" Height="1*" />
        <RowDefinition Height="Auto" />
        <RowDefinition x:Name="i" Height="25" />
        <RowDefinition x:Name="whatever" Height="14" />
        <RowDefinition x:Name="k" Height="20" />
    </Grid.ColumnDefinitions>
</Grid>

Define Grid Rows and Columns Inline compact syntax (NEW API)

<Grid ColumnDefinitions="a:1* b:2* c:Auto d:* e:300"
      RowDefinitions="a:1* Auto i:25 whatever:14 k:20">
</Grid>

Notes: the RowDefinition of Auto above does not have naming. This will fallback to index number 1.

Grid Row and Column Implementation (NEW API)

When we need to reference on which rows and column, we just do:

<TextBlock Grid.Row="a" Grid.Column="b" />
<TextBlock Grid.Row="whatever" Grid.Column="e" />
<TextBlock Grid.Row="1" Grid.Column="c" />  <!-- using index number on Row -->

If no variable name, simply fallback to use index number.

New Attribute Grid.RowColumn (NEW API)

This to simplify without the assignment for the row and column:

<TextBlock Grid.RowColumn="a b" />
<TextBlock Grid.RowColumn="whatever e" />
<TextBlock Grid.RowColumn="1 c" />

With these hopefully easy to refactor as well.
However, this will also allow to use index numbers, i.e.,

<TextBlock Grid.RowColumn="0,1" />
<TextBlock Grid.RowColumn="3,4" />
<TextBlock Grid.RowColumn="1,2" />
@rizamarhaban rizamarhaban added the feature proposal New feature proposal label Mar 10, 2020
@msft-github-bot msft-github-bot added this to Freezer in Feature tracking Mar 10, 2020
@msft-github-bot msft-github-bot added the needs-triage Issue needs to be triaged by the area owners label Mar 10, 2020
@japf
Copy link
Contributor

japf commented Mar 11, 2020

This looks like a duplicate of #673

@Felix-Dev
Copy link
Contributor

Felix-Dev commented Mar 11, 2020

@japf It's not. Issue #673 is about simplifying the syntax used to define rows and columns:

Ability for rows and columns to be defined as a collection or list of values (using CreateFromString attribute)

Adding a name to a row/columm so we can use that name to reference a row/column instead of its index in the row/column collection is purposefully not part of that proposal. This issue here proposes adding that naming capability to rows/columns so that changes like inserting new rows/columns in the grid don't require changing the indices in affected <Control Grid.Row/Column=.../> XAML.

The example you see in this proposal shows how naming rows/columns could work alongside the new syntax proposed in #673.

This issue is part of the very broad Make Grid Better proposal.

@mdtauk
Copy link
Contributor

mdtauk commented Mar 11, 2020

The name should change to reflect to focus on the unique proposal here. Naming Grid Rows and Columns for Simplified Syntax.

@Felix-Dev
Copy link
Contributor

Yes, the name of the proposal should be changed. An example name could be: "Able to refer to rows / columns by name" (based on the Make Grid Better proposal).

@ranjeshj
Copy link
Contributor

It does look very similar to #673 except for the naming aspect. I like the @mdtauk 's idea of scoping this down to just the naming and keep the rest of the discussions in the other proposal. Thanks.

@Felix-Dev
Copy link
Contributor

Felix-Dev commented Mar 11, 2020

Since issue #673 has been greenlighted based on my understanding the idea

Allowing to add variable for each row and column index definition value. I propose to use a colon simply [variable name]:[value]

should be kept in this proposal (i.e. how to name a row/column with the new compact syntax).

@rizamarhaban Right now it seems to me you are proposing naming rows/columns ony when using the new compact syntax. What about adding this capability to the current Grid API/syntax? Should we have a separate issue for that? Do we even need to add naming capability to the current Grid API/syntax or will this be a feature exclusive to the new compact defintion syntax?

@ranjeshj
Copy link
Contributor

@rizamarhaban Thank you for filing the proposal. Can you please update the proposal and scope it down to just naming rows and columns (on top of what is proposed in #673 ) ?

@rizamarhaban
Copy link
Author

@Felix-Dev @ranjeshj I include the syntax simplification like similar in #673 because has not been implemented and I have the same idea but with adding variables or naming row/column. I also add the new attribute Grid.RowColumn which is not the same with #673. I can update the title to reflect this, but this is not just simply inlining the row/col values.

@rizamarhaban rizamarhaban changed the title Proposal: Simplify Grid Rows, Columns Definition and Implementation Proposal: Naming Grid Rows/Columns on Simplified Inline Syntax and Grid.RowColumn Mar 11, 2020
@Felix-Dev
Copy link
Contributor

Felix-Dev commented Mar 11, 2020

@rizamarhaban You seem to propose two new features here - naming rows/columns and a new API for a compact row/column specification (Grid.RowColumn). The naming feature alone should be in its own proposal as suggested by @ranjeshj as it raises a couple of questions worth thinking about. (I.e. will it only work for the new compact syntax or also for the current grid syntax? If the latter, do we need to add new API and how would that look like?)

Given this, it would be best if you would split this proposal into two proposals...one covering the new naming feature and the other one proposing the new Grid.RowColumn API.

@rizamarhaban
Copy link
Author

@Felix-Dev yes it's 2 new features and will add into the scope.

@ranjeshj ranjeshj added area-Layouts needs-winui-3 Indicates that feature can only be done in WinUI 3.0 or beyond. (needs winui 3) and removed needs-triage Issue needs to be triaged by the area owners labels Mar 11, 2020
@michael-hawker
Copy link
Collaborator

Should also be an option name:

<Grid ColumnDefinitions="1* b:2* Auto d:* 300"
      RowDefinitions="1* Auto i:25 14 20">
</Grid>

@rizamarhaban
Copy link
Author

@michael-hawker yes it is. In the example, if there is no variable name then fallback to indexes. We can use both indexes or the name if assigned.

@Felix-Dev
Copy link
Contributor

Felix-Dev commented Mar 12, 2020

@rizamarhaban Please answer the following questions:
Will your naming proposal only work for the new compact syntax or also for the current grid syntax? If the latter, do we need to add new API and how would that look like?

The way I'm reading your current naming proposal you are proposing a feature which would make the current Grid syntax and the new compact syntax in issue #673 no longer equivalent. In other words, you want to add a feature available to the new compaxt syntax only. Issue #673 only proposes a syntax simplication, it does not add new features exclusive to the new compact syntax. Your current proposal would change that and thus this should be discussed.

@rizamarhaban
Copy link
Author

@Felix-Dev

Will your naming proposal only work for the new compact syntax or also for the current grid syntax? If the latter, do we need to add new API and how would that look like?

Technically, this can be use for the current syntax as well. Alternatively, both new compact syntax and current syntax may implement this. Meaning, there will be new API requirements. As this won't break any changes of the current XAML, need to discuss what the API may look like. I will update the Example to include the current syntax as well.

@Felix-Dev
Copy link
Contributor

@rizamarhaban Thanks for updating 🙂

@jtorjo
Copy link

jtorjo commented Mar 16, 2020

This looks like a very very good idea. This way, when re-arranging rows/columns, it will be very easy.

One thing that comes to mind is adding a grid splitter (which takes one row or column) <- this right now requires me manually editing the xaml and hope I don't miss anything. With this proposal, I would not need any xaml editing.

Also, very useful when you need to quickly re-arrange rows (you realize some low row needs to come to the top or so).

@anawishnoff
Copy link
Contributor

Thanks for filing this, @rizamarhaban! Sorry I am just getting around to seeing it. The naming idea (as others mentioned) was brought up in the initial Make Grid Better proposal #54, and it seems like a popular want to help deal with large numbers of rows and columns. I like that your proposal accounts for making sure that it works with the old syntax and the new, compact syntax - we don't want to cause breaking changes or leave pure UWP (non-WinUI 3) apps out of the feature.

In terms of the RowColumn attribute, I was wondering what the motivation was. Is it just a shorter way of assigning placement to elements, instead of writing Grid.Column="0" and Grid.Row="0"? To me, this seems like it would take almost the same amount of effort to write and I'm not sold that it's something that needs to be implemented... feel free to prove me wrong! Would love to hear more arguments for this.

One more detail question about your proposed changes: I'm wondering what happens behind the scenes when a name for a row is omitted, as shown here with Auto:

<Grid ColumnDefinitions="a:1* b:2* c:Auto d:* e:300"
      RowDefinitions="a:1* Auto i:25 whatever:14 k:20">
</Grid>

You use the index number to refer to it here:
<TextBlock Grid.Row="1" Grid.Column="c" /> <!-- using index number on Row -->

Does this mean that even if a row/column does have a name, you can still use its index to refer to it? I assume that the Name property for a RowDefinition or ColumnDefinition is a string. Would the index get converted into a string and used for the Name property, or would it just be the general rule that rows/columns can be referred to by their index or Name? If so, that leads to more questions about accessing rows and columns in the code-behind...

Maybe I'm thinking too far ahead, but let me know if anyone has thoughts on these points :)

@michael-hawker
Copy link
Collaborator

@anawishnoff I could see the Row/Column attached properties still being defined as integers, it'd just be the XAML TypeConverter that knows how to do the magic with the string to look-up an index. Then there'd be a method like GetIndexFromName(string) on Grid that's used in that function as well as for code-behind use?

@rizamarhaban
Copy link
Author

rizamarhaban commented Mar 21, 2020

@anawishnoff the motivation is:

  1. Easy refactoring (finding row and column). We used to marked it with <!-- ... --> for rows or columns for easy searching. Especially on a large Grid.
  2. Suppose we wanted to insert a row or column on existing grid, we don't have to reassigned each index, however that is what happening currently. With this proposal, we can just add new row or column with variable naming and use the name for Grid.Row/Column on other elements. This will not disturb the other rows/columns. Or Grid.RowColumn for short.
  3. The fallback to indexes is just a lookup as mentioned by @michael-hawker. The important thing is to look at the implementation of the naming on other elements, and not from the definitions only. Because defining rows/columns still uses index behind the scene. Doesn't matter if we reorder the definitions, the index is always sequential. So if we use naming on the definition, then reordering the definitions will also effect the implementation. Currently it is not, unless we specify Height or Width, then we can see the difference. Example;
<Grid>
    <Grid.RowDefinitions>
        <RowDefinition  />
        <RowDefinition Height="Auto" />
        <RowDefinition Height="25" />
        <RowDefinition  />
    </Grid.ColumnDefinitions>
</Grid>

In the above example, if we reorder the definition of index 0 and 3, it doesn't have any changes on the UI. However, index 1 and 2 might change the UI because height different. However, if we can do like this:

<Grid>
    <Grid.RowDefinitions>
        <RowDefinition x:Name="a"  />
        <RowDefinition Height="Auto" />
        <RowDefinition Height="25" />
        <RowDefinition x:Name="b" />
    </Grid.ColumnDefinitions>
</Grid>

and I switch the definitions to be like this:

<Grid>
    <Grid.RowDefinitions>
        <RowDefinition x:Name="b"  />
        <RowDefinition Height="Auto" />
        <RowDefinition Height="25" />
        <RowDefinition x:Name="a" />
    </Grid.ColumnDefinitions>
</Grid>

Then all the UI which uses Grid.Row="a" or Grid.Row="b" will switch as well. Nice right. No need to replace the index number from 3 to 0 and 0 to 3. Unless they did not define naming like on row index 1 and 2. That will need to follow the current existing rule on Grid index. Also, if we need to do like this:

<Grid>
    <Grid.RowDefinitions>
        <RowDefinition x:Name="a"  />
        <RowDefinition Height="Auto" />
        <RowDefinition x:Name="c" Height="Auto"  />
        <RowDefinition Height="25" />
        <RowDefinition x:Name="b" />
    </Grid.ColumnDefinitions>
</Grid>

There will be no changes on Grid.Row 'a' and 'b' elements. However, we need to replace the index number row '3' as '4' to adjust the Grid.Row element. :) That's the motivation. Let's make it simple like this, e.g.:

<!--     Index:            0            1      2 -->
<Grid RowDefinitions="awesome:Auto lovely:Auto *">

<TextBlock Grid.RowColumn="awesome,0" Text="Awesome..." />
<TextBlock Grid.RowColumn="lovely,0" Text="Lovely..." />
<StackPanel Grid.RowColumn="2,0" />
</Grid>

This will show Awesome and Lovely on each row. And someday we can just switch the definition to be like this:

<!--     Index:            0            1      2 -->
<Grid RowDefinitions="lovely:Auto awesome:Auto *">

<TextBlock Grid.RowColumn="awesome,0" Text="Awesome..." />
<TextBlock Grid.RowColumn="lovely,0" Text="Lovely..." />
<StackPanel Grid.RowColumn="2,0" />
</Grid>

The UI changes to show Lovely and Awesome on each row. And there you go.

  1. And possibly more usage in the future.

@anawishnoff
Copy link
Contributor

@michael-hawker @rizamarhaban

I definitely think it would be necessary to have a GetIndexFromName() method here, which makes things a bit easier. Thank you for all of the examples - it definitely clears up how helpful named rows and columns would be in terms of switching around the Grid and interacting with it.

The unnamed row/column scenario is a bit messy, but it would be the developer's prerogative as to whether or not they want to name their rows/columns for more ease.

I'm gonna push just a little bit harder on the Grid.RowColumn attribute 😊: I do get what you're saying that it may make refactoring easier for large grids, although for searching within a grid I feel like searching for individual named rows and columns would be similarly easy. Is that enough of a reason to implement a whole other attribute into Grid which already has such a large API? Do you know of any other Xaml controls which implement similar attributes? Would need some more convincing on this one.

@michael-hawker
Copy link
Collaborator

Yeah, I don't see as much of a need for RowColumn, especially as I'm sure @mrlacey could come up with some amazing helper as part of his extensions for moving items around in a Grid for refactoring... 😋

@rizamarhaban
Copy link
Author

@anawishnoff @michael-hawker yeah, fair enough.

@mrlacey
Copy link
Contributor

mrlacey commented Mar 26, 2020

The RowColumn idea seems like it would be easy to try out with a custom attached property. It feels like the kind of thing that could/should be tried out before a lot of time is spent on incorporating it into something larger (like WinUI.)
After trying it out, it would be good to get feedback on what it's like to use, how beneficial it is, whether new developers (unfamiliar with the syntax) find it easy to understand or confusing because it's too different to what they already know, etc.

@murilocurti
Copy link

murilocurti commented Aug 13, 2020

A good point is that inline syntax will enable to configure the grid using Style Setters:

<Setter Property="ColumnDefinitions" Value="*,*,*" />

@sjb-sjb
Copy link

sjb-sjb commented Dec 17, 2022

Suggesting that the syntax be expanded to include a trailing >=n and/or <=n representing MinHeight and MaxHeight for each row/column specifier. I have done this successfully in my own version of this feature.

Another suggestion, if it is not too much of a distraction, is to allow just A as a short form of Auto. Hence we could have, for example, RowDefinitions="A, A>=24, A, *, A, 2*<=300".

P.S. personally I believe the commas should be required, not optional.

@bpulliam bpulliam added the team-Controls Issue for the Controls team label Aug 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Layouts feature proposal New feature proposal needs-winui-3 Indicates that feature can only be done in WinUI 3.0 or beyond. (needs winui 3) team-Controls Issue for the Controls team
Projects
Development

No branches or pull requests