Skip to content

Commit 7191cb6

Browse files
committed
fix(grid): stop double-subtracting a cell child's margin (Auto-cell truncation)
GridLayout applied a cell child's own Margin twice: once when sizing/placing the child (it subtracted childMargin to form the pass-2 re-measure width and the arrange alignment box) and again inside the control's own paint (e.g. MarkupControl insets its content by Margin). The result was content measured and arranged at cellWidth - 2*margin, which truncated a tight Auto-cell label ("Animation" -> "Animati") and could force needless wrapping. The framework convention (VerticalStackLayout / WindowContentLayout) is that the container hands the child its full content box — border and container padding removed — and the CONTROL owns its own margin. GridLayout now follows it: - pass-2 re-measure: add the child margin back onto the measure width (innerW already excludes it; the control re-subtracts it), and - arrange: align DesiredSize within the full content box without pre- insetting by the child margin. Update GridArrangeTests.CellChildMargin_InsetsWithinCell, which encoded the old double-inset (it expected the child's bounds inset by its margin); it now asserts full-cell bounds, matching every other layout. Add a regression test (AutoColumn_MarginedMarkup_DoesNotWrap) and drop the Cells() workaround in the GridDashboard Settings page (natural Auto tracks now render correctly). Full suite green; AOT clean.
1 parent e2ea15b commit 7191cb6

4 files changed

Lines changed: 74 additions & 17 deletions

File tree

Examples/GridDashboard/Pages.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -263,8 +263,8 @@ public static GridControl BuildLogsGrid(DashboardRefs refs)
263263
public static GridControl BuildSettingsGrid()
264264
{
265265
var grid = FillGrid()
266-
.Columns(GridLength.Cells(16), GridLength.Star(1))
267-
.Rows(GridLength.Cells(2), GridLength.Cells(2), GridLength.Cells(3), GridLength.Star(1))
266+
.Columns(GridLength.Auto(), GridLength.Star(1))
267+
.Rows(GridLength.Auto(), GridLength.Auto(), GridLength.Auto(), GridLength.Star(1))
268268
.RowGap(1)
269269
.ColumnGap(2)
270270
.WithPadding(2, 2, 2, 2)
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
// -----------------------------------------------------------------------
2+
// ConsoleEx - GridControl Auto-track margin double-subtraction regression
3+
// -----------------------------------------------------------------------
4+
using SharpConsoleUI;
5+
using SharpConsoleUI.Controls;
6+
using SharpConsoleUI.Layout;
7+
using SharpConsoleUI.Tests.Infrastructure;
8+
using Xunit;
9+
using CB = SharpConsoleUI.Builders.Controls;
10+
using Color = SharpConsoleUI.Color;
11+
12+
namespace SharpConsoleUI.Tests.Controls;
13+
14+
public class GridAutoMarginMeasureTests
15+
{
16+
// An Auto column whose content is a Markup WITH a horizontal margin must size to the content's
17+
// FULL width (incl. its margin) and NOT force the content to wrap. Previously the grid subtracted
18+
// the child margin to compute the pass-2 re-measure width, and the control's own MeasureDOM
19+
// subtracted the margin AGAIN — so content got (colWidth - 2*margin) and a single-line label
20+
// like "Animation" wrapped to two lines.
21+
[Fact]
22+
public void AutoColumn_MarginedMarkup_DoesNotWrap()
23+
{
24+
var label = CB.Markup("Animation").WithMargin(1, 0, 1, 0).Build();
25+
var grid = CB.Grid()
26+
.Columns(GridLength.Auto(), GridLength.Star(1))
27+
.Rows(GridLength.Star(1))
28+
.Place(label, 0, 0)
29+
.Place(CB.Markup("value").Build(), 0, 1)
30+
.WithAlignment(HorizontalAlignment.Stretch)
31+
.WithVerticalAlignment(VerticalAlignment.Fill)
32+
.Build();
33+
34+
var system = TestWindowSystemBuilder.CreateTestSystem(60, 10);
35+
var window = new Window(system) { Title = "T", Left = 0, Top = 0, Width = 60, Height = 10 };
36+
window.AddControl(grid);
37+
system.AddWindow(window);
38+
system.Render.UpdateDisplay();
39+
system.Render.UpdateDisplay();
40+
41+
// "Animation" is 9 columns; on one line the label height stays 1. A wrapped label is 2+ rows.
42+
Assert.Equal(1, label.ActualHeight);
43+
}
44+
}

SharpConsoleUI.Tests/Layout/GridArrangeTests.cs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -144,11 +144,15 @@ public void CellChildMargin_InsetsWithinCell()
144144
MeasureArrange(layout, node, 40, 10);
145145

146146
var child = node.Children[0];
147-
// Cell is the full 40x10; child margin insets by 1 on each side.
148-
Assert.Equal(1, child.Bounds.X);
149-
Assert.Equal(1, child.Bounds.Y);
150-
Assert.Equal(40 - 2, child.Bounds.Width);
151-
Assert.Equal(10 - 2, child.Bounds.Height);
147+
// The grid arranges the child into the FULL cell content box and does NOT pre-inset by the child's
148+
// own margin — the control applies its margin itself during paint (its leftInset/topInset). This
149+
// matches VerticalStackLayout/WindowContentLayout (the framework convention) and avoids the
150+
// double-subtraction that truncated tight Auto-cell labels. So the child's bounds span the whole
151+
// cell; the margin shows up as the control's own internal inset, not a smaller arranged rect.
152+
Assert.Equal(0, child.Bounds.X);
153+
Assert.Equal(0, child.Bounds.Y);
154+
Assert.Equal(40, child.Bounds.Width);
155+
Assert.Equal(10, child.Bounds.Height);
152156
}
153157

154158
[Fact]

SharpConsoleUI/Layout/GridLayout.cs

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -345,12 +345,21 @@ private bool RemeasureCellsAgainstCellExtent(
345345
continue;
346346
}
347347

348+
// Add the child's own margin back onto the measure width. CellInnerExtent already subtracted
349+
// the child margin (it is the alignment box used by arrange), but a control's MeasureDOM treats
350+
// constraints.MaxWidth as its TOTAL allocation and subtracts its margin again internally. Passing
351+
// innerW directly would double-subtract the margin, starving the content by 2*margin and forcing
352+
// a single-line label (e.g. "Animation" in an Auto column) to wrap. Re-add it so the control's
353+
// content gets exactly cellWidth - border - cellPadding - margin, matching pass 1 and arrange.
354+
Margin childMargin = child.Control?.Margin ?? default;
355+
int measureW = innerW + childMargin.Left + childMargin.Right;
356+
348357
// Constrain WIDTH to the cell's inner width so a wrapping control reflows to its column, but
349358
// leave HEIGHT unbounded: the goal is to discover the wrapped height. Clamping height to the
350359
// pass-1 row size would cap the reported height at the single-line measure and prevent an Auto
351360
// row from growing. Fixed/Star rows ignore the reported height (only Auto rows size to content),
352361
// and arrange clips each cell to its track, so an over-tall report is harmless for those rows.
353-
child.Measure(LayoutConstraints.Loose(innerW, int.MaxValue));
362+
child.Measure(LayoutConstraints.Loose(measureW, int.MaxValue));
354363
anyReflow = true;
355364
}
356365

@@ -572,18 +581,18 @@ public void ArrangeChildren(LayoutNode node, LayoutRect finalRect)
572581
var cellRect = new LayoutRect(contentX, contentY, contentW, contentH);
573582
_cellRects[child] = cellRect;
574583

575-
// Inset the content area by the child's own margin to form the alignment box.
576-
Margin childMargin = child.Control?.Margin ?? default;
577-
int innerX = contentX + childMargin.Left;
578-
int innerY = contentY + childMargin.Top;
579-
int innerW = Math.Max(0, contentW - childMargin.Left - childMargin.Right);
580-
int innerH = Math.Max(0, contentH - childMargin.Top - childMargin.Bottom);
581-
584+
// Align the child within the cell's CONTENT box (border + cell-padding already removed). Do NOT
585+
// pre-subtract the child's own margin here: the control's own paint (its leftInset/topInset)
586+
// already accounts for its margin, and its DesiredSize already includes it. Subtracting the
587+
// margin to form the alignment box AND letting the control subtract it again in paint would
588+
// double-subtract — starving the content by 2*margin and truncating a tight Auto-cell label
589+
// (e.g. "Animation" -> "Animati"). The grid reserves border + cell-padding; the control owns
590+
// its margin.
582591
HorizontalAlignment hAlign = child.Control?.HorizontalAlignment ?? HorizontalAlignment.Stretch;
583592
VerticalAlignment vAlign = child.Control?.VerticalAlignment ?? VerticalAlignment.Fill;
584593

585-
(int childX, int childW) = AlignHorizontal(hAlign, innerX, innerW, child.DesiredSize.Width);
586-
(int childY, int childH) = AlignVertical(vAlign, innerY, innerH, child.DesiredSize.Height);
594+
(int childX, int childW) = AlignHorizontal(hAlign, contentX, contentW, child.DesiredSize.Width);
595+
(int childY, int childH) = AlignVertical(vAlign, contentY, contentH, child.DesiredSize.Height);
587596

588597
child.Arrange(new LayoutRect(childX, childY, childW, childH));
589598
}

0 commit comments

Comments
 (0)