Skip to content

fix(FlexPanel): honor VerticalAlignment.Stretch as definite block axis#176

Merged
codemonkeychris merged 2 commits into
mainfrom
fix/flexpanel-vertical-stretch-fill
May 7, 2026
Merged

fix(FlexPanel): honor VerticalAlignment.Stretch as definite block axis#176
codemonkeychris merged 2 commits into
mainfrom
fix/flexpanel-vertical-stretch-fill

Conversation

@codemonkeychris
Copy link
Copy Markdown
Collaborator

Summary

  • FlexPanel's block axis was keyed off the panel's declared Height only — the parent's offer was ignored even when VerticalAlignment.Stretch + a finite parent slot meant "fill". The canonical web flex pattern of header(auto) / body(flex:1) / footer(auto) filling a viewport required a hardcoded .Height(N), and a flex:1 body resolved to an empty pool.
  • Three coordinated changes in MeasureOverride, ArrangeOverride, and the Yoga MeasureFunction wrapper fix this without a parent-type sniff and without regressing the wobble fix from fix(FlexPanel): eliminate +/-1 px height wobble on resize #172.
  • New self-test Flex_HeaderBodyFooterFillsParentSlot covers the canonical pattern under a Reactor host with no explicit .Height(N) on the column.

Why three changes

  1. MeasureOverride — when no explicit Height, VerticalAlignment.Stretch, and parent offer is finite, treat block axis as definite. Symmetric counterpart to the existing HorizontalAlignment.Stretch inline-axis fill — same WinUI Stretch contract on both axes.
  2. ArrangeOverride — same opt-in reruns Yoga at finalSize.Height to cover the case where the parent arranges at a different size than the measure offer. _arranging continues to suppress child re-measure, preserving the resize-wobble fix from fix(FlexPanel): eliminate +/-1 px height wobble on resize #172.
  3. MeasureFunction wrapper (load-bearing) — differentiate Yoga's Exactly mode (real stretch-fit, pass finite) from AtMost / Undefined (basis / content phase, pass infinity). Without this, a nested FlexPanel with default VerticalAlignment.Stretch would treat AtMost's soft cap as a fill target and report the cap as its DesiredSize, defeating the outer's flex-grow distribution across siblings. With it, nested FlexPanels behave correctly under any intermediate (Border, Grid, ContentControl, etc.).

Test plan

  • dotnet test tests/Reactor.Tests — 6819 passed, 0 failed
  • dotnet test tests/Reactor.SelfTests — 668 passed, 0 failed (includes the new Flex_HeaderBodyFooterFillsParentSlot fixture)
  • dotnet run --project tests/Reactor.AppTests.Host -- --self-test --filter \"Flex\" — all 31 flex fixtures pass, including new HBF
  • Manual: a demo app with FlexColumn(titleBar, header, ..., listView.Flex(grow: 1, basis: 0)).Padding(24) (no explicit .Height) now fills the window and the ScrollView produces a scrollbar instead of overflowing
  • Recommended for reviewers: spot-check the wobble-prone scenarios (pure-width window resize over a header-rich column) — _arranging should still suppress the rerun's child re-measure, and Measure-time DesiredSize is still content-sized when parent offer is infinite

🤖 Generated with Claude Code

FlexPanel's MeasureOverride keyed the block axis off the panel's
declared Height only — the parent's offer (availableSize.Height) was
ignored even when the panel's VerticalAlignment.Stretch + a finite
parent slot meant "fill". As a result the canonical web flex pattern
of header(auto) / body(flex:1) / footer(auto) filling a viewport
required a hardcoded .Height(N), and a flex:1 body would resolve to
an empty pool (basis:0 with no container size to grow into).

Changes:

  - MeasureOverride: when no explicit Height, VerticalAlignment.Stretch,
    and parent's offer is finite, treat block axis as definite. This is
    the symmetric counterpart to the existing HorizontalAlignment.Stretch
    inline-axis fill — same WinUI Stretch contract on both axes.

  - ArrangeOverride: same opt-in reruns Yoga at finalSize.Height to
    cover the case where the parent's arrange allocation differs from
    the measure offer. _arranging suppresses child re-measure during
    the rerun, preserving the resize-wobble fix from #172.

  - MeasureFunction wrapper (the load-bearing piece): differentiate
    Yoga's Exactly mode (true stretch-fit — pass finite to inner) from
    AtMost / Undefined (basis / content phase — pass infinity). Without
    this, a nested FlexPanel with default VerticalAlignment.Stretch
    would treat AtMost's soft cap as a fill target and report the cap
    as its DesiredSize, defeating the outer's flex-grow distribution
    across siblings. With it, nested FlexPanels behave correctly under
    any intermediate (Border, Grid, ContentControl, etc.) without a
    parent-type sniff.

New self-test: Flex_HeaderBodyFooterFillsParentSlot covers the
canonical pattern under a Reactor host with no explicit .Height(N) on
the column — body resolves to slot − header − footer and overflowing
content scrolls.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

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 updates FlexPanel’s layout policy so a FlexColumn can treat the block axis as definite when VerticalAlignment.Stretch and the parent provides a finite slot, enabling the common header(auto) / body(flex:1) / footer(auto) pattern to fill a viewport without requiring an explicit .Height(N).

Changes:

  • Update FlexPanel.MeasureOverride to treat block-axis size as definite when VerticalAlignment.Stretch and the parent’s height offer is finite.
  • Update FlexPanel.ArrangeOverride to optionally re-run Yoga at finalSize.Height (while preserving the _arranging guard from #172).
  • Add a new self-test fixture and register it in the self-test registry.
Show a summary per file
File Description
src/Reactor/Yoga/FlexPanel.cs Adjusts block-axis definiteness rules in measure/arrange and changes Yoga→WinUI measure-mode mapping.
tests/Reactor.AppTests.Host/SelfTest/Fixtures/FlexLayoutFixtures.cs Adds an end-to-end self-test for header/body/footer filling behavior without explicit height.
tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs Registers the new self-test fixture name and factory mapping.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 3/3 changed files
  • Comments generated: 2

Comment thread src/Reactor/Yoga/FlexPanel.cs Outdated
Comment thread tests/Reactor.AppTests.Host/SelfTest/Fixtures/FlexLayoutFixtures.cs Outdated
Changes from review:

1. Revert MeasureFunction wrapper to original AtMost → finite mapping.
   The earlier AtMost → infinity mapping broke TextBlock wrapping in
   cross-axis flex layouts: Measure(width=∞) yields a 1-line tall
   result; Measure(width=200) yields the correct wrapped multi-line
   height. Yoga uses AtMost for FitContent sizing, so any wrapping
   text under a non-Stretch align-items would have under-reported its
   row height and clipped.

2. Move the nested-FlexPanel disambiguation to a [ThreadStatic]
   _outerYogaHeightMode the wrapper publishes around child.Measure.
   FlexPanel.MeasureOverride consults it to tell apart:
     - WinUI AtMost from any non-Yoga parent → standard contract
       (Stretch + finite = fill).
     - Yoga AtMost from an outer FlexPanel doing basis/FitContent
       measurement → soft cap, NOT a fill target. Treating it as fill
       would make the inner report the cap as DesiredSize and defeat
       the outer's flex-grow distribution across siblings.
     - Yoga Exactly → stretch-fit allocation, fill.
   This is what handles nested FlexPanels through any intermediate
   (Border, Grid, ContentControl) without a brittle parent-type sniff.

3. Rename FlexColumnFillsParentSlot →
   FlexHeaderBodyFooterFillsParentSlot to match the registry key
   (Copilot review nit; matches grepability of other fixtures).

4. New self-test FlexBorderAutoHeightInflatesWithStretch documents
   the known trade-off of measure-time fillBlockAxis: a Border with
   auto-height around a default-Stretch FlexColumn inflates to the
   parent's offer rather than shrink-wrapping. Asserts BOTH halves —
   default Stretch inflates above 120px content, explicit
   .VAlign(Top) shrink-wraps to 120px. If a future refactor lands web
   flex semantics without inflating auto-height intermediates, the
   inflate assertion should flip.

5. New self-test FlexAtMostPreservesTextWrapping verifies the
   wrapper's AtMost handling didn't regress text wrapping. Long text
   in a 200px column wraps to ≥ 40px height; under a broken
   AtMost-as-infinity wrapper it would collapse to ~18px (1 line).

Test gates: 6819 unit tests pass, 670 selftests pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@codemonkeychris
Copy link
Copy Markdown
Collaborator Author

Update pushed (3aec44e) addressing both Copilot review comments and the broader agent review.

What I changed

1. AtMost → infinity mapping reverted (Copilot point 1, agent point 1). Both reviewers correctly flagged that mapping YogaMeasureMode.AtMost to +∞ would under-report wrapped text height: TextBlock.Measure(width=∞) yields a 1-line result, TextBlock.Measure(width=200) yields the correct wrapped multi-line height. The wrapper now passes finite w + margin for both AtMost and Exactly, restoring the standard WinUI Measure contract.

The nested-FlexPanel discrimination (the original reason I changed the wrapper) is now done via a [ThreadStatic] _outerYogaHeightMode the wrapper publishes around child.Measure. FlexPanel.MeasureOverride reads it to tell:

  • null → not nested under a flex measurement → use the WinUI Stretch contract.
  • Exactly → stretch-fit allocation, fill.
  • Undefined / AtMost → outer flex wants content size, do not fill (otherwise the inner reports the cap as DesiredSize and defeats the outer's flex-grow distribution).

This handles nesting through any intermediate (Border, Grid, ContentControl) without a brittle parent-type sniff, which was the failure mode of an earlier draft.

2. Class name (Copilot point 2). Renamed FlexColumnFillsParentSlotFlexHeaderBodyFooterFillsParentSlot to match the registry key.

3. New regression test for AtMost (agent point 5). Flex_AtMostPreservesTextWrapping measures long text in a 200px column and asserts wrapped height ≥ 40px. Under a broken AtMost-as-infinity wrapper the height collapses to ~18px (1 line) — guards against the regression both reviewers flagged.

What I'm pushing back on

Agent point 2 — "keep auto block-size content-sized during measure unless Height is explicit; the arrange-time rerun handles fill."

I tried this. It does not fix the user's primary scenario. Diagnostic from running the HBF fixture with measure-time fillBlockAxis removed:

# DIAG col=784x750 sv.Render=640 sv.Viewport=640 sv.Extent=640 sv.Scrollable=0 sv.Desired=640

The column resolved to its content height (750) instead of the host's slot. The host's ContentPresenter arranges at min(slot, child.DesiredSize) = DesiredSize when the child is shorter than the slot, so ArrangeOverride.sizeChanged is false and the rerun never fires. Without measure-time fill, flex-grow children have no pool to expand into in the underflow case, which is the normal pre-overflow state of any web flex layout.

The arrange-time rerun is necessary but not sufficient — it covers the slot-overflow case (parent passes slot, child wants more) but misses the slot-underflow case (parent passes slot, child wants less). The user's dryrun.cs is the underflow case (8 items × 80px + chrome = 750 < 768 window), which is exactly when measure-time fill matters.

The trade-off the agent is right about is real, though: a Border with auto-height wrapping a default-Stretch FlexColumn will inflate to the parent's offer rather than shrink-wrap to content, because we are mapping "Stretch + finite parent" to CSS web-flex height: 100% at measure time. New self-test Flex_BorderAutoHeightInflatesWithStretch makes this testable and documents the opt-out — .VAlign(VerticalAlignment.Top) makes the inner shrink-wrap as expected. Asserts both halves so a future refactor that lands web-flex semantics without auto-height inflation has a known signpost to flip.

Agent points 3 (AlignContent default) and 4 (arrange-time stale measurements) — both fair observations but out of scope for this PR. AlignContent default is a separate semantic change (would affect any wrapped multi-line flex layout); arrange-time stale measurements is the existing wobble-fix tradeoff from #172, which this PR doesn't change.

Test results

  • dotnet test tests/Reactor.Tests — 6819 passed, 0 failed
  • dotnet test tests/Reactor.SelfTests — 670 passed, 0 failed (3 new fixtures: HBF, Border auto-height, AtMost text wrapping)

@codemonkeychris codemonkeychris merged commit 0c3452c into main May 7, 2026
6 checks passed
@codemonkeychris codemonkeychris deleted the fix/flexpanel-vertical-stretch-fill branch May 7, 2026 00:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants