fix(grid): stretch unequal flex-col cells without the IntrinsicHeight assert (#139)#140
Conversation
… assert grid items-stretch (#126/#137) crashed with "RenderBox was not laid out" (rooted at RenderIntrinsicHeight -> "LayoutBuilder does not support returning intrinsic dimensions") whenever it had to STRETCH an unequal cell whose content is a flex flex-col (a card): such a cell carries a LayoutBuilder (cross-axis stretch / h-full / basis-*), and IntrinsicHeight queries child intrinsics, which LayoutBuilder cannot answer. So the feature crashed exactly the unequal-height case it exists for. Replace IntrinsicHeight + Row(stretch) with WindEqualHeightRow, a RenderObject that measures each cell with a real loose-height layout and re-lays it tight to the row's max height. Real layout is what LayoutBuilder supports, so flex-col cells stretch correctly. Each cell also gets an equal share of the width (matching the fixed column count). Bonus: h-full inside a stretched cell now resolves too. Post-change sync: wind_equal_height_row_test (stretch, width split, spacing setter, LayoutBuilder child, hit-test, unbounded-width, empty) + grid _stretch_test #139 case; doc/layout/grid.md, skill (tokens.md + layouts.md, version 2.8.1), CHANGELOG. Closes #139
|
Warning Review limit reached
Next review available in: 45 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughReplaces the IntrinsicHeight-based equal-height grid stretching in ChangesEqual-Height Row Fix
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant WDiv
participant WindEqualHeightRow
participant RenderEqualHeightRow
WDiv->>WindEqualHeightRow: build rowChildren with spacing (gapX)
WindEqualHeightRow->>RenderEqualHeightRow: performLayout
RenderEqualHeightRow->>RenderEqualHeightRow: measure each child (loose height)
RenderEqualHeightRow->>RenderEqualHeightRow: compute max height across children
RenderEqualHeightRow->>RenderEqualHeightRow: re-layout each child (tight to max height)
RenderEqualHeightRow-->>WDiv: return sized, positioned row
Related issues: Suggested labels: bug, layout, widgets Suggested reviewers: fluttersdk maintainers familiar with 🐇 A row of cards, once tall and small, 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
Fixes a regression in the grid ... items-stretch implementation by removing the IntrinsicHeight-based equal-height mechanism (which asserts with LayoutBuilder children) and replacing it with a real two-pass layout render helper.
Changes:
- Introduces
WindEqualHeightRow(aRenderObject-backed widget) to measure tallest cell via real layout and then relayout all cells to that height. - Updates
WDiv’sitems-stretchgrid path to useWindEqualHeightRowwithgapXasspacing. - Updates docs/skill/changelog and adds tests for the new equal-height row and the #139 regression case.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| test/widgets/wind_equal_height_row_test.dart | Adds widget tests for equal-height behavior, spacing updates, LayoutBuilder child, hit testing, and unbounded-width degradation. |
| test/widgets/w_div/grid_stretch_test.dart | Adds a regression test covering unequal flex flex-col grid cells with nested WDiv (#139). |
| lib/src/widgets/wind_equal_height_row.dart | Adds the intrinsic-free equal-height row render helper used by the grid items-stretch path. |
| lib/src/widgets/w_div.dart | Replaces IntrinsicHeight + Row(stretch) with WindEqualHeightRow for items-stretch grids. |
| doc/layout/grid.md | Updates grid docs to reflect the intrinsic-free stretch mechanism and removes the obsolete h-full caveat. |
| skills/wind-ui/SKILL.md | Bumps skill version to 2.8.1. |
| skills/wind-ui/references/tokens.md | Updates grid-cols-N token documentation to reflect real-layout equal-height rows and #139. |
| skills/wind-ui/references/layouts.md | Updates grid guidance to reflect intrinsic-free stretching and #139. |
| CHANGELOG.md | Adds a “Fixed” entry documenting the #139 regression and the switch to WindEqualHeightRow. |
| expect(tester.takeException(), isNull); | ||
| // The LayoutBuilder cell was stretched to the 80px tall sibling. | ||
| expect(tester.getSize(find.text('lb').first), isNotNull); | ||
| }); |
| const WindEqualHeightRow({ | ||
| super.key, | ||
| this.spacing = 0, | ||
| required super.children, | ||
| }); |
| double _spacing; | ||
| set spacing(double value) { | ||
| if (value == _spacing) return; | ||
| _spacing = value; | ||
| markNeedsLayout(); | ||
| } |
| // Pass 2: stretch every cell to the row height and position left to right. | ||
| double dx = 0; | ||
| child = firstChild; | ||
| while (child != null) { | ||
| child.layout( | ||
| BoxConstraints.tightFor(width: cellWidth, height: maxHeight), | ||
| parentUsesSize: true, | ||
| ); | ||
| (child.parentData as _EqualHeightParentData).offset = Offset(dx, 0); | ||
| dx += cellWidth + _spacing; | ||
| child = childAfter(child); | ||
| } | ||
|
|
||
| final double width = constraints.maxWidth.isFinite | ||
| ? constraints.maxWidth | ||
| : (dx - _spacing).clamp(0.0, double.infinity); | ||
| size = constraints.constrain(Size(width, maxHeight)); |
| // WindEqualHeightRow measures each cell with a real (loose-height) layout | ||
| // and re-lays it tight to the row max, instead of IntrinsicHeight's | ||
| // intrinsic query. A `flex flex-col` cell (which carries a LayoutBuilder) | ||
| // can then be stretched without the "LayoutBuilder does not support | ||
| // returning intrinsic dimensions" assert (#139). | ||
| rows.add(WindEqualHeightRow(spacing: gapX, children: rowChildren)); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/src/widgets/wind_equal_height_row.dart`:
- Around line 88-93: `WindEqualHeightRow` is still allowing `h-full` to use
viewport height during the first measuring pass, which can incorrectly stretch
an `items-stretch` row. Update the measuring logic in
`wind_equal_height_row.dart` so pass 1 treats `h-full` as content-sized before
tightening constraints, likely in the layout path around `child.layout` and the
equal-height measurement flow. Add a regression test for a `grid grid-cols-2
items-stretch` row with one shorter `h-full` cell to verify the row matches the
tallest sibling instead of the viewport.
In `@test/widgets/wind_equal_height_row_test.dart`:
- Around line 69-91: The test in the WindEqualHeightRow layout case does not
actually verify the claimed stretch behavior because checking that
tester.getSize(find.text('lb').first) is not null adds no meaningful assertion.
Update the testWidgets scenario for WindEqualHeightRow to assert the
LayoutBuilder child’s rendered height matches the 80px sibling height, using the
existing find.text('lb') lookup and a real size comparison rather than a null
check.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d708a89c-befb-4e48-b54d-295fffa01ce7
📒 Files selected for processing (9)
CHANGELOG.mddoc/layout/grid.mdlib/src/widgets/w_div.dartlib/src/widgets/wind_equal_height_row.dartskills/wind-ui/SKILL.mdskills/wind-ui/references/layouts.mdskills/wind-ui/references/tokens.mdtest/widgets/w_div/grid_stretch_test.darttest/widgets/wind_equal_height_row_test.dart
| child.layout( | ||
| BoxConstraints( | ||
| minWidth: cellWidth, | ||
| maxWidth: cellWidth, | ||
| maxHeight: constraints.maxHeight, | ||
| ), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
git ls-files lib/src/widgets/wind_equal_height_row.dart lib/src | sed -n '1,120p'Repository: fluttersdk/wind
Length of output: 3483
🏁 Script executed:
ast-grep outline lib/src/widgets/wind_equal_height_row.dart --view expanded && printf '\n---\n' && ast-grep outline lib/src/widgets/w_div.dart --view expandedRepository: fluttersdk/wind
Length of output: 256
🏁 Script executed:
wc -l lib/src/widgets/wind_equal_height_row.dart lib/src/widgets/w_div.dart
printf '\n--- wind_equal_height_row.dart ---\n'
cat -n lib/src/widgets/wind_equal_height_row.dart | sed -n '1,220p'
printf '\n--- w_div.dart grep ---\n'
rg -n "h-full|MediaQuery\.size\.height|maxHeight|height" lib/src/widgets/w_div.dart lib/src/widgets/wind_equal_height_row.dartRepository: fluttersdk/wind
Length of output: 13771
🏁 Script executed:
sed -n '1140,1215p' lib/src/widgets/w_div.dart
printf '\n---\n'
sed -n '1490,1665p' lib/src/widgets/w_div.dart
printf '\n--- tests grep ---\n'
rg -n "WindEqualHeightRow|items-stretch|h-full|equal-height" test lib/src -g '*test.dart'Repository: fluttersdk/wind
Length of output: 13415
🏁 Script executed:
sed -n '1,220p' test/widgets/w_div/grid_stretch_test.dart
printf '\n---\n'
sed -n '1,220p' test/widgets/wind_equal_height_row_test.dartRepository: fluttersdk/wind
Length of output: 10558
🏁 Script executed:
python3 - <<'PY'
from pathlib import Path
row = Path('lib/src/widgets/wind_equal_height_row.dart').read_text()
div = Path('lib/src/widgets/w_div.dart').read_text()
checks = {
"row_pass1_forwards_constraints_maxHeight": "maxHeight: constraints.maxHeight" in row,
"h_full_unbounded_uses_mediaquery": "MediaQuery.of(context).size.height" in div and "h-full needs LayoutBuilder to handle unbounded vertical axis" in div,
"stretch_grid_uses_equal_height_row": "WindEqualHeightRow(spacing: gapX, children: rowChildren)" in div,
}
print(checks)
assert all(checks.values()), checks
print("source-path supports the reported behavior")
PYRepository: fluttersdk/wind
Length of output: 333
Avoid measuring h-full against the viewport in the first pass
WindEqualHeightRow still lets h-full resolve to MediaQuery.size.height during pass 1, so one cell can make an items-stretch row viewport-tall instead of matching the tallest content sibling. Add a regression test for a grid grid-cols-2 items-stretch row with a shorter h-full cell, and treat h-full as content-sized during the measuring pass before tightening.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/src/widgets/wind_equal_height_row.dart` around lines 88 - 93,
`WindEqualHeightRow` is still allowing `h-full` to use viewport height during
the first measuring pass, which can incorrectly stretch an `items-stretch` row.
Update the measuring logic in `wind_equal_height_row.dart` so pass 1 treats
`h-full` as content-sized before tightening constraints, likely in the layout
path around `child.layout` and the equal-height measurement flow. Add a
regression test for a `grid grid-cols-2 items-stretch` row with one shorter
`h-full` cell to verify the row matches the tallest sibling instead of the
viewport.
- Validate spacing (finite, non-negative) in both the widget constructor and the render-object setter (debug asserts). - Honor the incoming height constraint: cells stretch to maxHeight.clamp(minHeight, maxHeight), not just the measured tallest, so a parent-imposed tight/min height stays consistent with the cells. - Update the stale _buildStretchGrid dartdoc (was still describing the old IntrinsicHeight + h-full caveat). - Make the LayoutBuilder-child test assert the actual stretched height (80) instead of a vacuous non-null check.
|
Addressed all five review comments:
@coderabbitai review |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
fd wind_equal_height_row.dart
echo "---"
cat -n lib/src/widgets/wind_equal_height_row.dartLength of output: 6345
I'll kick off a fresh review to double-check the test file changes and everything else. ᕙ(⇀‸↼‶)ᕗ ✅ Action performedReview finished.
|
Summary
Fixes #139, a regression in the #126/#137
items-stretchgrid.grid ... items-stretchcrashed withRenderBox was not laid out(rooted atRenderIntrinsicHeight→LayoutBuilder does not support returning intrinsic dimensions) the moment it had to stretch an unequalflex flex-colcell — i.e. the exact case the feature exists for. Equal-height cells hid the bug; a taller sibling triggered it.Root cause (reproduced first)
The stretch grid wrapped each row in
IntrinsicHeight+Row(crossAxisAlignment: stretch).IntrinsicHeightmeasures children via the intrinsic protocol. Aflex flex-colcell (or one usingh-full/basis-*) carries aLayoutBuilder, andLayoutBuildercannot answer intrinsic (or dry-layout) queries — so stretching a shorter cell asserted. Same root construct as #129.Fix
Replace
IntrinsicHeight+Row(stretch)withWindEqualHeightRow, a smallRenderObjectthat:LayoutBuilderfully supports) to find the tallest,spacingbetween columns.Real layout instead of intrinsic queries means LayoutBuilder-bearing cells stretch correctly. Bonus:
h-fullinside a stretched cell now resolves too (previously documented as unsupported).Tests
test/widgets/wind_equal_height_row_test.dart: stretch-to-tallest, equal width split,spacingsetter (rebuild), aLayoutBuilderchild laid out without asserting, hit-testing, unbounded-width degradation, empty.test/widgets/w_div/grid_stretch_test.dart: newitems-stretchasserts (RenderIntrinsicHeight) when stretching an unequal-heightflex flex-colcell #139 case — unequalflex flex-colcells with a nestedWDivstretch without asserting and equalize. The existing 5 grid tests still pass.Definition of done
dart analyze: clean.dart format: no diff.flutter test: all pass, coverage 94.4% ≥ 90% (wind_equal_height_row.dartfully covered).doc/layout/grid.md(mechanism + dropped the now-obsolete "no h-full on a stretched cell" caveat), skill (tokens.md+references/layouts.md,SKILL.mdversion 2.8.1),CHANGELOG.md.WindEqualHeightRowis an internal render helper (not exported / not a public W-widget), so the README roster is unchanged.Summary by CodeRabbit
New Features
Bug Fixes
Documentation