Skip to content

Commit

Permalink
Fix issue where start/end would not be respected in flex edge getters
Browse files Browse the repository at this point in the history 
Summary:
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
  • Loading branch information
@joevilches @facebook-github-bot
joevilches authored and facebook-github-bot committed Nov 29, 2023
1 parent 4ca7688 commit faa24e1
Show file tree
Hide file tree
Showing 3 changed files with 117 additions and 53 deletions.
39 changes: 24 additions & 15 deletions yoga/algorithm/CalculateLayout.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down
100 changes: 68 additions & 32 deletions yoga/node/Node.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand All @@ -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();
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down
31 changes: 25 additions & 6 deletions yoga/node/Node.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit faa24e1

Please sign in to comment.