Added WrapPanel item alignment

[TomEdwardsEnscape]

Adds WrapPanel.ItemAlignment, which allows wrapped items to be left/top, centre, or right/bottom aligned.

The property is an enum type with the values are Start, Center, and End. These names encompass both horizontal and vertical orientations.

What is the current behavior?

Items are always laid out starting from the left/top of a WrapPanel. There is no way to configure this.

Breaking changes

None.

Obsoletions / Deprecations

None.

[avaloniaui-bot]

You can test this PR using the following package version. 11.3.999-cibuild0054001-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

[robloo]

I think we should debate the API here. It doesn't really match with HorizontalAlignment, HorizontalContentAlignment or TextAlignment.

[TomEdwardsEnscape]

This is because the panel can also be oriented vertically too. A unified alignment property like the one in this PR is also used by the CSS flex-box.

[timunie]

Yeah the names are okay to me. However I would like to align the new enums, given the team accept this PR, with Avalonia.Labs VirtualizingWrapPanel, which I have a draft PR open.

In addition to start, end and center, should we add Stretch? Also uniform distribution may be nice. Not sure if the later needs to be part of this PR.

[stevemonaco]

Keep in mind that FlexPanel exists in Avalonia.Labs which has a lot of overlapping functionality. See screenshot:

Some decision probably needs to be made regarding whether WrapPanel is a layout control specifically intended for WPF ports, possibly with spacing added to match WCT's UWP/WinUI3 implementation, or whether it starts to march towards a flex box.

[MrJul]

API diff for review:

 namespace Avalonia.Controls {

+    public enum WrapItemAlignment
+    {
+        Start,
+        Center,
+        End,
+    }

     public class WrapPanel : Panel
     {
+        public static readonly Avalonia.StyledProperty<WrapItemAlignment> ItemAlignmentProperty;
+        public WrapItemAlignment ItemAlignment { get { throw null; } set { } }
     }
 }

[MrJul]

Notes from the API review meeting:

After a debate about whether it should be called alignment or justification, we settled on alignment.
However, the alignment applies to all items in a row/column rather than a single item, so we chose the plural form with WrapPanelItemsAlignment.

Expected API:

public enum WrapPanelItemsAlignment
{
    Start,
    Center,
    End,
}

public class WrapPanel : Panel
{
    public static readonly StyledProperty<WrapPanelItemsAlignment> ItemsAlignmentProperty;
    public WrapPanelItemsAlignment ItemsAlignment { get; set; }
}

[robloo]

@MrJul

Why not something like WrapPanelContentAlignment to avoid item vs items.

Along that train of thought why not just a generalized ContentAlignment enum so we can use this other places?

A big design flaw of UWP is they started making control-specific enums rather than general-purpose ones. It's easier to extend but also isnt as well thought out and integrated as one system.

Side note: is it possible for the community to join these design reviews? I would personally like to comment here and there once in a while.

[robloo]

Another point here. We already have HorizontalContentAlignment and VerticalContentAlignment throughout the framework.

A generalized term unifying these is simply ContentAlignment which could apply for any direction or orientation.

Therefore, I would also argue the property itself should be called ContentAlignment rather than ItemsAlignment. It's a generalization of an existing concept.

[TomEdwardsEnscape]

If everything shares the same enum, then from a usability perspective everything has to support the same values.

This isn't at all practical. HorizontalAlignment.Stretch doesn't make sense for WrapPanel. A theoretical "Uniform" value doesn't make sense for ContentControl. And what if eventually a control arrives which needs "None"?

[robloo]

If everything shares the same enum, then from a usability perspective everything has to support the same values.

Correct. But I'm basing my thinking on HorizontalAlignment, VerticalAlignment and the HorizontalContentAlignment and VerticalContentAlignment properties. The same argument could have been used in the early WPF development cycle. In the end it actually worked out quite well.

HorizontalAlignment.Stretch doesn't make sense for WrapPanel

Good point on the missing Stretch value; however, it does make sense to include it. A value of Stretch will stretch an item to take the full "row" or "column". Each item would have its own line. In effect it would function as a StackPanel with a special feature: It stretches the children to fit (rather than just sizing to children). You've actually pointed out a functionality gap and filling it would be useful.

A theoretical "Uniform" value doesn't make sense for ContentControl. And what if eventually a control arrives which needs "None"?

We have UniformGrid. I'm not sure that applies here and am not arguing for it. We also have NEVER needed a "None" value for the existing HorizontalContentAlignment/VerticalContentAlignment properties. So these arguments seem invalid here.

---

Below is an extremely elegant design that combines the ideas from both HorizontalAlignment and VerticalAlignment. I wouldn't dismiss it easily. It is also general-purpose enough to be used in other controls.

ContentAlignment
{
  Start,
  Center,
  End,
  Stretch,
}

WrapPanel gets it's property, again following existing conventions that have been generalized.

public ContentAlignment ContentAlignment { get; set; }

WrapPanel ABSOLUTELY can support a Stretch value and it's functionality would fit in nicely with these types of controls.

[avaloniaui-bot]

You can test this PR using the following package version. 11.3.999-cibuild0054401-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

[MrJul]

Along that train of thought why not just a generalized ContentAlignment enum so we can use this other places?

I completely agree with @TomEdwardsEnscape here:

If everything shares the same enum, then from a usability perspective everything has to support the same values.

We aren't aiming for a one-size-fits-all enum. As mentioned, Stretch isn't implemented, and while it could be for exhaustiveness, nobody requested it (a UniformGrid with a single column would work just as well). I understand wanting to have a "perfect API", but in this case, I'd say that perfect is the enemy of good.

Suppose tomorrow we want to implement different spacing options for the wrap panel, such as space-between, space-around and space-evenly. We'll need breaking changes, or enum values that won't work elsewhere, because these values are just meaningless for single item contents.

The PR is useful, a single specific enum is perfectly fine, extensible, and doesn't really bloat the public API.

Side note: is it possible for the community to join these design reviews? I would personally like to comment here and there once in a while.

Currently, no. We're just getting started with this review format and we need some time to make sure it's right for the team. We might revisit this decision later.

[MrJul]

Could you please add a couple of unit tests to WrapPanelTests for Center and End?
The implementation looks go to me.

[avaloniaui-bot]

You can test this PR using the following package version. 11.3.999-cibuild0054439-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

[robloo]

@MrJul

We aren't aiming for a one-size-fits-all enum. As mentioned, Stretch isn't implemented, and while it could be for exhaustiveness, nobody requested it (a UniformGrid with a single column would work just as well). I understand wanting to have a "perfect API", but in this case, I'd say that perfect is the enemy of good.

When working with frameworks (rather than apps) I would use a more involved API review for NEW functionality. This is a brand-new concept we don't have elsewhere at the moment so it's important to get right. Future-proofing the design is one of the most important design tenants and it has nothing to do with perfect being the enemy of good. Frameworks -- for everyone's benefit -- shouldn't be left in all cases to evolve organically piece by piece. You end up with a lot of mess to clean up later with breaking changes (if you clean it up at all).

In this case we should ask a key question here: what other controls could benefit from this in the future? No one is asking that question and its answer is pretty important.

In framework-level API design you would be listing ALL related controls that could have this functionality in the future and then design how the API should work for ALL of them. Then you take a step back and implement the subset for this one control as required. You DO NOT want to design stuff like this thinking about only one control to start, it's bad practice I'm trying to guide you away from. This requires framework-level thinking.

Again, this is new functionality that is potentially generally useful.

Suppose tomorrow we want to implement different spacing options for the wrap panel, such as space-between, space-around and space-evenly. We'll need breaking changes, or enum values that won't work elsewhere, because these values are just meaningless for single item contents.

Yes, so that should be designed now while we are talking about it.

WrapPanel in the community toolkit has VerticalSpacing/HorizontalSpacing. I would argue that the FlexPanel/CSS concept of "space-between, space-around and space-evenly" don't fit right now with XAML frameworks. They are designed to work differently with explicit spacing defined in cases like this. VerticalSpacing/HorizontalSpacing+Padding can already handle space-between and space-around. However, space-evenly needs something like a UniformWrapPanel like we have UniformGrid.

All this stuff is related and there should be a real specification for it. If we want to go the CSS route long term it needs to be discovered how to fit that in with what we already have. It certainly isn't clear right now.

The PR is useful, a single specific enum is perfectly fine, extensible, and doesn't really bloat the public API.

It's adequate but I hope in time you see my point.

[stevemonaco]

In this case we should ask a key question here: what other controls could benefit from this in the future? No one is asking that question and its answer is pretty important.

This is why I alluded to FlexPanel. Although the API in this PR is a solid, incremental improvement (one that I was considering PR'g if not for the following text), there should be some overarching vision of what layout controls can or should look like in 2-3 years. Depending on how many breaking changes are acceptable for v12. Changing ArrangeCore for v12 to be more WPF-like will likely have large effects, including on WrapPanel.

[MrJul]

When working with frameworks (rather than apps) I would use a more involved API review for NEW functionality. This is a brand-new concept we don't have elsewhere at the moment so it's important to get right. Future-proofing the design is one of the most important design tenants and it has nothing to do with perfect being the enemy of good. Frameworks -- for everyone's benefit -- shouldn't be left in all cases to evolve organically piece by piece. You end up with a lot of mess to clean up later with breaking changes (if you clean it up at all).

Except we are not trying to design the future of all layoutable controls here. We are not introducing a new important base framework concept, impacting everyone. We are adding a specific feature to a specific control. A single property. We are addressing a simple need with a simple solution.

For what it's worth, know that we did discuss merging the enum with other potential ones, or simply rejecting the PR in favor of the more feature complete FlexPanel; it isn't like we just reviewed the API in a vacuum. We ultimately decided against it, because the improvement provided by this PR is real and solves user needs, with a very low impact on the public API.

That being said, I'm aware that some parts have evolved organically in the framework and should probably be kept in check a little more. There are various minor inconsistencies in different APIs that I'd ideally like to see fixed in the future. You will disagree, but this PR isn't one.

I would argue that the FlexPanel/CSS concept of "space-between, space-around and space-evenly" don't fit right now with XAML frameworks.

And I would agree. These were examples, I'm not saying that we want to implement these exact features on WrapPanel (FlexPanel is a more appropriate replacement). However, if we ever do, an extensible enum is fine.

The balance between generic (in this case, a single enum for all content alignment concepts) and specific (one enum per control) is always a difficult one. For this PR, we decided that a specific enum is sufficient.

[MrJul]

LGTM, thank you!