Skip to content

[Grid] Add RowSpacing and ColumnSpacing #18077

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

Merged
merged 12 commits into from
Mar 8, 2025

Conversation

zxbmmmmmmmmm
Copy link
Contributor

What does the pull request do?

Add RowSpacing and ColumnSpacing properties for Grid, just like in UWP and WinUI 3.
image

What is the updated/expected behavior with this PR?

  1. Add a Grid with various children.
  2. Arrange children in different row and columns.
  3. Set RowSpacing and ColumnSpacing to non-zero values.

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

Add the corresponding spacing before calculating the position of each RowDefinition or ColumnDefinition.
Subtract spacing before calculating stars when GridUnitType=star.

Checklist

Breaking changes

None

Obsoletions / Deprecations

None

Fixed issues

Fixes #5152

@avaloniaui-bot
Copy link

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

@cla-avalonia
Copy link
Collaborator

cla-avalonia commented Jan 30, 2025

  • All contributors have signed the CLA.

@zxbmmmmmmmmm
Copy link
Contributor Author

@cla-avalonia agree

/// Defines the <see cref="ColumnSpacing"/> property.
/// </summary>
public static readonly StyledProperty<double> ColumnSpacingProperty =
AvaloniaProperty.Register<Grid, double>(nameof(ColumnSpacingProperty));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to add coercion for negative values? Or does UWP support them?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

negative spacing is not recommended feature, but it is supported in uwp/winui3. And so do this PR. This PR support negative spacing, too.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also think we should support negative Spacing if possible. It matches with what is allowed for Margin/Padding which can be negative.

There are some rare layout cases it's useful and by default the math usually supports negatives for free.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now both the spacing in uwp/winui3 and in these 3 PRs don't support negative values in principle, but in practice they do support (

@robloo
Copy link
Contributor

robloo commented Jan 30, 2025

Great work! This has been a nice to have for a long time.

@MrJul
Copy link
Member

MrJul commented Feb 1, 2025

I'm marking this PR with needs-api-review for the sake of it but we were fine with this for UniformGrid in #17993 (comment) so I really don't see it being rejected.

@MrJul MrJul added the needs-api-review The PR adds new public APIs that should be reviewed. label Feb 1, 2025
@robloo
Copy link
Contributor

robloo commented Feb 1, 2025

I'm marking this PR with needs-api-review for the sake of it but we were fine with this for UniformGrid... so I really don't see it being rejected.

It also matches UWP/WinUI as-is and I've never heard of concerns with that API.

@DoubleDBE
Copy link

We need this 😁

@MrJul
Copy link
Member

MrJul commented Mar 5, 2025

Public API for review:

namespace Avalonia.Controls
{
    public class Grid : Panel
    {
+        public static readonly StyledProperty<double> RowSpacingProperty;
+        public static readonly StyledProperty<double> ColumnSpacingProperty;
+        public double RowSpacing { get; set; }
+        public double ColumnSpacing { get; set; }
    }
}

@MrJul
Copy link
Member

MrJul commented Mar 6, 2025

The API has been approved.

The implementation will be reviewed soon.
@zxbmmmmmmmmm would you be able to handle any bug in the future that comes from the new implementation?

@MrJul MrJul added api-approved The new public APIs have been approved. and removed needs-api-review The PR adds new public APIs that should be reviewed. labels Mar 6, 2025
@zxbmmmmmmmmm
Copy link
Contributor Author

This PR is partially based on the implementation of WinUI 3 and was written by @Poker-sang and me.
If there are any issues, we should be able to solve them together.

Copy link
Member

@MrJul MrJul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you!
A very nice feature to have :)

@MrJul MrJul added this pull request to the merge queue Mar 8, 2025
Merged via the queue into AvaloniaUI:master with commit 7d3f490 Mar 8, 2025
11 checks passed
@zxbmmmmmmmmm zxbmmmmmmmmm deleted the feat/grid-spacing branch March 8, 2025 23:44
@xllifi
Copy link

xllifi commented Mar 18, 2025

Grid lines are broken
image


Elements overflow when there's not enough space

2025-03-19.02-08-20.mp4

Element shown in video
<Border Background="{DynamicResource UiTheme01}" BorderBrush="{DynamicResource UiTheme04}"
    BorderThickness="1" CornerRadius="8">
    <Grid Margin="8" ColumnSpacing="8" HorizontalAlignment="Stretch" VerticalAlignment="Top">
        <Grid.ColumnDefinitions>
            <ColumnDefinition Width="Auto" />
            <ColumnDefinition Width="*" />
            <ColumnDefinition Width="Auto" />
        </Grid.ColumnDefinitions>
        <Grid.RowDefinitions>
            <RowDefinition />
        </Grid.RowDefinitions>

        <TextBlock Grid.Column="0" VerticalAlignment="Center" Text="Путь до Steam:" />
        <TextBox Name="SteamPathTextBox" Grid.Column="1" VerticalAlignment="Center" 
            Background="{DynamicResource UiTheme02}" BorderThickness="0" Text="C:\Program Files\Steam\steam.exe" />
        <Button Grid.Column="2" Width="32" Height="32" VerticalAlignment="Center" Click="SetSteamPath">
            <lucideAvalonia:Lucide Icon="Folder" StrokeBrush="{DynamicResource White}" />
        </Button>
    </Grid>
</Border>

Avalonia nightly 11.3.999-cibuild0055611-alpha
I'm using Romzetron.Avalonia 11.2.1 theme but the same happens with Avalonia.Themes.Fluent 11.3.999-cibuild0055611-alpha as well

@Poker-sang Poker-sang mentioned this pull request Mar 18, 2025
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-approved The new public APIs have been approved. feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Grid: Add RowSpacing and ColumnSpacing
9 participants