DaddyFrosty/yoga
1
0
Fork 0
Code Issues 96 Pull Requests 31 Actions Packages Projects Releases 33 Wiki Activity

Fix issue where marginStart and marginEnd were not working with rowReverse flex direction (#1420)

Browse Source 
Summary:
X-link: https://github.com/facebook/litho/pull/962

X-link: https://github.com/facebook/react-native/pull/40804

Pull Request resolved: https://github.com/facebook/yoga/pull/1420

This stack is ultimately aiming to solve https://github.com/facebook/yoga/issues/1208

**The problem**
Turns out that we do not even check direction when determining which edge is the leading (start) and trailing (end) edges. This is not how web does it as the start/end is based on the writing direction NOT the flex direction: https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_flexible_box_layout/Basic_concepts_of_flexbox#start_and_end_lines. While web does not have marginStart and marginEnd, they do have margin-inline-start/end which relies on the writing mode to determine the "start"/"end": https://developer.mozilla.org/en-US/docs/Web/CSS/margin-inline-start.

This means that if you do something like
```
export default function Playground(props: Props): React.Node {
  return (
    <View style={styles.container}>
      <View style={styles.item} />
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    marginEnd: 100,
    flexDirection: 'row-reverse',
    backgroundColor: 'red',
    display: 'flex',
    width: 100,
    height: 100,
  },
  item: {
    backgroundColor: 'blue',
    width: 10,
  },
});
```

You get  {F1116264350}
As you can see the margin gets applied to the left edge even thought the direction is ltr and it should be applied to the right edge.

**The solution**
I ended up fixing this by creating a new `leadingLayoutEdge` and `trailingLayoutEdge` function that take the flex direction as well as the direction. Based on the errata, the a few functions will use these new functions to determine which `YGEdge` is the starting/ending.

You might be wondering why I did not put this logic inside of `leadingEdge(flexDirection)` / `trailingEdge(flexDirection)` since other areas could potentially have the same bug like `getLeadingPadding`. These functions are a bit overloaded and there are cases where we actually want to use the flexDirection to get the edge in question. For example, many of the calls to `setLayoutPosition` in `CalculateLayout.cpp` call `leadingEdge()` / `trailingEdge()` to set the proper position for cases like row-reverse where items need to line up in a different direction.

Reviewed By: NickGerleman

Differential Revision: D50140503

fbshipit-source-id: 5b580c7570f6ae1e2d031971926ac4e8f52dd362
This commit is contained in:
Joe Vilches
2023-10-12 16:22:27 -07:00
committed by Facebook GitHub Bot
parent 50dc5ae2d1
commit f4337fbb07
4 changed files with 115 additions and 33 deletions
Download Patch File Download Diff File Expand all files Collapse all files

41
yoga/algorithm/CalculateLayout.cpp
View File

@@ -54,8 +54,7 @@ static inline float dimensionWithMargin(
const FlexDirection axis,
const float widthSize) {
return node->getLayout().measuredDimension(dimension(axis)) +
(node->getLeadingMargin(axis, widthSize) +
node->getTrailingMargin(axis, widthSize));
node->getMarginForAxis(axis, widthSize);
}
static inline bool styleDefinesDimension(
@@ -453,7 +452,8 @@ static void layoutAbsoluteChild(
node->getLayout().measuredDimension(dimension(mainAxis)) -
child->getLayout().measuredDimension(dimension(mainAxis)) -
node->getTrailingBorder(mainAxis) -
child->getTrailingMargin(mainAxis, isMainAxisRow ? width : height) -
child->getTrailingMargin(
mainAxis, direction, isMainAxisRow ? width : height) -
child->getTrailingPosition(
mainAxis, isMainAxisRow ? width : height),
leadingEdge(mainAxis));
@@ -483,6 +483,7 @@ static void layoutAbsoluteChild(
node->getLeadingBorder(mainAxis) +
child->getLeadingMargin(
mainAxis,
direction,
node->getLayout().measuredDimension(dimension(mainAxis))),
leadingEdge(mainAxis));
}
@@ -494,7 +495,7 @@ static void layoutAbsoluteChild(
child->getLayout().measuredDimension(dimension(crossAxis)) -
node->getTrailingBorder(crossAxis) -
child->getTrailingMargin(
crossAxis, isMainAxisRow ? height : width) -
crossAxis, direction, isMainAxisRow ? height : width) -
child->getTrailingPosition(
crossAxis, isMainAxisRow ? height : width),
leadingEdge(crossAxis));
@@ -526,6 +527,7 @@ static void layoutAbsoluteChild(
node->getLeadingBorder(crossAxis) +
child->getLeadingMargin(
crossAxis,
direction,
node->getLayout().measuredDimension(dimension(crossAxis))),
leadingEdge(crossAxis));
}
@@ -1186,6 +1188,7 @@ static void justifyMainAxis(
const size_t startOfLineIndex,
const FlexDirection mainAxis,
const FlexDirection crossAxis,
const Direction direction,
const MeasureMode measureModeMainDim,
const MeasureMode measureModeCrossDim,
const float mainAxisownerSize,
@@ -1303,7 +1306,8 @@ static void justifyMainAxis(
child->setLayoutPosition(
child->getLeadingPosition(mainAxis, availableInnerMainDim) +
node->getLeadingBorder(mainAxis) +
child->getLeadingMargin(mainAxis, availableInnerWidth),
child->getLeadingMargin(
mainAxis, direction, availableInnerWidth),
leadingEdge(mainAxis));
}
} else {
@@ -1352,7 +1356,7 @@ static void justifyMainAxis(
// calculated by adding maxAscent and maxDescent from the baseline.
const float ascent = calculateBaseline(child) +
child->getLeadingMargin(
FlexDirection::Column, availableInnerWidth);
FlexDirection::Column, direction, availableInnerWidth);
const float descent =
child->getLayout().measuredDimension(Dimension::Height) +
child->getMarginForAxis(
@@ -1498,16 +1502,16 @@ static void calculateLayoutImpl(
const YGEdge endEdge = direction == Direction::LTR ? YGEdgeRight : YGEdgeLeft;
const float marginRowLeading =
node->getLeadingMargin(flexRowDirection, ownerWidth);
node->getLeadingMargin(flexRowDirection, direction, ownerWidth);
node->setLayoutMargin(marginRowLeading, startEdge);
const float marginRowTrailing =
node->getTrailingMargin(flexRowDirection, ownerWidth);
node->getTrailingMargin(flexRowDirection, direction, ownerWidth);
node->setLayoutMargin(marginRowTrailing, endEdge);
const float marginColumnLeading =
node->getLeadingMargin(flexColumnDirection, ownerWidth);
node->getLeadingMargin(flexColumnDirection, direction, ownerWidth);
node->setLayoutMargin(marginColumnLeading, YGEdgeTop);
const float marginColumnTrailing =
node->getTrailingMargin(flexColumnDirection, ownerWidth);
node->getTrailingMargin(flexColumnDirection, direction, ownerWidth);
node->setLayoutMargin(marginColumnTrailing, YGEdgeBottom);
const float marginAxisRow = marginRowLeading + marginRowTrailing;
@@ -1793,6 +1797,7 @@ static void calculateLayoutImpl(
startOfLineIndex,
mainAxis,
crossAxis,
direction,
measureModeMainDim,
measureModeCrossDim,
mainAxisownerSize,
@@ -1849,7 +1854,8 @@ static void calculateLayoutImpl(
child->setLayoutPosition(
child->getLeadingPosition(crossAxis, availableInnerCrossDim) +
node->getLeadingBorder(crossAxis) +
child->getLeadingMargin(crossAxis, availableInnerWidth),
child->getLeadingMargin(
crossAxis, direction, availableInnerWidth),
leadingEdge(crossAxis));
}
// If leading position is not defined or calculations result in Nan,
@@ -1859,7 +1865,8 @@ static void calculateLayoutImpl(
child->getLayout().position[leadingEdge(crossAxis)])) {
child->setLayoutPosition(
node->getLeadingBorder(crossAxis) +
child->getLeadingMargin(crossAxis, availableInnerWidth),
child->getLeadingMargin(
crossAxis, direction, availableInnerWidth),
leadingEdge(crossAxis));
}
} else {
@@ -2053,7 +2060,7 @@ static void calculateLayoutImpl(
if (resolveChildAlignment(node, child) == Align::Baseline) {
const float ascent = calculateBaseline(child) +
child->getLeadingMargin(
FlexDirection::Column, availableInnerWidth);
FlexDirection::Column, direction, availableInnerWidth);
const float descent =
child->getLayout().measuredDimension(Dimension::Height) +
child->getMarginForAxis(
@@ -2083,7 +2090,8 @@ static void calculateLayoutImpl(
case Align::FlexStart: {
child->setLayoutPosition(
currentLead +
child->getLeadingMargin(crossAxis, availableInnerWidth),
child->getLeadingMargin(
crossAxis, direction, availableInnerWidth),
leadingEdge(crossAxis));
break;
}
@@ -2091,7 +2099,7 @@ static void calculateLayoutImpl(
child->setLayoutPosition(
currentLead + lineHeight -
child->getTrailingMargin(
crossAxis, availableInnerWidth) -
crossAxis, direction, availableInnerWidth) -
child->getLayout().measuredDimension(
dimension(crossAxis)),
leadingEdge(crossAxis));
@@ -2109,7 +2117,8 @@ static void calculateLayoutImpl(
case Align::Stretch: {
child->setLayoutPosition(
currentLead +
child->getLeadingMargin(crossAxis, availableInnerWidth),
child->getLeadingMargin(
crossAxis, direction, availableInnerWidth),
leadingEdge(crossAxis));
// Remeasure child with the line height as it as been only

20
yoga/algorithm/FlexDirection.h
View File

@@ -77,6 +77,26 @@ inline YGEdge trailingEdge(const FlexDirection flexDirection) {
fatalWithMessage("Invalid FlexDirection");
}
inline YGEdge leadingLayoutEdge(
const FlexDirection flexDirection,
const Direction direction) {
if (isRow(flexDirection)) {
return direction == Direction::RTL ? YGEdgeRight : YGEdgeLeft;
}
return YGEdgeTop;
}
inline YGEdge trailingLayoutEdge(
const FlexDirection flexDirection,
const Direction direction) {
if (isRow(flexDirection)) {
return direction == Direction::RTL ? YGEdgeLeft : YGEdgeRight;
}
return YGEdgeBottom;
}
inline Dimension dimension(const FlexDirection flexDirection) {
switch (flexDirection) {
case FlexDirection::Column: