[DockPanel] Add Spacing properties

[Poker-sang]

What does the pull request do?

Add HorizontalSpacing, VerticalSpacing properties for DockPanel

What is the current behavior?

DockPanel only has LastChildFill property

What is the updated/expected behavior with this PR?

Add HorizontalSpacing, VerticalSpacing Property

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

None

Obsoletions / Deprecations

None

Fixed issues

[avaloniaui-bot]

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

[cla-avalonia]

• All contributors have signed the CLA.

[Poker-sang]

@cla-avalonia agree

[rabbitism]

by convention panel should not have padding.

[Poker-sang]

Thanks for quick response. But I saw a Grid in WinUI3 has padding

anyway I can remove that property if needed

[MrJul]

Diff for API review:

 namespace Avalonia.Controls
 {
     public class DockPanel
     {
+        public static readonly StyledProperty<Thickness> PaddingProperty;
+        public static readonly StyledProperty<double> HorizontalSpacingProperty;
+        public static readonly StyledProperty<double> VerticalSpacingProperty;
+        public Thickness Padding { get; set; }
+        public double HorizontalSpacing { get; set; }
+        public double VerticalSpacing { get; set; }
     }
 }

[MrJul]

Notes from the API review meeting:

Remove Padding. Other Panel implementations don't have this property (and don't need to - they also don't have a concept of border, radius, etc.).

Regarding HorizontalSpacing and VerticalSpacing, we couldn't really see the use cases here.

Most panels are displaying a series of items, and spacing makes perfect sense there: the user doesn't need to add a margin to each item, with special handling of the first or last ones.

DockPanel is different, is that there's usually only one "fill" child, and a fixed number of children with specific dock positions. As such, we don't see what Spacing would bring to the table that isn't easily doable with Margin on children.

Could you please provide use cases?

[Poker-sang]

Thanks for reviewing.

For any Panel, the Margin of the child items can be used to achieve a similar effect to Spacing, so for any Panel, Spacing is not required, but a more convenient option.

To answer your question. This is an example that use DockPanel and need Spacing:

Also, generally DockPanel is used to simplify the writing of parts of the Grid. And many people have a need for Spacing on the Grid, so of course they should have a need for Spacing on the DockPanel.

[Poker-sang]

Padding property is removed

[avaloniaui-bot]

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

[robloo]

@MrJul

Remove Padding. Other Panel implementations don't have this property (and don't need to - they also don't have a concept of border, radius, etc.).

Padding is actually required to fully support all spacing cases. Existing Spacing properties only control what is between children. If you want to have spacing at the start or the end that's why Padding is there. Microsoft realized it's the same concept as the existing Padding property used for layout so didn't add anything new. But Padding+Spacing very much does go together and is there to support all cases.

I also agree with the above use-case justification. It seems obvious how this is useful in other panels (StackPanel, Grid, etc.) and will be useful here. By this same logic you are using I can say get rid of DockPanel and lets just use a Grid for everything. Of course it's possible, it just isn't as fast or convenient. This is a pretty common use-case so was pulled out as it's own control.

This also aligns pretty closely with WrapPanel in the community toolkit: https://learn.microsoft.com/en-us/windows/communitytoolkit/controls/wrappanel#properties

[MrJul]

Padding is actually required to fully support all spacing cases. Existing Spacing properties only control what is between children. If you want to have spacing at the start or the end that's why Padding is there.

The point is that this isn't specific to DockPanel but to all panels. If we chose to add Padding, it should probably be on Panel directly. Let's keep this PR focused on spacing.

To answer your question. This is an example that use DockPanel and need Spacing:

Thank you. I guess none of us in the team really use DockPanel that way - hence the ask for use cases. (I would definitely model that image as a Grid.) The use case is perfectly valid. We've been adding spacing on most panels, DockPanel shouldn't be an exception.

[Poker-sang]

Thank you @robloo for your long reply, you expressed exactly what I had in mind about Padding. ❤️

And yes @MrJul, I also think it's better to add the Padding property directly on the Panel, it should be shared by all Panels.

[Poker-sang]

But the specific implementation of Padding lies in each individual Panel. i think if i remove the Padding from the DockPanel, it will take more effort to add it back next time. Whereas if I keep it, we can just extract the Padding to the base class in the future with no breaking changes.

It's just my opinion, but all in all I'll follow the advice of the Avalonia team.

[MrJul]

i think if i remove the Padding from the DockPanel, it will take more effort to add it back next time

Padding is trivial to implement: it's deflating the available bounds before doing anything else. We could even have it in the base Panel class if we choose to add it. But there's absolutely no rush: it's so easy for the user to have padding by wrapping the panel inside a Border. If the panel doesn't have any Background, simply setting the Margin also works as a substitute.

Please, let's keep this PR focused on DockPanel spacing. If you wish, feel free to open an issue to discuss adding Padding to panels.

[robloo]

Please, let's keep this PR focused on DockPanel spacing.

My point is Padding is related to Spacing in this context. Padding+Spacing go together. We ARE focusing on DockPanel spacing.

Since not all panels support Spacing there is no reason to pull this out into the base class yet. Although, I agree it could go there in the future.

You are also correct that it's simple to work around this. However, you missed some things in the API review so it's worth revisiting. I'm pretty sure everywhere in WinUI you see Spacing properties you will also find Padding.

[maxkatz6]

However, you missed some things in the API review so it's worth revisiting. I'm pretty sure everywhere in WinUI you see Spacing properties you will also find Padding.

I am pretty sure general rule of a thumb is adding Padding to controls that have BorderBrush/Thickness, and not Spacing.
As it makes a lot of sense to adjust border padding when you have one.

Avalonia panels don't have any border properties except Background. Which can be discussed as a feature on itself separately.

[robloo]

@maxkatz6 Yes, historically that's true.

But my argument is simply Padding is now dual-use for controls that also have Spacing properties. Spacing controls distance between items, Padding controls distance outside items.

Edit: I'm really not sure why this isn't being understood or acknowledged. Even if you disagree. But it seems the two are related which is why I don't want to separate this out as a new topic and then apply it to all panels. Not all panels need it yet. The argument that it's better to add the property incrementally for controls (like this one) that can use it now is pretty strong as well. Then it can be moved to the base class as a non (API) breaking change when the time is right.