From 3f6207b18750bccabd611ea89bd23e4bbf4f5c00 Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Thu, 14 Sep 2023 04:48:23 -0700 Subject: [PATCH] Fixup hack for flex line size calculation (#1380) Summary: X-link: https://github.com/facebook/react-native/pull/39433 Back when rolling out flex gap, we encountered a bug where gap was added to the end of the main axis when a size was not specified. During flex line justification/sizing, we calculate the amount of space that should be in between children. We erroneously add this, even after the last child element. For `justify-content`, this space between children is derived from free space along the axis. The only time we have free space is if we had a dimension/dimension constraint already set on the parent. In this case, the extra space added to the end of the flex line is never noticed, becasue we bound `maxLineMainDim` to container dimension constraints at the end of layout, and the error doesn't effect how any children are positioned or sized. `betweenMainDim` may still be set for `gap` even if we don't have a sized parent, which makes the extra space propagated to `maxLineMainDim` effect parent size. Because we were in a code freeze, I opted to have us go with a solution just effecting flex gap, instead of the right one, in case there were any side effects. This cleans up the code to use the right calculation everywhere, and fixes a separate bug, where `endOfLineIndex` and `startOfLineIndex` may not be the last/first in the line if they are out of the layout flow (absolutely positioned, or display: none_ See the original conversation on https://github.com/facebook/yoga/pull/1188 Reviewed By: javache Differential Revision: D49260049 --- yoga/algorithm/CalculateLayout.cpp | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/yoga/algorithm/CalculateLayout.cpp b/yoga/algorithm/CalculateLayout.cpp index 4288363721..a8cb925f5f 100644 --- a/yoga/algorithm/CalculateLayout.cpp +++ b/yoga/algorithm/CalculateLayout.cpp @@ -1306,11 +1306,6 @@ static void YGJustifyMainAxis( const auto child = node->getChild(i); const Style& childStyle = child->getStyle(); const LayoutResults& childLayout = child->getLayout(); - const bool isLastChild = i == flexLine.endOfLineIndex - 1; - // remove the gap if it is the last element of the line - if (isLastChild) { - betweenMainDim -= gap; - } if (childStyle.display() == YGDisplayNone) { continue; } @@ -1344,6 +1339,10 @@ static void YGJustifyMainAxis( leadingEdge(mainAxis)); } + if (child != flexLine.itemsInFlow.back()) { + flexLine.layout.mainDim += betweenMainDim; + } + if (child->marginTrailingValue(mainAxis).unit == YGUnitAuto) { flexLine.layout.mainDim += flexLine.layout.remainingFreeSpace / static_cast(numberOfAutoMarginsOnCurrentLine); @@ -1354,14 +1353,14 @@ static void YGJustifyMainAxis( // If we skipped the flex step, then we can't rely on the measuredDims // because they weren't computed. This means we can't call // dimensionWithMargin. - flexLine.layout.mainDim += betweenMainDim + + flexLine.layout.mainDim += child->getMarginForAxis(mainAxis, availableInnerWidth).unwrap() + childLayout.computedFlexBasis.unwrap(); flexLine.layout.crossDim = availableInnerCrossDim; } else { // The main dimension is the sum of all the elements dimension plus // the spacing. - flexLine.layout.mainDim += betweenMainDim + + flexLine.layout.mainDim += dimensionWithMargin(child, mainAxis, availableInnerWidth); if (isNodeBaselineLayout) {