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

Fix issue where absolute children of row-reverse containers would inset on the wrong side (#1446)

Browse Source 
Summary:
X-link: https://github.com/facebook/react-native/pull/41293

Pull Request resolved: https://github.com/facebook/yoga/pull/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
This commit is contained in:
Joe Vilches
2023-11-07 11:02:20 -08:00
committed by Facebook GitHub Bot
parent c09554056d
commit 283e3203f6
7 changed files with 6929 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files

192
gentest/fixtures/YGFlexDirectionTest.html
View File

@@ -208,3 +208,195 @@
<div style="width: 10px;"></div> <div style="width: 10px;"></div>
</div> </div>
</div> </div>
<div id="flex_direction_row_reverse_inner_pos_left" style="height: 100px; width: 100px;">
<div style="height: 100px; width: 100px; flex-direction: row-reverse; ">
<div style="width: 10px; height: 10px; position: absolute; left: 10px"></div>
<div style="width: 10px;"></div>
<div style="width: 10px;"></div>
</div>
</div>
<div id="flex_direction_row_reverse_inner_pos_right" style="height: 100px; width: 100px;">
<div style="height: 100px; width: 100px; flex-direction: row-reverse; ">
<div style="width: 10px; height: 10px; position: absolute; right: 10px"></div>
<div style="width: 10px;"></div>
<div style="width: 10px;"></div>
</div>
</div>
<div id="flex_direction_col_reverse_inner_pos_top" style="height: 100px; width: 100px;">
<div style="height: 100px; width: 100px; flex-direction: column-reverse; ">
<div style="width: 10px; height: 10px; position: absolute; top: 10px"></div>
<div style="width: 10px;"></div>
<div style="width: 10px;"></div>
</div>
</div>
<div id="flex_direction_col_reverse_inner_pos_bottom" style="height: 100px; width: 100px;">
<div style="height: 100px; width: 100px; flex-direction: column-reverse; ">
<div style="width: 10px; height: 10px; position: absolute; bottom: 10px"></div>
<div style="width: 10px;"></div>
<div style="width: 10px;"></div>
</div>
</div>
<div id="flex_direction_row_reverse_inner_pos_start" data-disabled="true" style="height: 100px; width: 100px;">
<div style="height: 100px; width: 100px; flex-direction: row-reverse; ">
<div style="width: 10px; height: 10px; position: absolute; start: 10px"></div>
<div style="width: 10px;"></div>
<div style="width: 10px;"></div>
</div>
</div>
<div id="flex_direction_row_reverse_inner_pos_end" data-disabled="true" style="height: 100px; width: 100px;">
<div style="height: 100px; width: 100px; flex-direction: row-reverse; ">
<div style="width: 10px; height: 10px; position: absolute; end: 10px"></div>
<div style="width: 10px;"></div>
<div style="width: 10px;"></div>
</div>
</div>
<div id="flex_direction_row_reverse_inner_margin_left" style="height: 100px; width: 100px;">
<div style="height: 100px; width: 100px; flex-direction: row-reverse; ">
<div style="width: 10px; height: 10px; position: absolute; margin-left: 10px"></div>
<div style="width: 10px;"></div>
<div style="width: 10px;"></div>
</div>
</div>
<div id="flex_direction_row_reverse_inner_margin_right" style="height: 100px; width: 100px;">
<div style="height: 100px; width: 100px; flex-direction: row-reverse; ">
<div style="width: 10px; height: 10px; position: absolute; margin-right: 10px"></div>
<div style="width: 10px;"></div>
<div style="width: 10px;"></div>
</div>
</div>
<div id="flex_direction_col_reverse_inner_margin_top" style="height: 100px; width: 100px;">
<div style="height: 100px; width: 100px; flex-direction: column-reverse; ">
<div style="width: 10px; height: 10px; position: absolute; margin-top: 10px"></div>
<div style="width: 10px;"></div>
<div style="width: 10px;"></div>
</div>
</div>
<div id="flex_direction_col_reverse_inner_margin_bottom" style="height: 100px; width: 100px;">
<div style="height: 100px; width: 100px; flex-direction: column-reverse; ">
<div style="width: 10px; height: 10px; position: absolute; margin-bottom: 10px"></div>
<div style="width: 10px;"></div>
<div style="width: 10px;"></div>
</div>
</div>
<div id="flex_direction_row_reverse_inner_marign_start" style="height: 100px; width: 100px;">
<div style="height: 100px; width: 100px; flex-direction: row-reverse; ">
<div style="width: 10px; height: 10px; position: absolute; margin-start: 10px"></div>
<div style="width: 10px;"></div>
<div style="width: 10px;"></div>
</div>
</div>
<div id="flex_direction_row_reverse_inner_margin_end" style="height: 100px; width: 100px;">
<div style="height: 100px; width: 100px; flex-direction: row-reverse; ">
<div style="width: 10px; height: 10px; position: absolute; margin-end: 10px"></div>
<div style="width: 10px;"></div>
<div style="width: 10px;"></div>
</div>
</div>
<div id="flex_direction_row_reverse_inner_border_left" style="height: 100px; width: 100px;">
<div style="height: 100px; width: 100px; flex-direction: row-reverse; ">
<div style="width: 10px; height: 10px; position: absolute; border-left: 10px"></div>
<div style="width: 10px;"></div>
<div style="width: 10px;"></div>
</div>
</div>
<div id="flex_direction_row_reverse_inner_border_right" style="height: 100px; width: 100px;">
<div style="height: 100px; width: 100px; flex-direction: row-reverse; ">
<div style="width: 10px; height: 10px; position: absolute; border-right: 10px"></div>
<div style="width: 10px;"></div>
<div style="width: 10px;"></div>
</div>
</div>
<div id="flex_direction_col_reverse_inner_border_top" style="height: 100px; width: 100px;">
<div style="height: 100px; width: 100px; flex-direction: column-reverse; ">
<div style="width: 10px; height: 10px; position: absolute; border-top: 10px"></div>
<div style="width: 10px;"></div>
<div style="width: 10px;"></div>
</div>
</div>
<div id="flex_direction_col_reverse_inner_border_bottom" style="height: 100px; width: 100px;">
<div style="height: 100px; width: 100px; flex-direction: column-reverse; ">
<div style="width: 10px; height: 10px; position: absolute; border-bottom: 10px"></div>
<div style="width: 10px;"></div>
<div style="width: 10px;"></div>
</div>
</div>
<div id="flex_direction_row_reverse_inner_border_start" style="height: 100px; width: 100px;">
<div style="height: 100px; width: 100px; flex-direction: row-reverse; ">
<div style="width: 10px; height: 10px; position: absolute; border-start: 10px"></div>
<div style="width: 10px;"></div>
<div style="width: 10px;"></div>
</div>
</div>
<div id="flex_direction_row_reverse_inner_border_end" style="height: 100px; width: 100px;">
<div style="height: 100px; width: 100px; flex-direction: row-reverse; ">
<div style="width: 10px; height: 10px; position: absolute; border-end: 10px"></div>
<div style="width: 10px;"></div>
<div style="width: 10px;"></div>
</div>
</div>
<div id="flex_direction_row_reverse_inner_padding_left" style="height: 100px; width: 100px;">
<div style="height: 100px; width: 100px; flex-direction: row-reverse; ">
<div style="width: 10px; height: 10px; position: absolute; padding-left: 10px"></div>
<div style="width: 10px;"></div>
<div style="width: 10px;"></div>
</div>
</div>
<div id="flex_direction_row_reverse_inner_padding_right" style="height: 100px; width: 100px;">
<div style="height: 100px; width: 100px; flex-direction: row-reverse; ">
<div style="width: 10px; height: 10px; position: absolute; padding-right: 10px"></div>
<div style="width: 10px;"></div>
<div style="width: 10px;"></div>
</div>
</div>
<div id="flex_direction_col_reverse_inner_padding_top" style="height: 100px; width: 100px;">
<div style="height: 100px; width: 100px; flex-direction: column-reverse; ">
<div style="width: 10px; height: 10px; position: absolute; padding-top: 10px"></div>
<div style="width: 10px;"></div>
<div style="width: 10px;"></div>
</div>
</div>
<div id="flex_direction_col_reverse_inner_padding_bottom" style="height: 100px; width: 100px;">
<div style="height: 100px; width: 100px; flex-direction: column-reverse; ">
<div style="width: 10px; height: 10px; position: absolute; padding-bottom: 10px"></div>
<div style="width: 10px;"></div>
<div style="width: 10px;"></div>
</div>
</div>
<div id="flex_direction_row_reverse_inner_padding_start" style="height: 100px; width: 100px;">
<div style="height: 100px; width: 100px; flex-direction: row-reverse; ">
<div style="width: 10px; height: 10px; position: absolute; padding-start: 10px"></div>
<div style="width: 10px;"></div>
<div style="width: 10px;"></div>
</div>
</div>
<div id="flex_direction_row_reverse_inner_padding_end" style="height: 100px; width: 100px;">
<div style="height: 100px; width: 100px; flex-direction: row-reverse; ">
<div style="width: 10px; height: 10px; position: absolute; padding-end: 10px"></div>
<div style="width: 10px;"></div>
<div style="width: 10px;"></div>
</div>
</div>

2162
java/tests/com/facebook/yoga/YGFlexDirectionTest.java
View File

File diff suppressed because it is too large Load Diff

2304
javascript/tests/generated/YGFlexDirectionTest.test.ts
View File

File diff suppressed because it is too large Load Diff

2188
tests/generated/YGFlexDirectionTest.cpp
View File

File diff suppressed because it is too large Load Diff

53
yoga/algorithm/CalculateLayout.cpp
View File

@@ -448,19 +448,17 @@ static void layoutAbsoluteChild(
depth, depth,
generationCount); generationCount);
if (child->isInlineEndPositionDefined(mainAxis, direction) && if (child->isFlexEndPositionDefined(mainAxis) &&
!child->isInlineStartPositionDefined(mainAxis, direction)) { !child->isFlexStartPositionDefined(mainAxis)) {
child->setLayoutPosition( child->setLayoutPosition(
node->getLayout().measuredDimension(dimension(mainAxis)) - node->getLayout().measuredDimension(dimension(mainAxis)) -
child->getLayout().measuredDimension(dimension(mainAxis)) - child->getLayout().measuredDimension(dimension(mainAxis)) -
node->getInlineEndBorder(mainAxis, direction) - node->getFlexEndBorder(mainAxis, direction) -
child->getInlineEndMargin( child->getFlexEndMargin(mainAxis, isMainAxisRow ? width : height) -
mainAxis, direction, isMainAxisRow ? width : height) - child->getFlexEndPosition(mainAxis, isMainAxisRow ? width : height),
child->getInlineEndPosition(
mainAxis, direction, isMainAxisRow ? width : height),
flexStartEdge(mainAxis)); flexStartEdge(mainAxis));
} else if ( } else if (
!child->isInlineStartPositionDefined(mainAxis, direction) && !child->isFlexStartPositionDefined(mainAxis) &&
node->getStyle().justifyContent() == Justify::Center) { node->getStyle().justifyContent() == Justify::Center) {
child->setLayoutPosition( child->setLayoutPosition(
(node->getLayout().measuredDimension(dimension(mainAxis)) - (node->getLayout().measuredDimension(dimension(mainAxis)) -
@@ -468,7 +466,7 @@ static void layoutAbsoluteChild(
2.0f, 2.0f,
flexStartEdge(mainAxis)); flexStartEdge(mainAxis));
} else if ( } else if (
!child->isInlineStartPositionDefined(mainAxis, direction) && !child->isFlexStartPositionDefined(mainAxis) &&
node->getStyle().justifyContent() == Justify::FlexEnd) { node->getStyle().justifyContent() == Justify::FlexEnd) {
child->setLayoutPosition( child->setLayoutPosition(
(node->getLayout().measuredDimension(dimension(mainAxis)) - (node->getLayout().measuredDimension(dimension(mainAxis)) -
@@ -477,34 +475,31 @@ static void layoutAbsoluteChild(
} else if ( } else if (
node->getConfig()->isExperimentalFeatureEnabled( node->getConfig()->isExperimentalFeatureEnabled(
ExperimentalFeature::AbsolutePercentageAgainstPaddingEdge) && ExperimentalFeature::AbsolutePercentageAgainstPaddingEdge) &&
child->isInlineStartPositionDefined(mainAxis, direction)) { child->isFlexStartPositionDefined(mainAxis)) {
child->setLayoutPosition( child->setLayoutPosition(
child->getInlineStartPosition( child->getFlexStartPosition(
mainAxis, mainAxis,
direction,
node->getLayout().measuredDimension(dimension(mainAxis))) + node->getLayout().measuredDimension(dimension(mainAxis))) +
node->getInlineStartBorder(mainAxis, direction) + node->getFlexStartBorder(mainAxis, direction) +
child->getInlineStartMargin( child->getFlexStartMargin(
mainAxis, mainAxis,
direction,
node->getLayout().measuredDimension(dimension(mainAxis))), node->getLayout().measuredDimension(dimension(mainAxis))),
flexStartEdge(mainAxis)); flexStartEdge(mainAxis));
} }
if (child->isInlineEndPositionDefined(crossAxis, direction) && if (child->isFlexEndPositionDefined(crossAxis) &&
!child->isInlineStartPositionDefined(crossAxis, direction)) { !child->isFlexStartPositionDefined(crossAxis)) {
child->setLayoutPosition( child->setLayoutPosition(
node->getLayout().measuredDimension(dimension(crossAxis)) - node->getLayout().measuredDimension(dimension(crossAxis)) -
child->getLayout().measuredDimension(dimension(crossAxis)) - child->getLayout().measuredDimension(dimension(crossAxis)) -
node->getInlineEndBorder(crossAxis, direction) - node->getFlexEndBorder(crossAxis, direction) -
child->getInlineEndMargin( child->getFlexEndMargin(crossAxis, isMainAxisRow ? height : width) -
crossAxis, direction, isMainAxisRow ? height : width) - child->getFlexEndPosition(
child->getInlineEndPosition( crossAxis, isMainAxisRow ? height : width),
crossAxis, direction, isMainAxisRow ? height : width),
flexStartEdge(crossAxis)); flexStartEdge(crossAxis));
} else if ( } else if (
!child->isInlineStartPositionDefined(crossAxis, direction) && !child->isFlexStartPositionDefined(crossAxis) &&
resolveChildAlignment(node, child) == Align::Center) { resolveChildAlignment(node, child) == Align::Center) {
child->setLayoutPosition( child->setLayoutPosition(
(node->getLayout().measuredDimension(dimension(crossAxis)) - (node->getLayout().measuredDimension(dimension(crossAxis)) -
@@ -512,7 +507,7 @@ static void layoutAbsoluteChild(
2.0f, 2.0f,
flexStartEdge(crossAxis)); flexStartEdge(crossAxis));
} else if ( } else if (
!child->isInlineStartPositionDefined(crossAxis, direction) && !child->isFlexStartPositionDefined(crossAxis) &&
((resolveChildAlignment(node, child) == Align::FlexEnd) ^ ((resolveChildAlignment(node, child) == Align::FlexEnd) ^
(node->getStyle().flexWrap() == Wrap::WrapReverse))) { (node->getStyle().flexWrap() == Wrap::WrapReverse))) {
child->setLayoutPosition( child->setLayoutPosition(
@@ -522,16 +517,14 @@ static void layoutAbsoluteChild(
} else if ( } else if (
node->getConfig()->isExperimentalFeatureEnabled( node->getConfig()->isExperimentalFeatureEnabled(
ExperimentalFeature::AbsolutePercentageAgainstPaddingEdge) && ExperimentalFeature::AbsolutePercentageAgainstPaddingEdge) &&
child->isInlineStartPositionDefined(crossAxis, direction)) { child->isFlexStartPositionDefined(crossAxis)) {
child->setLayoutPosition( child->setLayoutPosition(
child->getInlineStartPosition( child->getFlexStartPosition(
crossAxis, crossAxis,
direction,
node->getLayout().measuredDimension(dimension(crossAxis))) + node->getLayout().measuredDimension(dimension(crossAxis))) +
node->getInlineStartBorder(crossAxis, direction) + node->getFlexStartBorder(crossAxis, direction) +
child->getInlineStartMargin( child->getFlexStartMargin(
crossAxis, crossAxis,
direction,
node->getLayout().measuredDimension(dimension(crossAxis))), node->getLayout().measuredDimension(dimension(crossAxis))),
flexStartEdge(crossAxis)); flexStartEdge(crossAxis));
} }

54
yoga/node/Node.cpp
View File

@@ -99,6 +99,15 @@ YGEdge Node::getInlineEndEdgeUsingErrata(
: inlineEndEdge(flexDirection, direction); : 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) bool Node::isInlineStartPositionDefined(FlexDirection axis, Direction direction)
const { const {
const YGEdge startEdge = getInlineStartEdgeUsingErrata(axis, direction); const YGEdge startEdge = getInlineStartEdgeUsingErrata(axis, direction);
@@ -109,6 +118,15 @@ bool Node::isInlineStartPositionDefined(FlexDirection axis, Direction direction)
return leadingPosition.isDefined(); 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) bool Node::isInlineEndPositionDefined(FlexDirection axis, Direction direction)
const { const {
const YGEdge endEdge = getInlineEndEdgeUsingErrata(axis, direction); const YGEdge endEdge = getInlineEndEdgeUsingErrata(axis, direction);
@@ -119,6 +137,15 @@ bool Node::isInlineEndPositionDefined(FlexDirection axis, Direction direction)
return trailingPosition.isDefined(); 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( float Node::getInlineStartPosition(
FlexDirection axis, FlexDirection axis,
Direction direction, Direction direction,
@@ -131,6 +158,15 @@ float Node::getInlineStartPosition(
return resolveValue(leadingPosition, axisSize).unwrapOrDefault(0.0f); 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( float Node::getInlineEndPosition(
FlexDirection axis, FlexDirection axis,
Direction direction, Direction direction,
@@ -143,6 +179,15 @@ float Node::getInlineEndPosition(
return resolveValue(trailingPosition, axisSize).unwrapOrDefault(0.0f); 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( float Node::getInlineStartMargin(
FlexDirection axis, FlexDirection axis,
Direction direction, Direction direction,
@@ -155,6 +200,15 @@ float Node::getInlineStartMargin(
return resolveValue(leadingMargin, widthSize).unwrapOrDefault(0.0f); 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( float Node::getInlineEndMargin(
FlexDirection axis, FlexDirection axis,
Direction direction, Direction direction,

6
yoga/node/Node.h
View File

@@ -199,22 +199,28 @@ class YG_EXPORT Node : public ::YGNode {
YGEdge edge); YGEdge edge);
// Methods related to positions, margin, padding and border // Methods related to positions, margin, padding and border
bool isFlexStartPositionDefined(FlexDirection axis) const;
bool isInlineStartPositionDefined(FlexDirection axis, Direction direction) bool isInlineStartPositionDefined(FlexDirection axis, Direction direction)
const; const;
bool isFlexEndPositionDefined(FlexDirection axis) const;
bool isInlineEndPositionDefined(FlexDirection axis, Direction direction) bool isInlineEndPositionDefined(FlexDirection axis, Direction direction)
const; const;
float getFlexStartPosition(FlexDirection axis, float axisSize) const;
float getInlineStartPosition( float getInlineStartPosition(
FlexDirection axis, FlexDirection axis,
Direction direction, Direction direction,
float axisSize) const; float axisSize) const;
float getFlexEndPosition(FlexDirection axis, float axisSize) const;
float getInlineEndPosition( float getInlineEndPosition(
FlexDirection axis, FlexDirection axis,
Direction direction, Direction direction,
float axisSize) const; float axisSize) const;
float getFlexStartMargin(FlexDirection axis, float widthSize) const;
float getInlineStartMargin( float getInlineStartMargin(
FlexDirection axis, FlexDirection axis,
Direction direction, Direction direction,
float widthSize) const; float widthSize) const;
float getFlexEndMargin(FlexDirection axis, float widthSize) const;
float getInlineEndMargin( float getInlineEndMargin(
FlexDirection axis, FlexDirection axis,
Direction direction, Direction direction,