Fix issue where start/end would not be respected in flex edge getters (…

…facebook#41682) Summary: X-link: facebook/yoga#1479 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
joevilches · Dec 4, 2023 · 65f019f · 65f019f
1 parent b71f396
commit 65f019f
Show file tree

Hide file tree

Showing 3 changed files with 117 additions and 53 deletions.
diff --git a/packages/react-native/ReactCommon/yoga/yoga/algorithm/CalculateLayout.cpp b/packages/react-native/ReactCommon/yoga/yoga/algorithm/CalculateLayout.cpp
@@ -352,40 +352,48 @@ 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))),
         flexStartEdge(axis));
   } 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/packages/react-native/ReactCommon/yoga/yoga/node/Node.cpp b/packages/react-native/ReactCommon/yoga/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/packages/react-native/ReactCommon/yoga/yoga/node/Node.h b/packages/react-native/ReactCommon/yoga/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,