Skip to content

Commit

Permalink
Fix issue where absolute children of row-reverse containers would ins…
Browse files Browse the repository at this point in the history 
…et on the wrong side (#41293)

Summary:
Pull Request resolved: #41293

X-link: facebook/yoga#1446

NickGerleman pointed out that my recent changes to fix the slew of row-reverse problems in Yoga actually ended up regressing some parts. Specifically, absolute children of row-reverse containers would have their insets set to the wrong side. So if you set left: 10 it would apply it to the right.

Turns out, in `layoutAbsoluteChild` there were cases where we were applying inlineStart/End values to the flexStart/End edge, which can never be right. So I changed the values to also be flexStart/End as the fix here.

Reviewed By: NickGerleman

Differential Revision: D50945475

fbshipit-source-id: 290de06dcc04e8e644a3a32c127af12fdabb2f75
  • Loading branch information
@joevilches @facebook-github-bot
joevilches authored and facebook-github-bot committed Nov 7, 2023
1 parent 3b13d3c commit 9847bca
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 30 deletions.
53 changes: 23 additions & 30 deletions packages/react-native/ReactCommon/yoga/yoga/algorithm/CalculateLayout.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -448,27 +448,25 @@ static void layoutAbsoluteChild(
depth,
generationCount);

if (child->isInlineEndPositionDefined(mainAxis, direction) &&
!child->isInlineStartPositionDefined(mainAxis, direction)) {
if (child->isFlexEndPositionDefined(mainAxis) &&
!child->isFlexStartPositionDefined(mainAxis)) {
child->setLayoutPosition(
node->getLayout().measuredDimension(dimension(mainAxis)) -
child->getLayout().measuredDimension(dimension(mainAxis)) -
node->getInlineEndBorder(mainAxis, direction) -
child->getInlineEndMargin(
mainAxis, direction, isMainAxisRow ? width : height) -
child->getInlineEndPosition(
mainAxis, direction, isMainAxisRow ? width : height),
node->getFlexEndBorder(mainAxis, direction) -
child->getFlexEndMargin(mainAxis, isMainAxisRow ? width : height) -
child->getFlexEndPosition(mainAxis, isMainAxisRow ? width : height),
flexStartEdge(mainAxis));
} else if (
!child->isInlineStartPositionDefined(mainAxis, direction) &&
!child->isFlexStartPositionDefined(mainAxis) &&
node->getStyle().justifyContent() == Justify::Center) {
child->setLayoutPosition(
(node->getLayout().measuredDimension(dimension(mainAxis)) -
child->getLayout().measuredDimension(dimension(mainAxis))) /
2.0f,
flexStartEdge(mainAxis));
} else if (
!child->isInlineStartPositionDefined(mainAxis, direction) &&
!child->isFlexStartPositionDefined(mainAxis) &&
node->getStyle().justifyContent() == Justify::FlexEnd) {
child->setLayoutPosition(
(node->getLayout().measuredDimension(dimension(mainAxis)) -
Expand All @@ -477,42 +475,39 @@ static void layoutAbsoluteChild(
} else if (
node->getConfig()->isExperimentalFeatureEnabled(
ExperimentalFeature::AbsolutePercentageAgainstPaddingEdge) &&
child->isInlineStartPositionDefined(mainAxis, direction)) {
child->isFlexStartPositionDefined(mainAxis)) {
child->setLayoutPosition(
child->getInlineStartPosition(
child->getFlexStartPosition(
mainAxis,
direction,
node->getLayout().measuredDimension(dimension(mainAxis))) +
node->getInlineStartBorder(mainAxis, direction) +
child->getInlineStartMargin(
node->getFlexStartBorder(mainAxis, direction) +
child->getFlexStartMargin(
mainAxis,
direction,
node->getLayout().measuredDimension(dimension(mainAxis))),
flexStartEdge(mainAxis));
}

if (child->isInlineEndPositionDefined(crossAxis, direction) &&
!child->isInlineStartPositionDefined(crossAxis, direction)) {
if (child->isFlexEndPositionDefined(crossAxis) &&
!child->isFlexStartPositionDefined(crossAxis)) {
child->setLayoutPosition(
node->getLayout().measuredDimension(dimension(crossAxis)) -
child->getLayout().measuredDimension(dimension(crossAxis)) -
node->getInlineEndBorder(crossAxis, direction) -
child->getInlineEndMargin(
crossAxis, direction, isMainAxisRow ? height : width) -
child->getInlineEndPosition(
crossAxis, direction, isMainAxisRow ? height : width),
node->getFlexEndBorder(crossAxis, direction) -
child->getFlexEndMargin(crossAxis, isMainAxisRow ? height : width) -
child->getFlexEndPosition(
crossAxis, isMainAxisRow ? height : width),
flexStartEdge(crossAxis));

} else if (
!child->isInlineStartPositionDefined(crossAxis, direction) &&
!child->isFlexStartPositionDefined(crossAxis) &&
resolveChildAlignment(node, child) == Align::Center) {
child->setLayoutPosition(
(node->getLayout().measuredDimension(dimension(crossAxis)) -
child->getLayout().measuredDimension(dimension(crossAxis))) /
2.0f,
flexStartEdge(crossAxis));
} else if (
!child->isInlineStartPositionDefined(crossAxis, direction) &&
!child->isFlexStartPositionDefined(crossAxis) &&
((resolveChildAlignment(node, child) == Align::FlexEnd) ^
(node->getStyle().flexWrap() == Wrap::WrapReverse))) {
child->setLayoutPosition(
Expand All @@ -522,16 +517,14 @@ static void layoutAbsoluteChild(
} else if (
node->getConfig()->isExperimentalFeatureEnabled(
ExperimentalFeature::AbsolutePercentageAgainstPaddingEdge) &&
child->isInlineStartPositionDefined(crossAxis, direction)) {
child->isFlexStartPositionDefined(crossAxis)) {
child->setLayoutPosition(
child->getInlineStartPosition(
child->getFlexStartPosition(
crossAxis,
direction,
node->getLayout().measuredDimension(dimension(crossAxis))) +
node->getInlineStartBorder(crossAxis, direction) +
child->getInlineStartMargin(
node->getFlexStartBorder(crossAxis, direction) +
child->getFlexStartMargin(
crossAxis,
direction,
node->getLayout().measuredDimension(dimension(crossAxis))),
flexStartEdge(crossAxis));
}
Expand Down
54 changes: 54 additions & 0 deletions packages/react-native/ReactCommon/yoga/yoga/node/Node.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,15 @@ YGEdge Node::getInlineEndEdgeUsingErrata(
: inlineEndEdge(flexDirection, direction);
}

bool Node::isFlexStartPositionDefined(FlexDirection axis) const {
const YGEdge startEdge = flexStartEdge(axis);
auto leadingPosition = isRow(axis)
? computeEdgeValueForRow(style_.position(), YGEdgeStart, startEdge)
: computeEdgeValueForColumn(style_.position(), startEdge);

return leadingPosition.isDefined();
}

bool Node::isInlineStartPositionDefined(FlexDirection axis, Direction direction)
const {
const YGEdge startEdge = getInlineStartEdgeUsingErrata(axis, direction);
Expand All @@ -109,6 +118,15 @@ bool Node::isInlineStartPositionDefined(FlexDirection axis, Direction direction)
return leadingPosition.isDefined();
}

bool Node::isFlexEndPositionDefined(FlexDirection axis) const {
const YGEdge endEdge = flexEndEdge(axis);
auto trailingPosition = isRow(axis)
? computeEdgeValueForRow(style_.position(), YGEdgeEnd, endEdge)
: computeEdgeValueForColumn(style_.position(), endEdge);

return !trailingPosition.isUndefined();
}

bool Node::isInlineEndPositionDefined(FlexDirection axis, Direction direction)
const {
const YGEdge endEdge = getInlineEndEdgeUsingErrata(axis, direction);
Expand All @@ -119,6 +137,15 @@ bool Node::isInlineEndPositionDefined(FlexDirection axis, Direction direction)
return trailingPosition.isDefined();
}

float Node::getFlexStartPosition(FlexDirection axis, float axisSize) const {
const YGEdge startEdge = flexStartEdge(axis);
auto leadingPosition = isRow(axis)
? computeEdgeValueForRow(style_.position(), YGEdgeStart, startEdge)
: computeEdgeValueForColumn(style_.position(), startEdge);

return resolveValue(leadingPosition, axisSize).unwrapOrDefault(0.0f);
}

float Node::getInlineStartPosition(
FlexDirection axis,
Direction direction,
Expand All @@ -131,6 +158,15 @@ float Node::getInlineStartPosition(
return resolveValue(leadingPosition, axisSize).unwrapOrDefault(0.0f);
}

float Node::getFlexEndPosition(FlexDirection axis, float axisSize) const {
const YGEdge endEdge = flexEndEdge(axis);
auto trailingPosition = isRow(axis)
? computeEdgeValueForRow(style_.position(), YGEdgeEnd, endEdge)
: computeEdgeValueForColumn(style_.position(), endEdge);

return resolveValue(trailingPosition, axisSize).unwrapOrDefault(0.0f);
}

float Node::getInlineEndPosition(
FlexDirection axis,
Direction direction,
Expand All @@ -143,6 +179,15 @@ float Node::getInlineEndPosition(
return resolveValue(trailingPosition, axisSize).unwrapOrDefault(0.0f);
}

float Node::getFlexStartMargin(FlexDirection axis, float widthSize) const {
const YGEdge startEdge = flexStartEdge(axis);
auto leadingMargin = isRow(axis)
? computeEdgeValueForRow(style_.margin(), YGEdgeStart, startEdge)
: computeEdgeValueForColumn(style_.margin(), startEdge);

return resolveValue(leadingMargin, widthSize).unwrapOrDefault(0.0f);
}

float Node::getInlineStartMargin(
FlexDirection axis,
Direction direction,
Expand All @@ -155,6 +200,15 @@ float Node::getInlineStartMargin(
return resolveValue(leadingMargin, widthSize).unwrapOrDefault(0.0f);
}

float Node::getFlexEndMargin(FlexDirection axis, float widthSize) const {
const YGEdge endEdge = flexEndEdge(axis);
auto trailingMargin = isRow(axis)
? computeEdgeValueForRow(style_.margin(), YGEdgeEnd, endEdge)
: computeEdgeValueForColumn(style_.margin(), endEdge);

return resolveValue(trailingMargin, widthSize).unwrapOrDefault(0.0f);
}

float Node::getInlineEndMargin(
FlexDirection axis,
Direction direction,
Expand Down
6 changes: 6 additions & 0 deletions packages/react-native/ReactCommon/yoga/yoga/node/Node.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,22 +199,28 @@ class YG_EXPORT Node : public ::YGNode {
YGEdge edge);

// Methods related to positions, margin, padding and border
bool isFlexStartPositionDefined(FlexDirection axis) const;
bool isInlineStartPositionDefined(FlexDirection axis, Direction direction)
const;
bool isFlexEndPositionDefined(FlexDirection axis) const;
bool isInlineEndPositionDefined(FlexDirection axis, Direction direction)
const;
float getFlexStartPosition(FlexDirection axis, float axisSize) const;
float getInlineStartPosition(
FlexDirection axis,
Direction direction,
float axisSize) const;
float getFlexEndPosition(FlexDirection axis, float axisSize) const;
float getInlineEndPosition(
FlexDirection axis,
Direction direction,
float axisSize) const;
float getFlexStartMargin(FlexDirection axis, float widthSize) const;
float getInlineStartMargin(
FlexDirection axis,
Direction direction,
float widthSize) const;
float getFlexEndMargin(FlexDirection axis, float widthSize) const;
float getInlineEndMargin(
FlexDirection axis,
Direction direction,
Expand Down

0 comments on commit 9847bca

Please sign in to comment.