Skip to content

Commit

Permalink
Fix issue where position insets were not working with row reverse (#4…
Browse files Browse the repository at this point in the history 
…1041)

Summary:
X-link: facebook/yoga#1431

Pull Request resolved: #41041

The last of the row-reverse issues hurray!

The position insets were broken with row-reverse since we were using the main-start/main-end edges to inset from and NOT the inline-start/inline-end edges as we should. This made it so that inset in left and right were swapped and same with top and bottom (with column-reverse). The solution here is the same as the previous ones were we are migrating to using inline-start/end as the leading/trailing edge now.

Reviewed By: NickGerleman

Differential Revision: D50390543

fbshipit-source-id: b714deab8489fbe11f7f6db21e4aad3b3aa314b3
  • Loading branch information
@joevilches @facebook-github-bot
joevilches authored and facebook-github-bot committed Oct 19, 2023
1 parent 8eb0617 commit 25575e5
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 54 deletions.
67 changes: 38 additions & 29 deletions packages/react-native/ReactCommon/yoga/yoga/algorithm/CalculateLayout.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -343,13 +343,13 @@ 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->isInlineStartPositionDefined(FlexDirection::Row, direction) &&
child->isInlineEndPositionDefined(FlexDirection::Row, direction)) {
childWidth = node->getLayout().measuredDimension(Dimension::Width) -
(node->getInlineStartBorder(FlexDirection::Row, direction) +
node->getInlineEndBorder(FlexDirection::Row, direction)) -
(child->getFlexStartPosition(FlexDirection::Row, width) +
child->getFlexEndPosition(FlexDirection::Row, width));
(child->getInlineStartPosition(FlexDirection::Row, direction, width) +
child->getInlineEndPosition(FlexDirection::Row, direction, width));
childWidth =
boundAxis(child, FlexDirection::Row, childWidth, width, width);
}
Expand All @@ -363,13 +363,15 @@ 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->isInlineStartPositionDefined(FlexDirection::Column, direction) &&
child->isInlineEndPositionDefined(FlexDirection::Column, direction)) {
childHeight = node->getLayout().measuredDimension(Dimension::Height) -
(node->getInlineStartBorder(FlexDirection::Column, direction) +
node->getInlineEndBorder(FlexDirection::Column, direction)) -
(child->getFlexStartPosition(FlexDirection::Column, height) +
child->getFlexEndPosition(FlexDirection::Column, height));
(child->getInlineStartPosition(
FlexDirection::Column, direction, height) +
child->getInlineEndPosition(
FlexDirection::Column, direction, height));
childHeight =
boundAxis(child, FlexDirection::Column, childHeight, height, width);
}
Expand Down Expand Up @@ -446,26 +448,27 @@ static void layoutAbsoluteChild(
depth,
generationCount);

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

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

} else if (
!child->isFlexStartPositionDefined(crossAxis) &&
!child->isInlineStartPositionDefined(crossAxis, direction) &&
resolveChildAlignment(node, child) == Align::Center) {
child->setLayoutPosition(
(node->getLayout().measuredDimension(dimension(crossAxis)) -
child->getLayout().measuredDimension(dimension(crossAxis))) /
2.0f,
flexStartEdge(crossAxis));
} else if (
!child->isFlexStartPositionDefined(crossAxis) &&
!child->isInlineStartPositionDefined(crossAxis, direction) &&
((resolveChildAlignment(node, child) == Align::FlexEnd) ^
(node->getStyle().flexWrap() == Wrap::WrapReverse))) {
child->setLayoutPosition(
Expand All @@ -518,10 +522,11 @@ static void layoutAbsoluteChild(
} else if (
node->getConfig()->isExperimentalFeatureEnabled(
ExperimentalFeature::AbsolutePercentageAgainstPaddingEdge) &&
child->isFlexStartPositionDefined(crossAxis)) {
child->isInlineStartPositionDefined(crossAxis, direction)) {
child->setLayoutPosition(
child->getFlexStartPosition(
child->getInlineStartPosition(
crossAxis,
direction,
node->getLayout().measuredDimension(dimension(crossAxis))) +
node->getInlineStartBorder(crossAxis, direction) +
child->getInlineStartMargin(
Expand Down Expand Up @@ -1305,13 +1310,14 @@ static void justifyMainAxis(
continue;
}
if (childStyle.positionType() == PositionType::Absolute &&
child->isFlexStartPositionDefined(mainAxis)) {
child->isInlineStartPositionDefined(mainAxis, direction)) {
if (performLayout) {
// In case the child is position absolute and has left/top being
// defined, we override the position to whatever the user said (and
// margin/border).
child->setLayoutPosition(
child->getFlexStartPosition(mainAxis, availableInnerMainDim) +
child->getInlineStartPosition(
mainAxis, direction, availableInnerMainDim) +
node->getInlineStartBorder(mainAxis, direction) +
child->getInlineStartMargin(
mainAxis, direction, availableInnerWidth),
Expand Down Expand Up @@ -1864,10 +1870,11 @@ static void calculateLayoutImpl(
// top/left/bottom/right set, override all the previously computed
// positions to set it correctly.
const bool isChildLeadingPosDefined =
child->isFlexStartPositionDefined(crossAxis);
child->isInlineStartPositionDefined(crossAxis, direction);
if (isChildLeadingPosDefined) {
child->setLayoutPosition(
child->getFlexStartPosition(crossAxis, availableInnerCrossDim) +
child->getInlineStartPosition(
crossAxis, direction, availableInnerCrossDim) +
node->getInlineStartBorder(crossAxis, direction) +
child->getInlineStartMargin(
crossAxis, direction, availableInnerWidth),
Expand Down Expand Up @@ -2195,8 +2202,10 @@ static void calculateLayoutImpl(
child->setLayoutPosition(
currentLead + maxAscentForCurrentLine -
calculateBaseline(child) +
child->getFlexStartPosition(
FlexDirection::Column, availableInnerCrossDim),
child->getInlineStartPosition(
FlexDirection::Column,
direction,
availableInnerCrossDim),
YGEdgeTop);

break;
Expand Down
55 changes: 35 additions & 20 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,36 +99,46 @@ YGEdge Node::getInlineEndEdgeUsingErrata(
: inlineEndEdge(flexDirection, direction);
}

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

return !leadingPosition.isUndefined();
}

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

return !trailingPosition.isUndefined();
}

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

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

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

return resolveValue(trailingPosition, axisSize).unwrapOrDefault(0.0f);
}
Expand Down Expand Up @@ -440,12 +450,15 @@ void Node::setLayoutDimension(float dimensionValue, Dimension dimension) {

// If both left and right are defined, then use left. Otherwise return +left or
// -right depending on which is defined.
float Node::relativePosition(FlexDirection axis, float axisSize) const {
if (isFlexStartPositionDefined(axis)) {
return getFlexStartPosition(axis, axisSize);
float Node::relativePosition(
FlexDirection axis,
Direction direction,
float axisSize) const {
if (isInlineStartPositionDefined(axis, direction)) {
return getInlineStartPosition(axis, direction, axisSize);
}

return -1 * getFlexEndPosition(axis, axisSize);
return -1 * getInlineEndPosition(axis, direction, axisSize);
}

void Node::setPosition(
Expand All @@ -465,8 +478,10 @@ void Node::setPosition(
// Here we should check for `PositionType::Static` and in this case zero inset
// properties (left, right, top, bottom, begin, end).
// https://www.w3.org/TR/css-position-3/#valdef-position-static
const float relativePositionMain = relativePosition(mainAxis, mainSize);
const float relativePositionCross = relativePosition(crossAxis, crossSize);
const float relativePositionMain =
relativePosition(mainAxis, directionRespectingRoot, mainSize);
const float relativePositionCross =
relativePosition(crossAxis, directionRespectingRoot, crossSize);

const YGEdge mainAxisLeadingEdge =
getInlineStartEdgeUsingErrata(mainAxis, direction);
Expand Down
21 changes: 16 additions & 5 deletions packages/react-native/ReactCommon/yoga/yoga/node/Node.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,10 @@ class YG_EXPORT Node : public ::YGNode {
std::array<YGValue, 2> resolvedDimensions_ = {
{YGValueUndefined, YGValueUndefined}};

float relativePosition(FlexDirection axis, const float axisSize) const;
float relativePosition(
FlexDirection axis,
Direction direction,
const float axisSize) const;

YGEdge getInlineStartEdgeUsingErrata(
FlexDirection flexDirection,
Expand Down Expand Up @@ -196,10 +199,18 @@ class YG_EXPORT Node : public ::YGNode {
YGEdge edge);

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

0 comments on commit 25575e5

Please sign in to comment.