From eb9993fb3ee3591f654bbfc81a2b9145fe46ed2b Mon Sep 17 00:00:00 2001 From: Om Singhal Date: Fri, 23 Oct 2026 04:21:55 -0700 Subject: [PATCH] Keep cross axis margins and relative insets of items in wrapping containers Summary: Fixes https://github.com/react/yoga/issues/1856 Fixes https://github.com/react/yoga/issues/1067 In wrapping containers, STEP 8 (multi line alignment) of `CalculateLayout.cpp` repositions items on the cross axis and drops part of each box. In https://github.com/react/yoga/issues/1856 the container is a column (Yoga's default), so `left` is on the cross axis. - `flex-start` uses `computeFlexStartPosition` (the inset) where `computeFlexStartMargin` belongs. v2.0.1 used the margin, so this is a 3.0 regression from https://github.com/react/yoga/issues/1561. - `stretch`, `center` and `flex-end` never add the relative inset. - `center` centers the border box instead of the margin box, which is https://github.com/react/yoga/issues/1067. Every case now uses the leading margin, trailing margin and `relativePosition`. `wrap-reverse` mirrors positions afterwards, so it leads with the flex end margin and applies the inset after mirroring, keeping each margin on its edge and `top: 10` moving the item down. Also, a percentage inset on a `flex-start` item now resolves against the container's cross size, as without wrap (`top: 10%` in a 100x200 row: 10 to 20), and baseline rows, which run this pass even without wrap, honor `bottom` and ignore insets on static items. Column container, padding 10, `flex-wrap: wrap`, default `align-content: flex-start`, item with margin 5 and `left: 10` (item left; expected worked out from the spec): | `align-items` | main | this PR | expected | |---|---|---|---| | `flex-start` | 20 | 25 | 25 | | `stretch` | 15 | 25 | 25 | | `center` | 15 | 25 | 25 | | `flex-end` | 15 | 25 | 25 | | `flex-start`, no inset | 10 | 15 | 15 | The https://github.com/react/yoga/issues/1067 repro goes from top 50 to 100. I added `tests/YGFlexWrapCrossAlignmentTest.cpp`: 9 hand written gtest cases like the other non generated tests, covering row and column, LTR and RTL, `wrap` and `wrap-reverse` (including one sided margins), baseline, static items and both issue repros. All 9 fail without the `CalculateLayout.cpp` change; with it, all 877 tests pass (868 plus 9). I can move them into gentest fixtures or add an `Errata` flag if you prefer. X-link: https://github.com/react/yoga/pull/2045 Differential Revision: D127132637 Pulled By: javache --- .../yoga/yoga/algorithm/CalculateLayout.cpp | 59 ++++++++++++------- 1 file changed, 38 insertions(+), 21 deletions(-) diff --git a/packages/react-native/ReactCommon/yoga/yoga/algorithm/CalculateLayout.cpp b/packages/react-native/ReactCommon/yoga/yoga/algorithm/CalculateLayout.cpp index 7b1d7461d745..88e5d1d38be9 100644 --- a/packages/react-native/ReactCommon/yoga/yoga/algorithm/CalculateLayout.cpp +++ b/packages/react-native/ReactCommon/yoga/yoga/algorithm/CalculateLayout.cpp @@ -2325,6 +2325,9 @@ static void calculateLayoutImpl( float leadPerLine = 0; float currentLead = leadingPaddingAndBorderCross; float extraSpacePerLine = 0; + // With wrap-reverse the cross positions set here are mirrored further + // below, so an item's flex end margin is the one that leads here. + const bool isWrapReverse = node->style().flexWrap() == Wrap::WrapReverse; const float unclampedCrossDim = sizingModeCrossDim == SizingMode::StretchFit ? availableInnerCrossDim + paddingAndBorderAxisCross @@ -2443,6 +2446,17 @@ static void calculateLayoutImpl( continue; } if (child->style().positionType() != PositionType::Absolute) { + const float flexStartMargin = child->style().computeFlexStartMargin( + crossAxis, direction, availableInnerWidth); + const float flexEndMargin = child->style().computeFlexEndMargin( + crossAxis, direction, availableInnerWidth); + const float leadingCrossMargin = + isWrapReverse ? flexEndMargin : flexStartMargin; + const float trailingCrossMargin = + isWrapReverse ? flexStartMargin : flexEndMargin; + const float relativePositionCross = child->relativePosition( + crossAxis, direction, availableInnerCrossDim); + switch (resolveChildAlignment(node, child)) { case Align::Start: case Align::End: @@ -2450,36 +2464,33 @@ static void calculateLayoutImpl( break; case Align::FlexStart: { child->setLayoutPosition( - currentLead + - child->style().computeFlexStartPosition( - crossAxis, direction, availableInnerWidth), + currentLead + leadingCrossMargin + relativePositionCross, flexStartEdge(crossAxis)); break; } case Align::FlexEnd: { child->setLayoutPosition( - currentLead + lineHeight - - child->style().computeFlexEndMargin( - crossAxis, direction, availableInnerWidth) - + currentLead + lineHeight - trailingCrossMargin - child->getLayout().measuredDimension( - dimension(crossAxis)), + dimension(crossAxis)) + + relativePositionCross, flexStartEdge(crossAxis)); break; } case Align::Center: { - float childHeight = - child->getLayout().measuredDimension(dimension(crossAxis)); + const float childOuterCrossDim = + child->getLayout().measuredDimension(dimension(crossAxis)) + + leadingCrossMargin + trailingCrossMargin; child->setLayoutPosition( - currentLead + (lineHeight - childHeight) / 2, + currentLead + (lineHeight - childOuterCrossDim) / 2 + + leadingCrossMargin + relativePositionCross, flexStartEdge(crossAxis)); break; } case Align::Stretch: { child->setLayoutPosition( - currentLead + - child->style().computeFlexStartMargin( - crossAxis, direction, availableInnerWidth), + currentLead + leadingCrossMargin + relativePositionCross, flexStartEdge(crossAxis)); // Remeasure child with the line height as it as been only @@ -2527,11 +2538,7 @@ static void calculateLayoutImpl( case Align::Baseline: { child->setLayoutPosition( currentLead + maxAscentForCurrentLine - - calculateBaseline(child) + - child->style().computeFlexStartPosition( - FlexDirection::Column, - direction, - availableInnerCrossDim), + calculateBaseline(child) + relativePositionCross, PhysicalEdge::Top); break; @@ -2643,14 +2650,24 @@ static void calculateLayoutImpl( } // As we only wrapped in normal direction yet, we need to reverse the - // positions on wrap-reverse. + // positions on wrap-reverse. A relative inset is a physical offset, so it is + // taken out before reversing and added back afterwards. if (performLayout && node->style().flexWrap() == Wrap::WrapReverse) { for (auto child : node->getLayoutChildren()) { if (child->style().positionType() != PositionType::Absolute) { + const float relativePositionCross = + child->style().display() == Display::None + ? 0.0f + : child->relativePosition( + crossAxis, direction, availableInnerCrossDim); + const float positionInFlow = + child->getLayout().position(flexStartEdge(crossAxis)) - + relativePositionCross; child->setLayoutPosition( node->getLayout().measuredDimension(dimension(crossAxis)) - - child->getLayout().position(flexStartEdge(crossAxis)) - - child->getLayout().measuredDimension(dimension(crossAxis)), + positionInFlow - + child->getLayout().measuredDimension(dimension(crossAxis)) + + relativePositionCross, flexStartEdge(crossAxis)); } }