[WrapPanel] Add Spacing Properties

[Poker-sang]

What does the pull request do?

Add ItemSpacing and LineSpacing Properties for WrapPanel

What is the current behavior?

WrapPanel has no spacing properties

What is the updated/expected behavior with this PR?

How was the solution implemented (if it's not obvious)?

Checklist

• Added unit tests (if possible)?
• Added XML documentation to any related classes?
• Consider submitting a PR to https://github.com/AvaloniaUI/avalonia-docs with user documentation

Breaking changes

Obsoletions / Deprecations

Fixed issues

[avaloniaui-bot]

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

[robloo]

In the UWP/WinUI community toolkit naming if these properties was discussed and decided as HorizontalSpacing and VerticalSpacing. I think we should follow that terminology here.

https://learn.microsoft.com/en-us/windows/communitytoolkit/controls/wrappanel#properties

Horizontal/Vertical terminology is more general-purpose (can apply to other controls) and fits well with existing properties for alignment.

[Poker-sang]

@robloo I was going to use that name, but after careful consideration I realized:

1. for unaligned controls, people care more about ItemSpacing and LineSpacing than H/V Spacing. If you set a different H/V Spacing and change the orientation, the new layout can be confusing.

2. In WindowsAppSDK 1.5 and later, has LinedFlowLayout which is a very similar layout to WrapLayout, and its properties' names, MinItemSpacing and LineSpacing. I think these names make more sense.

So after weighing them, I choose to use ItemSpacing and LineSpacing instead of H/V Spacing in Toolkit.

[robloo]

@Poker-sang You've thought this through more than I thought and more than what was decided in the toolkit. I think the logic for your point 1 above is the strongest argument.

With controls that have an orientation property -- itself with horizontal/vertical -- that is a complication. It would be nice to be able to set the Spacing and just have it work and make sense for all orientations.

Let me think about this a bit more. I suspect you're right though.

[timunie]

@robloo I was also like you at the beginning but am mostly sold on it for this control. Take StackPanel, it has only Spacing and will lead to the same results regardless of the orientation. That should be the same here imo. But the team also needs to agree on it

[robloo]

I'm basically sold that we shouldn't be using H/V Spacing terminology for the above mentioned reasons.

Additional thoughts I've had on naming these properties are:

• ItemSpacing
  • At first look this terminology seems fine
  • But then perhaps "Item" is redundant. The standard spacing property of course will control space between children.
  • Perhaps just the term Spacing is enough. It is pretty intuitive what a Spacing property would be for and the line/row spacing property is clearly differentiated by another name.
  • Spacing would align the property with StackPanel. Both properties actually do the same thing.
  • ItemSpacing does however align well with existing Item* properties on the control.
• LineSpacing
  • The first thought when I see LineSpacing is for spacing between multi-line text. I suspect I wouldn't be the only one thinking that and there could be confusion caused by this.
  • We also have RowSpacing for UniformGrid/Grid now. Both properties actually do the same thing.
  • The WrapPanel has no usage of the term "Line" within the control. Also for that matter it has no usage of "Row". Therefore, these terms seem arbitrary.
  • I think we should consider using the term RowSpacing here just like Grid and UniformGrid rather than LineSpacing.
  • Yes, this would deviate from the WinUI example of the LinedFlowLayout but it also aligns better with existing controls.

So bottom line, we could align with existing controls/properties more if we changed the wording:

1. ItemSpacing -> Spacing
2. LineSpacing -> RowSpacing (edit below)

Edit: On second thought, "Row" does have an orientation implied. "Line" might make more sense for both orientations as a line could either be horizontal or vertical.

[MrJul]

Public API for review:

namespace Avalonia.Controls
{
    public class WrapPanel : Panel
    {
+        public static readonly StyledProperty<double> ItemSpacingProperty;
+        public static readonly StyledProperty<double> LineSpacingProperty;
+        public double ItemSpacing { get; set; }
+        public double LineSpacing { get; set; }
    }
}

[MrJul]

During the API review session, we were hesitating between ItemSpacing + LineSpacing (original proposal) versus Spacing + RowSpacing (as suggested by @robloo).

At the end of the day, ItemSpacing + LineSpacing won.
The proposed API is accepted as-is.

[robloo]

@MrJul Yea, it kind-of fell in the middle and terminology wasn't exactly clear. I trust the debate you had.

Out of Curiosity did you discuss Spacing and LineSpacing? A hybrid of the two viewpoints. At the end of the day that might have been the sweet spot all things considered.

[avaloniaui-bot]

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

[Poker-sang]

I cant run the project now after it updated to main🥺

[MrJul]

I cant run the project now after it updated to main🥺

Probably the submodules move from #18431. Sorry, it was a necessary pain.
First update the submodules:
git submodule update --init --recursive

If git complains about not being able to remove some directories, or if you've got duplicated symbol errors when building, clean your working directory and all submodules:
git clean -ffxd

[avaloniaui-bot]

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

[avaloniaui-bot]

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

[avaloniaui-bot]

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

[avaloniaui-bot]

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

[MrJul]

LGTM, thank you!