Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Grid improvements #166

Closed
stevenbrix opened this issue Dec 13, 2018 · 18 comments
Closed

Grid improvements #166

stevenbrix opened this issue Dec 13, 2018 · 18 comments
Labels
API suggestion Early API idea and discussion, it is NOT ready for implementation
Milestone

Comments

@stevenbrix
Copy link
Contributor

Specifying Grid/ColumnDefinitions in markup is tedious. Avalonia came up with a really nice syntax that lets you specify them like this:

<Grid ColumnDefintions = "Auto,*,*" RowDefinitions="Auto,*,*" >
   ...
</Grid>

Instead of the old syntax:

<Grid>
   <Grid.ColumnDefinitions>
     <ColumnDefintion Width="Auto"/>
     <ColumnDefintion Width="*"/>
     <ColumnDefintion Width="*"/>
   </Grid.ColumnDefinitions>
   <Grid.RowDefinitions>
     <RowDefinition Height="Auto"/>
     <RowDefinition Height="*"/>
     <RowDefinition Height="*"/>
   </Grid.RowDefinitions>
…
</Grid>
@stevenbrix stevenbrix added the Enhancement Requested Product code improvement that does NOT require public API changes/additions label Dec 13, 2018
@stevenbrix stevenbrix added this to the Future milestone Dec 13, 2018
@Rand-Random
Copy link

Nice idea, just wanted to throw in that your issue title is very very generic.
Maybe consider rename it to actually contain the improvement your issue talks about.
Sorry, for being picky.

@dotMorten
Copy link
Contributor

I agree, and I've seen this suggested several times before. It's really nothing more than adding a simple type converter here. I'd be more than happy to volunteer for providing this - however I assume there needs to be some sort of spec/language/API discussion first?

A few immediate issues I see though that will complicate "it's just a typeconverter" argument:

  • Grid.RowDefinitions/ColumnDefinitions are read-only.
  • RowDefinitionCollection/ColumnDefinitionCollection are not creatable publicly, and internally they are only creatable with a Grid parent parsed into the constructor.

@rladuca rladuca added API suggestion Early API idea and discussion, it is NOT ready for implementation and removed Enhancement Requested Product code improvement that does NOT require public API changes/additions labels Dec 13, 2018
@dotMorten
Copy link
Contributor

Poking around in the reference source, looks like there's a lot more to it: There doesn't seem to be anything anywhere that can convert a string to a set of adds on an existing read-only collection. Rather than making the collection creatable and settable, which seems to introduce a lot of other issues, to solve this right, we might have to introduce an entirely new CollectionTypeConverter concept of sorts first (this would actually be useful for a lot of other scenarios too)

@Symbai
Copy link
Contributor

Symbai commented Dec 13, 2018

I really like this idea, but would also heavily appreciate if this can be adopted to children as well in any kind. The less repetitive code the better.

For example instead of writing lots of repetitive code like this:

<Button Grid.Row="0" Grid.Column="0"/>
<Button Grid.Row="0" Grid.Column="1"/>
<Button Grid.Row="0" Grid.Column="2"/>
<Button Grid.Row="1" Grid.Column="0"/>
<Button Grid.Row="1" Grid.Column="1"/>
<Button Grid.Row="1" Grid.Column="2"/>

Maybe something like this:

<Button Grid.Placement="0"/>
<Button Grid.Placement="0,1"/>
<Button Grid.Placement="0,2"/>
<Button Grid.Placement="1"/>
<Button Grid.Placement="1,1"/>
<Button Grid.Placement="1,2"/>

Not sure if this goes too far but I always had the problem that if you want to add a new row or column between existing, you always have to go through ALL children and fix the row / column number. I always asked myself why don't it just take these information from the order in XAML? It would be really great when we can do something like a stackpanel, obviously the syntax is free to change and just for demonstration the idea:

<Grid ColumnDefintions = "Auto,*,*" >
 <Grid.RowDefinitions ChildrenPlacement="Auto">
  <Button Grid.Column="0"/> -- placed in row 0
  <Button Grid.Column="1"/> -- placed in row 1
  <Button Grid.Column="2"/> -- placed in row 2
 </Grid.RowDefinitions>

@thomasclaudiushuber
Copy link
Contributor

thomasclaudiushuber commented Dec 13, 2018

@stevenbrix Why even two properties for Rows and Columns? :) I've just played around with a litte project and I came up with an attached property like this:

<Grid local:GridExtensions.Structure="*,100|200,*">

</Grid>

It creates actually this behind the scenes:

<Grid>
  <Grid.RowDefinitions>
    <RowDefinition Height="*"/>
    <RowDefinition Height="100"/>
  </Grid.RowDefinitions>
  <Grid.ColumnDefinitions>
    <ColumnDefinition Width="200"/>
    <ColumnDefinition Width="*"/>
  </Grid.ColumnDefinitions>
  
</Grid>

Here's the repo (Note: It's not production code, I've just hacked it down in a few minutes):
thomasclaudiushuber/Wpf-Grid-Extensions

@dotMorten Adding a brand new property for the shorthand syntax that sets the other properties (RowDef & ColDef) behind the scenes could work as well. I also noticed that the RowDefinitionCollection constructor is not public.

@Symbai Creating an additional Placement-Property to set Row and Column on a Child with 1 string wouldn't be a big deal.

@Symbai Putting elements in rows and columns in the order they appear in XAML doesn't work, or it works, but you won't get what you want. Actually the UniformGrid does this. But to build a typical Window layout there are too many scenarios where you need control, and just the order is not enough to
a) put multiple elements in the same row or column
b) find out whether the next element goes into a column or into a row

@Symbai The last thing you mentioned with "fixing the row number" on an element could be solved with naming. Instead of using numbers, you name Rows and then an element is actually glued to that row.

Note that the Grid is discussed in parallel for UWP here: microsoft/microsoft-ui-xaml#54

@dotMorten
Copy link
Contributor

Adding a brand new property for the shorthand syntax that sets the other properties (RowDef & ColDef) behind the scenes could work as well.

I don't think that's a good story. That gives you two 1st class properties that do the same thing. And as you just demonstrated, you can just use a custom attached property instead then.

@thomasclaudiushuber
Copy link
Contributor

@dotMorten Yes, true, two 1st class properties is not the cleanest way. That would mean we should go the TypeConverter way to get the shorthand syntax directly on the properties RowDefinitions and ColumnDefinitions like shown in the initial comment from @stevenbrix. This means we need to find out how to solve this with the readonly collections like you mentioned already.

@michael-hawker
Copy link

michael-hawker commented Dec 14, 2018

Would be nice to coordinate this with the proposed improvements for the UWP Grid for consistency across WPF/UWP: microsoft/microsoft-ui-xaml#54

(just noticed someone else linked to above, sorry for the duplicate, but it'd be a shame to implement two different patterns!)

@thomasclaudiushuber
Copy link
Contributor

thomasclaudiushuber commented Dec 14, 2018

@michael-hawker Jep, I added already a reference above to the UWP Grid issue, so that you can see it there.

Now we reached already a point where we notice that there should be something like a XAML Standard for UWP and WPF. Maybe the original XAML Standard Idea between UWP and Xamarin Forms was too much, as these platforms are too far away from each other. But having a standard between UWP and WPF could make a lot of sense.

@Rand-Random
Copy link

@thomasclaudiushuber
Seems like you picked the wrong person to correspond to, since I never said those things you mention me in your post. #166 (comment)

@thomasclaudiushuber
Copy link
Contributor

@Rand-Random True, sorry. I've adjusted the comment. Thanks.

@michael-hawker
Copy link

@thomasclaudiushuber wonder what @harinikmsft thinks?

Now we reached already a point where we notice that there should be something like a XAML Standard for UWP and WPF. Maybe the original XAML Standard Idea between UWP and Xamarin Forms was too much, as these platforms are too far away from each other. But having a standard between UWP and WPF could make a lot of sense.

Would it make sense to use the xaml-standard repro for these general XAML discussions?

@trodent83
Copy link

trodent83 commented Dec 17, 2018

@stevenbrix

I think if one would define a Default display mode and then be able to redefine it later. So your orriginal example would look like something like this:

<Grid Rows="3" Columns="3" RowHeight="*" ColumnWidht="*"> <Grid.RowDefinitions> <RowDefinition Row ="0" Height="Auto"/> </Grid.RowDefinitions> <Grid.ColumnDefinitions> <RowDefinition Column ="0" Height="Auto"/> </Grid.ColumnDefinitions> </Grid>

This way you could also add the visibility property that we discussed in the other thread.

@stevenbrix
Copy link
Contributor Author

Thanks for all the great discussion @michael-hawker, @thomasclaudiushuber, @trodent83, @dotMorten, and @Rand-Random! Sorry for the crickets from me on this thread. I opened this due to some discussions we had internally with people on the WINUI team, so we'll definitely be coordinating efforts and bringing this work to both teams. I'm going to close this out to the others since there is more recent and interesting discussion on those!

@jnm2
Copy link

jnm2 commented Oct 20, 2021

I'm going to close this out to the others since there is more recent and interesting discussion on those!

Is this discussion microsoft/microsoft-ui-xaml#54 (currently in a GitHub project column called "Freezer") or somewhere else that we can follow?

If the discussion has stalled elsewhere, can the WPF team consider reopening this issue?

@michael-hawker
Copy link

@jnm2 this simplified syntax was implemented as part of microsoft/microsoft-ui-xaml#673 in WinUI 3 already. I agree it'd make sense to for this issue to be re-opened for the WPF team to back-port the WinUI 3 updates to WPF for compatibility. FYI @predavid @ryalanms?

@thomasclaudiushuber
Copy link
Contributor

thomasclaudiushuber commented Oct 26, 2021

I agree @michael-hawker. backporting would be great. In WPF, there are a few things to consider that @rrelyea pointed out in another PR:
a) Think about compatibility between netcoreapp WPF and netfx WPF, is there a strategy?
b) Can the designer in Visual Studio support the new syntax out-of-the-box, or are modifications needed?

For me, the answer to a) is the following: As long as netcoreapp WPF is a superset of netfx WPF, everything is fine, because people usually migrate from netfx=>netcoreapp, but not from netcoreapp=>netfx. That means we could introduce the new syntax in netcoreapp WPF, but not in netfx WPF. Of course we could also introduce it in both.

The answer to b) is one that I don't know. Someone at Microsoft with access to the internal sources of the WPF designer in Visual Studio has to look at this to make a statement here.

@lindexi
Copy link
Member

lindexi commented Oct 29, 2021

@vishalmsft Should move this to Discussions? Thank you

@dotnet dotnet locked and limited conversation to collaborators Nov 1, 2021

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
API suggestion Early API idea and discussion, it is NOT ready for implementation
Projects
None yet
Development

No branches or pull requests

10 participants