Microsoft-ui-xaml: Proposal: New Grid ColumnDefinition and RowDefinition Constructors

Created on 25 Nov 2019  路  25Comments  路  Source: microsoft/microsoft-ui-xaml

Proposal: New ColumnDefinition and RowDefinition Constructors

Summary

@robloo:

I've often wondered why the constructors for ColumnDefinition and RowDefinition don't have an
overloaded constructor that allows passing a GridLength directly. Adding this would potentially simplify a lot of code. I know this doesn't matter in pure MVVM, but when you have to do code-behind it would be quite helpful.

@anawishnoff:

With the recent changes to the Grid definition syntax in XAML, it seemed only natural to continue this progression towards simpler Grid definitions in code-behind. Currently, the ColumnDefinition and RowDefinition constructors takes no arguments, and in order to create a Grid with one column it takes four lines of code (as can be seen in the example below).

These proposed API changes would allow the ColumnDefinition and RowDefinition constructors to accept two arguments: an integer Width (or Height), and a GridUnitType object (i.e. Auto or Star *). These two parameters will create a GridLength object, which will be assigned to the Width or Height property.

Goal

@anawishnoff:

By defining the height/width and GridUnitType inside of the ColumnDefinition or RowDefinition constructor, you could cut the amount of required lines of code to define a Column or Row in half. This syntax change will make defining rows and columns in the code-behind easier and more consistent with the new way of defining them in XAML.

Example

@robloo:

Before:

var grid = new Grid();
var column = new ColumnDefinition();
column.Width = new GridLength(1.0, GridUnitType.Star);
grid.ColumnDefinitions.Add(column);

After:

(added from @chingucoding comments below)

var grid = new Grid();
grid.ColumnDefinitions.Add(new ColumnDefinition(1.0, GridUnitType.Star)); // Width="1*"
grid.ColumnDefinitions.Add(new ColumnDefinition(500.0)); // Width="500px"


This would be especially helpful when creating multiple rows/columns.

Constructor API

// Constructors with parameters from the GridLength as well
public ColumnDefinition(double pixelWidth) { ... }
public ColumnDefinition(double width, GridUnitType type) { ... }
public RowDefinition(double pixelHeight) { ... }
public RowDefinition(double height, GridUnitType type) { ... }

Rationale

@robloo:

Code-behind simplification. Shorter is better!

@anawishnoff:

  • This new shorter syntax will provide an easier experience in defining one of the most common controls.

  • This change follows a natural progression from the new XAML language feature that will shorten the Grid syntax - defining a Grid in the code-behind and defining a Grid in XAML will be consistent and short experiences.

Scope

| Capability | Priority |
| :---------- | :------- |
| Add overloaded constructor for ColumnDefinition with GridLength width | Must |
| Add overloaded constructor for RowDefinition with GridLength height | Must |

feature proposal

Most helpful comment

Pitched this to our team here, and we decided that the second syntax that involves giving RowDefinition() and ColumnDefinition() the arguments of GridLength works best. Updated the proposal above to reflect the changes! Thanks everyone for your contributions - this will be moved to the spec-level soon :)

All 25 comments

You can already do the following, which is almost like your proposal:
```c#
var column = new ColumnDefinition
{
Width = new GridLength(1.0, GridUnitType.Star)
};

Instead to make it easier, maybe the following would be even more convenient:

```c#
// Column with Width="1*"
var column = new ColumnDefinition(1.0, GridUnitType.Star);
// or
// Column with Width="500"
var column = new ColumnDefinition(500.0);

However this may be a bit unclean dependency wise, so I understand if this might not be the right way.

Yes, object initializers are always nice! I don't believe they exist in the C++ world for example though.

And it's a good idea to take the arguments from GridLength and also expose them in the Column/RowDefinition directly!

I think this is covered by #673? If it's not, maybe it should be? @anawishnoff?

Yes, object initializers are always nice! I don't believe they exist in the C++ world for example though.

Oh yes forgot about that. The creation of columns and rows should not be so "complicated" regardless of the language used!

I think this is covered by #673? If it's not, maybe it should be? @anawishnoff?

I think #673 only covers the XAML part. Maybe it is better to keep code behind and the XAML separated when discussing "API" changes?

@jevansaks, @chingucoding Yes, #673 only applies to XAML (which is more important BTW!).

As stated above, yes - #673 just deals with the XAML side of things. However, I'm always interested in an issue that leads us to shorter code and improved developer efficiency! This seems like a good match to #673 as it extends its motive into the code-behind.

In response to this point:

Yes, object initializers are always nice! I don't believe they exist in the C++ world for example though.

We want to make sure we're keeping all of our Windows devs in the loop regardless of the language/platform they're using. With that said, would it be more ideal to use the original shortened version suggested, i.e.:

grid.ColumnDefinitions.Add(new ColumnDefinition(new GridLength(1.0, GridUnitType.Star));

so that the ColumnDefinition constructor has a more structured/straightforward change?

With that said, would it be more ideal to use the original shortened version suggested, i.e.:

grid.ColumnDefinitions.Add(new ColumnDefinition(new GridLength(1.0, GridUnitType.Star));

so that the ColumnDefinition constructor has a more structured/straightforward change?

I'm fine if that's the only change that makes it in. I think we all understand it breaks a few concepts having the constructor parameters from GridLength within a Column/RowDefinition directly. One might normally not expect that from a class design standpoint. That said, it's so obvious that a Column must have a width and a Row must have a height that there could be some debate like we are having.

So it's up to you I think, if you want to keep the classes and their constructor parameters more cleanly separated I totally understand.

Hey @robloo, I'd like to pitch this proposal to the WinUI team here, but was hoping to make some edits. Wanted to check with you first if it would be alright to edit your proposal directly, or if you preferred that I start a new proposal (Github issue) and referenced yours.

Thanks! Looking forward to working on this.

Hi @anawishnoff, I would be happy to update the proposal as you need -- or if you can edit on your end, go for it!

It just might be a good idea to add a comment for the parts of the proposal removed to keep a record of it. For example, I'm guessing you would like "Add overloaded constructors for Column/RowDefintion using GridLength parameters" removed entirely.

Thanks for following up with this!

Just updated the proposal above, and tried to give credit where credit was due and keep your original language. 馃槉

Feel free to tell me if anything looks funky, and continue the conversation! This is still a WIP for the time being.

@anawishnoff, awesome, thanks for cleaning this up!

Just a couple of comments/questions:

but it will also expose ColumnDefinition and RowDefinition's properties in a way that makes the definitions more cohesive and easier to understand.

I'm not sure I understand this. It's only the constructor parameters that are changing. How would the properties be simplified? Maybe this is just a type-o?

With an object initializer that leaves out the explicit GridLength definition, basically passing GridLength's arugments into the Column/RowDefinition itself:

I wouldn't say object initializer here -- that's not actually what @chingucoding was stating here. Instead he was saying we should just add the GridLength constructor parameters directly to the ColumnDefinition/RowDefinition constructors. It can easily be inferred that those represent width or height respectively. Then the ColumnDefinition/RowDefinition would build it's own GridLength internally and use that for width/height. This would eliminate the need for the app developer to create a GridLength object in this case.

@robloo:

but it will also expose ColumnDefinition and RowDefinition's properties in a way that makes the definitions more cohesive and easier to understand.

I'm not sure I understand this. It's only the constructor parameters that are changing. How would the properties be simplified? Maybe this is just a type-o?

Fixed this - just deleted and replaced that sentence, it didn't make too much sense/impact reading over it again.

With an object initializer that leaves out the explicit GridLength definition, basically passing GridLength's arugments into the Column/RowDefinition itself:

I wouldn't say object initializer here -- that's not actually what @chingucoding was stating here. Instead he was saying we should just add the GridLength constructor parameters directly to the ColumnDefinition/RowDefinition constructors. It can easily be inferred that those represent width or height respectively. Then the ColumnDefinition/RowDefinition would build it's own GridLength internally and use that for width/height. This would eliminate the need for the app developer to create a GridLength object in this case.

Fixed! I think my words just got a little jumbled, tried to clarify.

Pitched this to our team here, and we decided that the second syntax that involves giving RowDefinition() and ColumnDefinition() the arguments of GridLength works best. Updated the proposal above to reflect the changes! Thanks everyone for your contributions - this will be moved to the spec-level soon :)

@anawishnoff that's good news! Thanks again for taking this forward.

In terms of API completeness and class design, it does make sense to have another set of constructors that take GridLength directly as well. It is very simple to code that. It would just be my suggestion to keep that as part of the changes. If it gets dropped I'm not going to worry about it though. I'm sure there were reasons discussed internally.

@robloo Yes, having a separate set of constructors that take a GridLength object argument was definitely discussed, but I believe the team came to the conclusion that it wouldn't be possible to have all of the options (a constructor that accepts just a width OR just a GridLength object OR width and GridUnitType object).

@MikeHillberg or @chrisglein - could you refresh my memory as to why it was not possible to overload the constructor with both the GridLength argument and the set of arguments that we ended up choosing?

Xaml's APIs are exported with WinRT, which has a restriction that you not overload methods only on type, there has to be a difference in parameter count. (So only arity overloading is allowed.) That's because WinRT projects unto untyped languages. So if here we have one constructor that takes a double (for px) and one that takes a GridLength, there's ambiguity at the call site.

@MikeHillberg Ouch, that's a pretty big sacrifice in the WinRT design that I definitely don't support. Will the curse of JavaScript never end? :) Anyway, I understand the explanation, thanks for clarifying!

Update: the spec for this change has been written! Feel free to look through it and leave any feedback.

In terms of a timeline - these changes, along with the new XAML language feature that changes the Grid definition syntax, will hopefully be implemented later this year with WinUI 3. See our roadmap for more details!

Good News! Thanks for the update.

I had a chance to go through the spec:

Definition of a ColumnDefinition's Width property or a RowDefinition's Height property must be done inside the creation of a Row/ColumnDefinition

Since you are talking about how things are done currently this should probably say "Definition of a ColumnDefinition's Width property or a RowDefinition's Height property must be done inside the creation of a GridLength..."

New syntax to define a Grid with one simple (no GridUnitType value) column in code-behind:

It is not "No GridUnitType" it's just the GridUnitType is implied to be pixel. That's why it's a good ideas to name the arguments 'pixelWidth' and 'pixelHeight' in the single argument constructors as you kept in the API Notes.

Finally, your API argument naming is inconsistent. What you copy-pasted from this discussion in API Notes doesn't match with API Details. For example RowDefinition(double height, GridUnitType type) and RowDefinition(double pixelHeight, GridUnitType type) are different and the API Details is actually in error as the two argument constructor cannot assume the height is pixels.

I believe the best API would be as below

RowDefinition(double height, GridUnitType type);
RowDefinition(double pixelHeight);

ColumnDefinition(double width, GridUnitType type);
ColumnDefinition(double pixelWidth);

This is heavily based on the GridLength API excepts adds height/width to the names because it is now known if the constructor is for a row or column.

public GridLength (double pixels);
public GridLength (double value, GridUnitType type);

@robloo Thanks so much for the feedback and catching these things. The API notes/details disparity was definitely a typo and that's been fixed. I've clarified the spec to cover your other points as well. :)

To simplify ColumnDefinition (api reference) (and RowDefinition which is the same):

  1. It should only contain the definition, so with ActualWidth removed because it is not part of the definition.
  2. It should be immutable (with the setters on MaxWidth, MinWidth, and Width removed, and MaxWidthProperty, MinWidthProperty and WidthProperty removed).

It should only contain the definition, so with ActualWidth removed because it is not part of the definition.

Well it is part in the sense that a Column-/Rowdefinition is something that once in a Grid will have an actual width/height. These objects (Column/Rowdefinition) state that there should be a Column with certain properties. Without an actual widht/height property it will get very hard to get information about the Grids structure.

It should be immutable (with the setters on MaxWidth, MinWidth, and Width removed, and MaxWidthProperty, MinWidthProperty and WidthProperty removed).

Why should it be immutable? What if some of those properties need to change, e.g. for responsive layouts? Should we throw the Column-/Rowdefinition and create a new object every time we need to make changes here?

@charlesroddie In addition to what @chingucoding stated, I have used ActualWidth several times in production apps. ActualWidth is not only very useful with Grid column/row (or any other UI control/element) due to the way the layout/measure is done, it is mandatory. In some cases you may have NaN as the set width before it's calculated or when set to Auto. However, you always need to know the real/actual width of the control for other control's layout/measure

It is also very useful to have ColumnDefinitions and RowDefinitions be mutable. It's much easier to change an existing column width than creating a new one and swapping it in the Grid. Again, I've used this in production apps.

Honestly, I wouldn't adopt any of the suggested changes and there is no reason to break an existing API like this.

Without an actual widht/height property it will get very hard to get information about the Grids structure.

I think there should be width/height properties. Just doesn't make sense to put them in a "definition".

Why should it be immutable? What if some of those properties need to change, e.g. for responsive layouts? Should we throw the Column-/Rowdefinition and create a new object every time we need to make changes here?

Yes that would simplify quite a lot of logic.

I think there should be width/height properties. Just doesn't make sense to put them in a "definition".

The next question then would be where exactly one should get this information from? There is no such thing as "Current Columns/Rows" property. The Colunm/Rowdefinition is, as far as I know, the only way we have access to that information.

Yes that would simplify quite a lot of logic.

So having to deal with the list of columns/rows is easier than just changing a property of one entry?

Was this page helpful?
0 / 5 - 0 ratings