Skip to content

Conversation

@laske185
Copy link
Contributor

@laske185 laske185 commented Aug 2, 2025

PR Details

Introduced gap functionality to Grid, UniformGrid and StackPanel similar to the gap in CSS.
Add ColumnGap, RowGap, and LayerGap properties in GridBase for Grid and UniformGrid to manage spacing between elements. Extend the StackPanel to include a gap property.

  • Grid: image
  • UniformGrid: image
  • Stackpanel: image

Related Issue

Closes #2852

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My change requires a change to the documentation.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • I have built and run the editor to try this change out.

…onality

- Introduced ColumnGap, RowGap, and LayerGap properties in GridBase to manage spacing between elements.
- Updated Grid and UniformGrid to account for gaps during measurement and arrangement, ensuring proper layout calculations.
- Enhanced UniformGrid tests to validate behavior with gaps, including tests for spanned elements and various configurations.
- Modified StackPanel to include a gap property, affecting the arrangement and measurement of child elements.
- Improved overall layout logic to ensure gaps are considered in both 2D and 3D grid scenarios.
Copilot AI review requested due to automatic review settings August 2, 2025 07:35
@laske185 laske185 changed the title feat: Add gap support to layout panels [UI] Add gap support to layout panels Aug 2, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces gap functionality to layout panels (Grid, UniformGrid, and StackPanel) to provide spacing between child elements, similar to CSS gap properties. The implementation adds column, row, and layer gap properties to grid-based panels and a general gap property to StackPanel.

Key changes:

  • Added ColumnGap, RowGap, and LayerGap properties to GridBase class
  • Implemented gap support in both Grid and UniformGrid classes with proper measure and arrange logic
  • Added Gap property to StackPanel with support for virtualization
  • Comprehensive test coverage for all gap functionality including edge cases

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
GridBase.cs Introduces gap properties and helper methods for grid-based panels
Grid.cs Implements gap logic in measure/arrange cycles and strip positioning
UniformGrid.cs Adds gap support for uniform grid layout with spanned elements
StackPanel.cs Implements gap property with virtualization support
GridTests.cs Comprehensive test coverage for Grid gap functionality
UniformGridTests.cs Test coverage for UniformGrid gap features
StackPanelTests.cs Test coverage for StackPanel gap functionality

Copy link
Collaborator

@Eideren Eideren left a comment

Choose a reason for hiding this comment

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

Sorry for the delay, this is great, just one minor nitpick - I understand that some part of those are there to mirror's how the previous logic was setup, but prefer vector operation over per component operation.

var x = new Vector3(v.X/v2.X, v.Y/v2.Y, v.Z/v2.Z);
// into
var x = v / v2;

I'll mark areas where this could be improved below.

Comment on lines 107 to 116
var totalGapSize = new Vector3(
CalculateTotalGapSize(0, Columns),
CalculateTotalGapSize(1, Rows),
CalculateTotalGapSize(2, Layers));

var availableForCells = availableSizeWithoutMargins - totalGapSize;
var availableForOneCell = new Vector3(
availableForCells.X / gridSize.X,
availableForCells.Y / gridSize.Y,
availableForCells.Z / gridSize.Z);
Copy link
Collaborator

Choose a reason for hiding this comment

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

CalculateTotalGapSize could take and return vectors, and the per component division could be a vector / vector

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Comment on lines 126 to 129
var availableForChildWithMargin = new Vector3(
CalculateSpannedSize(availableForOneCell.X, (int)childSpans.X, 0),
CalculateSpannedSize(availableForOneCell.Y, (int)childSpans.Y, 1),
CalculateSpannedSize(availableForOneCell.Z, (int)childSpans.Z, 2));
Copy link
Collaborator

Choose a reason for hiding this comment

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

CalculateSpannedSize could take and return vectors

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CalculateSpannedSize accepts now two vectors for the cell size and gaps.

Comment on lines 146 to 155
var totalGapSize = new Vector3(
CalculateTotalGapSize(0, Columns),
CalculateTotalGapSize(1, Rows),
CalculateTotalGapSize(2, Layers));

var availableForCells = finalSizeWithoutMargins - totalGapSize;
finalForOneCell = new Vector3(
availableForCells.X / gridSize.X,
availableForCells.Y / gridSize.Y,
availableForCells.Z / gridSize.Z);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Comment on lines 166 to 169
var finalForChildWithMargin = new Vector3(
CalculateSpannedSize(finalForOneCell.X, (int)childSpans.X, 0),
CalculateSpannedSize(finalForOneCell.Y, (int)childSpans.Y, 1),
CalculateSpannedSize(finalForOneCell.Z, (int)childSpans.Z, 2));
Copy link
Collaborator

Choose a reason for hiding this comment

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

And here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Use simplified CalculateSpannedSize too.

Introduces a new `GetGaps` method in `GridBase.cs` to calculate spacing gaps as a `Vector3`. Updates `CalculateSpannedSize` in `UniformGrid.cs` to accept `Vector3` parameters for improved handling of dimensions. Refactors available space calculations and child measurement logic to utilize the new methods, enhancing readability and maintainability. Overall, these changes improve the grid layout functionality and encapsulation of gap calculations.
@laske185
Copy link
Contributor Author

laske185 commented Aug 27, 2025

@Eideren thank you for your review. I applied all suggestions.

@Eideren
Copy link
Collaborator

Eideren commented Aug 27, 2025

Thanks for going through it, just one question

private Vector3 CalculateSpannedSize(Vector3 cellSize, Vector3 span)
{
var gaps = GetGaps() * (span - 1);
return Vector3.Max(cellSize * span + gaps, Vector3.One);
}

This doesn't seem right to me, but I might not fully understand the method, are you sure you want to always return a size >= Vector3.One here ?

Normalized the `span` parameter to ensure it is at least `Vector3.One`, preventing issues with spans less than one. Adjusted the calculation of `gaps` to use the normalized span for accurate gap computation. The return value now reflects these changes while maintaining similar functionality.
@laske185
Copy link
Contributor Author

There was an issue after refactoring what I fixed now.

The function calculates the size of a spanned element, including the gaps between the columns, rows, and layers. Originally the function has returned the size of the element in one dimension when the span is less than or equal to one, because there are no gaps to add in this case. I restored this behavior by normalizing the span parameter to make all dimensions at least one.

@Eideren Eideren merged commit a0f949c into stride3d:master Aug 27, 2025
2 checks passed
@Eideren
Copy link
Collaborator

Eideren commented Aug 27, 2025

Thanks !

@Eideren Eideren changed the title [UI] Add gap support to layout panels feat: [UI] Add gap support to layout panels Aug 27, 2025
@laske185 laske185 deleted the feature/2852-gaps-in-layout-components branch August 27, 2025 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Gaps in layout components

2 participants