From 2b08ba4eff24d1155c29aeb7b4dacb70aaa0a1c2 Mon Sep 17 00:00:00 2001 From: Joe Vilches Date: Tue, 28 Nov 2023 16:51:07 -0800 Subject: [PATCH] Fix issue where start/end would not be respected in flex edge getters (#1479) Summary: X-link: https://github.com/facebook/react-native/pull/41682 There are two ways to get the value of a style for a specific edge right now: 1) From the inline start/end edge which is determined via the writing direction (ltr or rtl), assuming you do not have errata on 2) From the flex start/end edge which is determined via the flex direction (row, row-reverse, column, column-reverse) There is a weird curiosity in the second case: you can define a style to be on the "start" or "end" edge when writing the stylex/css. The physical edge that this refers to is dependent on the writing direction. So `start` would be `left` in `ltr` and `right` in `rtl`, with `end` the opposite. It is **never** determined via the flex direction. Additionally, `start`/`end` takes precedence over the physical edge it corresponds to in the case both are defined. So, all of this means that to actually get the value of a style from the flex start/end edges, we need to account for the case that one of these relative edges was defined and would overwrite any physical edge. Since this mapping is solely determined by the writing direction, we need to pass that in to all the flex start/end getters and do that logic. This is done in `flexStartRelativeEdge`/`flexEndRelativeEdge` which was added earlier but for some reason only being used on border. Reviewed By: NickGerleman Differential Revision: D51293315 --- yoga/algorithm/CalculateLayout.cpp | 39 ++++++----- yoga/node/Node.cpp | 100 ++++++++++++++++++++--------- yoga/node/Node.h | 31 +++++++-- 3 files changed, 117 insertions(+), 53 deletions(-) diff --git a/yoga/algorithm/CalculateLayout.cpp b/yoga/algorithm/CalculateLayout.cpp index 2839826ff2..014cd0f776 100644 --- a/yoga/algorithm/CalculateLayout.cpp +++ b/yoga/algorithm/CalculateLayout.cpp @@ -352,25 +352,30 @@ static void positionAbsoluteChild( : ((resolveChildAlignment(parent, child) == Align::FlexEnd) ^ (parent->getStyle().flexWrap() == Wrap::WrapReverse)); - if (child->isFlexEndPositionDefined(axis) && - !child->isFlexStartPositionDefined(axis)) { + if (child->isFlexEndPositionDefined(axis, direction) && + !child->isFlexStartPositionDefined(axis, direction)) { child->setLayoutPosition( containingNode->getLayout().measuredDimension(dimension(axis)) - child->getLayout().measuredDimension(dimension(axis)) - containingNode->getFlexEndBorder(axis, direction) - child->getFlexEndMargin( axis, + direction, isAxisRow ? containingBlockWidth : containingBlockHeight) - child->getFlexEndPosition( - axis, isAxisRow ? containingBlockWidth : containingBlockHeight), + axis, + direction, + isAxisRow ? containingBlockWidth : containingBlockHeight), flexStartEdge(axis)); - } else if (!child->isFlexStartPositionDefined(axis) && shouldCenter) { + } else if ( + !child->isFlexStartPositionDefined(axis, direction) && shouldCenter) { child->setLayoutPosition( (parent->getLayout().measuredDimension(dimension(axis)) - child->getLayout().measuredDimension(dimension(axis))) / 2.0f, flexStartEdge(axis)); - } else if (!child->isFlexStartPositionDefined(axis) && shouldFlexEnd) { + } else if ( + !child->isFlexStartPositionDefined(axis, direction) && shouldFlexEnd) { child->setLayoutPosition( (parent->getLayout().measuredDimension(dimension(axis)) - child->getLayout().measuredDimension(dimension(axis))), @@ -378,14 +383,17 @@ static void positionAbsoluteChild( } else if ( parent->getConfig()->isExperimentalFeatureEnabled( ExperimentalFeature::AbsolutePercentageAgainstPaddingEdge) && - child->isFlexStartPositionDefined(axis)) { + child->isFlexStartPositionDefined(axis, direction)) { child->setLayoutPosition( child->getFlexStartPosition( axis, + direction, containingNode->getLayout().measuredDimension(dimension(axis))) + containingNode->getFlexStartBorder(axis, direction) + child->getFlexStartMargin( - axis, isAxisRow ? containingBlockWidth : containingBlockHeight), + axis, + direction, + isAxisRow ? containingBlockWidth : containingBlockHeight), flexStartEdge(axis)); } } @@ -425,15 +433,16 @@ static void layoutAbsoluteChild( } else { // If the child doesn't have a specified width, compute the width based on // the left/right offsets if they're defined. - if (child->isFlexStartPositionDefined(FlexDirection::Row) && - child->isFlexEndPositionDefined(FlexDirection::Row)) { + if (child->isFlexStartPositionDefined(FlexDirection::Row, direction) && + child->isFlexEndPositionDefined(FlexDirection::Row, direction)) { childWidth = containingNode->getLayout().measuredDimension(Dimension::Width) - (containingNode->getFlexStartBorder(FlexDirection::Row, direction) + containingNode->getFlexEndBorder(FlexDirection::Row, direction)) - (child->getFlexStartPosition( - FlexDirection::Row, containingBlockWidth) + - child->getFlexEndPosition(FlexDirection::Row, containingBlockWidth)); + FlexDirection::Row, direction, containingBlockWidth) + + child->getFlexEndPosition( + FlexDirection::Row, direction, containingBlockWidth)); childWidth = boundAxis( child, FlexDirection::Row, @@ -453,17 +462,17 @@ static void layoutAbsoluteChild( } else { // If the child doesn't have a specified height, compute the height based on // the top/bottom offsets if they're defined. - if (child->isFlexStartPositionDefined(FlexDirection::Column) && - child->isFlexEndPositionDefined(FlexDirection::Column)) { + if (child->isFlexStartPositionDefined(FlexDirection::Column, direction) && + child->isFlexEndPositionDefined(FlexDirection::Column, direction)) { childHeight = containingNode->getLayout().measuredDimension(Dimension::Height) - (containingNode->getFlexStartBorder( FlexDirection::Column, direction) + containingNode->getFlexEndBorder(FlexDirection::Column, direction)) - (child->getFlexStartPosition( - FlexDirection::Column, containingBlockHeight) + + FlexDirection::Column, direction, containingBlockHeight) + child->getFlexEndPosition( - FlexDirection::Column, containingBlockHeight)); + FlexDirection::Column, direction, containingBlockHeight)); childHeight = boundAxis( child, FlexDirection::Column, diff --git a/yoga/node/Node.cpp b/yoga/node/Node.cpp index 63639aafd0..c10fbfdfe7 100644 --- a/yoga/node/Node.cpp +++ b/yoga/node/Node.cpp @@ -98,11 +98,29 @@ Edge Node::getInlineEndEdgeUsingErrata( : inlineEndEdge(flexDirection, direction); } -bool Node::isFlexStartPositionDefined(FlexDirection axis) const { - const Edge startEdge = flexStartEdge(axis); +Edge Node::getFlexStartRelativeEdgeUsingErrata( + FlexDirection flexDirection, + Direction direction) const { + return hasErrata(Errata::StartingEndingEdgeFromFlexDirection) + ? Edge::Start + : flexStartRelativeEdge(flexDirection, direction); +} + +Edge Node::getFlexEndRelativeEdgeUsingErrata( + FlexDirection flexDirection, + Direction direction) const { + return hasErrata(Errata::StartingEndingEdgeFromFlexDirection) + ? Edge::End + : flexEndRelativeEdge(flexDirection, direction); +} + +bool Node::isFlexStartPositionDefined(FlexDirection axis, Direction direction) + const { auto leadingPosition = isRow(axis) - ? computeEdgeValueForRow<&Style::position>(Edge::Start, startEdge) - : computeEdgeValueForColumn<&Style::position>(startEdge); + ? computeEdgeValueForRow<&Style::position>( + getFlexStartRelativeEdgeUsingErrata(axis, direction), + flexStartEdge(axis)) + : computeEdgeValueForColumn<&Style::position>(flexStartEdge(axis)); return leadingPosition.isDefined(); } @@ -117,11 +135,13 @@ bool Node::isInlineStartPositionDefined(FlexDirection axis, Direction direction) return leadingPosition.isDefined(); } -bool Node::isFlexEndPositionDefined(FlexDirection axis) const { - const Edge endEdge = flexEndEdge(axis); +bool Node::isFlexEndPositionDefined(FlexDirection axis, Direction direction) + const { auto trailingPosition = isRow(axis) - ? computeEdgeValueForRow<&Style::position>(Edge::End, endEdge) - : computeEdgeValueForColumn<&Style::position>(endEdge); + ? computeEdgeValueForRow<&Style::position>( + getFlexEndRelativeEdgeUsingErrata(axis, direction), + flexEndEdge(axis)) + : computeEdgeValueForColumn<&Style::position>(flexEndEdge(axis)); return !trailingPosition.isUndefined(); } @@ -136,11 +156,15 @@ bool Node::isInlineEndPositionDefined(FlexDirection axis, Direction direction) return trailingPosition.isDefined(); } -float Node::getFlexStartPosition(FlexDirection axis, float axisSize) const { - const Edge startEdge = flexStartEdge(axis); +float Node::getFlexStartPosition( + FlexDirection axis, + Direction direction, + float axisSize) const { auto leadingPosition = isRow(axis) - ? computeEdgeValueForRow<&Style::position>(Edge::Start, startEdge) - : computeEdgeValueForColumn<&Style::position>(startEdge); + ? computeEdgeValueForRow<&Style::position>( + getFlexStartRelativeEdgeUsingErrata(axis, direction), + flexStartEdge(axis)) + : computeEdgeValueForColumn<&Style::position>(flexStartEdge(axis)); return resolveValue(leadingPosition, axisSize).unwrapOrDefault(0.0f); } @@ -157,11 +181,15 @@ float Node::getInlineStartPosition( return resolveValue(leadingPosition, axisSize).unwrapOrDefault(0.0f); } -float Node::getFlexEndPosition(FlexDirection axis, float axisSize) const { - const Edge endEdge = flexEndEdge(axis); +float Node::getFlexEndPosition( + FlexDirection axis, + Direction direction, + float axisSize) const { auto trailingPosition = isRow(axis) - ? computeEdgeValueForRow<&Style::position>(Edge::End, endEdge) - : computeEdgeValueForColumn<&Style::position>(endEdge); + ? computeEdgeValueForRow<&Style::position>( + getFlexEndRelativeEdgeUsingErrata(axis, direction), + flexEndEdge(axis)) + : computeEdgeValueForColumn<&Style::position>(flexEndEdge(axis)); return resolveValue(trailingPosition, axisSize).unwrapOrDefault(0.0f); } @@ -178,11 +206,15 @@ float Node::getInlineEndPosition( return resolveValue(trailingPosition, axisSize).unwrapOrDefault(0.0f); } -float Node::getFlexStartMargin(FlexDirection axis, float widthSize) const { - const Edge startEdge = flexStartEdge(axis); +float Node::getFlexStartMargin( + FlexDirection axis, + Direction direction, + float widthSize) const { auto leadingMargin = isRow(axis) - ? computeEdgeValueForRow<&Style::margin>(Edge::Start, startEdge) - : computeEdgeValueForColumn<&Style::margin>(startEdge); + ? computeEdgeValueForRow<&Style::margin>( + getFlexStartRelativeEdgeUsingErrata(axis, direction), + flexStartEdge(axis)) + : computeEdgeValueForColumn<&Style::margin>(flexStartEdge(axis)); return resolveValue(leadingMargin, widthSize).unwrapOrDefault(0.0f); } @@ -199,11 +231,15 @@ float Node::getInlineStartMargin( return resolveValue(leadingMargin, widthSize).unwrapOrDefault(0.0f); } -float Node::getFlexEndMargin(FlexDirection axis, float widthSize) const { - const Edge endEdge = flexEndEdge(axis); +float Node::getFlexEndMargin( + FlexDirection axis, + Direction direction, + float widthSize) const { auto trailingMargin = isRow(axis) - ? computeEdgeValueForRow<&Style::margin>(Edge::End, endEdge) - : computeEdgeValueForColumn<&Style::margin>(endEdge); + ? computeEdgeValueForRow<&Style::margin>( + getFlexEndRelativeEdgeUsingErrata(axis, direction), + flexEndEdge(axis)) + : computeEdgeValueForColumn<&Style::margin>(flexEndEdge(axis)); return resolveValue(trailingMargin, widthSize).unwrapOrDefault(0.0f); } @@ -231,10 +267,10 @@ float Node::getInlineStartBorder(FlexDirection axis, Direction direction) } float Node::getFlexStartBorder(FlexDirection axis, Direction direction) const { - const Edge leadRelativeFlexItemEdge = flexStartRelativeEdge(axis, direction); YGValue leadingBorder = isRow(axis) ? computeEdgeValueForRow<&Style::border>( - leadRelativeFlexItemEdge, flexStartEdge(axis)) + getFlexStartRelativeEdgeUsingErrata(axis, direction), + flexStartEdge(axis)) : computeEdgeValueForColumn<&Style::border>(flexStartEdge(axis)); return maxOrDefined(leadingBorder.value, 0.0f); @@ -250,10 +286,10 @@ float Node::getInlineEndBorder(FlexDirection axis, Direction direction) const { } float Node::getFlexEndBorder(FlexDirection axis, Direction direction) const { - const Edge trailRelativeFlexItemEdge = flexEndRelativeEdge(axis, direction); YGValue trailingBorder = isRow(axis) ? computeEdgeValueForRow<&Style::border>( - trailRelativeFlexItemEdge, flexEndEdge(axis)) + getFlexEndRelativeEdgeUsingErrata(axis, direction), + flexEndEdge(axis)) : computeEdgeValueForColumn<&Style::border>(flexEndEdge(axis)); return maxOrDefined(trailingBorder.value, 0.0f); @@ -275,10 +311,10 @@ float Node::getFlexStartPadding( FlexDirection axis, Direction direction, float widthSize) const { - const Edge leadRelativeFlexItemEdge = flexStartRelativeEdge(axis, direction); auto leadingPadding = isRow(axis) ? computeEdgeValueForRow<&Style::padding>( - leadRelativeFlexItemEdge, flexStartEdge(axis)) + getFlexStartRelativeEdgeUsingErrata(axis, direction), + flexStartEdge(axis)) : computeEdgeValueForColumn<&Style::padding>(flexStartEdge(axis)); return maxOrDefined(resolveValue(leadingPadding, widthSize).unwrap(), 0.0f); @@ -300,10 +336,10 @@ float Node::getFlexEndPadding( FlexDirection axis, Direction direction, float widthSize) const { - const Edge trailRelativeFlexItemEdge = flexEndRelativeEdge(axis, direction); auto trailingPadding = isRow(axis) ? computeEdgeValueForRow<&Style::padding>( - trailRelativeFlexItemEdge, flexEndEdge(axis)) + getFlexEndRelativeEdgeUsingErrata(axis, direction), + flexEndEdge(axis)) : computeEdgeValueForColumn<&Style::padding>(flexEndEdge(axis)); return maxOrDefined(resolveValue(trailingPadding, widthSize).unwrap(), 0.0f); diff --git a/yoga/node/Node.h b/yoga/node/Node.h index 0cb9fabc8e..81036affea 100644 --- a/yoga/node/Node.h +++ b/yoga/node/Node.h @@ -59,6 +59,12 @@ class YG_EXPORT Node : public ::YGNode { Edge getInlineEndEdgeUsingErrata( FlexDirection flexDirection, Direction direction) const; + Edge getFlexStartRelativeEdgeUsingErrata( + FlexDirection flexDirection, + Direction direction) const; + Edge getFlexEndRelativeEdgeUsingErrata( + FlexDirection flexDirection, + Direction direction) const; void useWebDefaults() { style_.setFlexDirection(FlexDirection::Row); @@ -196,28 +202,41 @@ class YG_EXPORT Node : public ::YGNode { } // Methods related to positions, margin, padding and border - bool isFlexStartPositionDefined(FlexDirection axis) const; + bool isFlexStartPositionDefined(FlexDirection axis, Direction direction) + const; bool isInlineStartPositionDefined(FlexDirection axis, Direction direction) const; - bool isFlexEndPositionDefined(FlexDirection axis) const; + bool isFlexEndPositionDefined(FlexDirection axis, Direction direction) const; bool isInlineEndPositionDefined(FlexDirection axis, Direction direction) const; - float getFlexStartPosition(FlexDirection axis, float axisSize) const; + float getFlexStartPosition( + FlexDirection axis, + Direction direction, + float axisSize) const; float getInlineStartPosition( FlexDirection axis, Direction direction, float axisSize) const; - float getFlexEndPosition(FlexDirection axis, float axisSize) const; + float getFlexEndPosition( + FlexDirection axis, + Direction direction, + float axisSize) const; float getInlineEndPosition( FlexDirection axis, Direction direction, float axisSize) const; - float getFlexStartMargin(FlexDirection axis, float widthSize) const; + float getFlexStartMargin( + FlexDirection axis, + Direction direction, + float widthSize) const; float getInlineStartMargin( FlexDirection axis, Direction direction, float widthSize) const; - float getFlexEndMargin(FlexDirection axis, float widthSize) const; + float getFlexEndMargin( + FlexDirection axis, + Direction direction, + float widthSize) const; float getInlineEndMargin( FlexDirection axis, Direction direction,