Skip to content

Commit

Permalink
Remove legacy absolute positioning path
Browse files Browse the repository at this point in the history
Summary:
X-link: facebook/react-native#46984

X-link: facebook/yoga#1725

The legacy (wrong) absolute positioning path positions in two places, including work that is definitely always overwritten in the new absolute layout path.

This came up before for position: static, but we didn't clean this up at the time. This code is also now leading display: contents impl being more annoying.

This diff tries to converge to the more spec correct implementation of positioning here, that also only happens in one place.

Previous path would potentially also incorrectly justify when `justify-content` was non-default, but not handled in the previous few cases? We don't have access to the flexLine at this point later, and apart from the existing tests now passing I reused the new correct logic for justification (spec says we should position child as if its the only child in the container https://www.w3.org/TR/css-flexbox-1/#abspos-items).

I added a new, more scoped errata `AbsolutePositionWithoutInsetsExcludesPadding` to preserve some of the legacy behavior that showed as very breaking.

I also did not try removing `AbsolutePercentAgainstInnerSize` which I suspect would be more breaking than this change.

Changelog:
[General][Breaking] - More spec compliant absolute positioning

Reviewed By: joevilches

Differential Revision: D64244949

fbshipit-source-id: ca97570e0de82e8f0424a0912adfd0b05254559e
  • Loading branch information
NickGerleman authored and facebook-github-bot committed Oct 18, 2024
1 parent c3a8dab commit 37bf399
Show file tree
Hide file tree
Showing 6 changed files with 207 additions and 351 deletions.
4 changes: 2 additions & 2 deletions lib/yoga/src/main/cpp/yoga/YGEnums.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,8 @@ const char* YGErrataToString(const YGErrata value) {
return "none";
case YGErrataStretchFlexBasis:
return "stretch-flex-basis";
case YGErrataAbsolutePositioningIncorrect:
return "absolute-positioning-incorrect";
case YGErrataAbsolutePositionWithoutInsetsExcludesPadding:
return "absolute-position-without-insets-excludes-padding";
case YGErrataAbsolutePercentAgainstInnerSize:
return "absolute-percent-against-inner-size";
case YGErrataAll:
Expand Down
2 changes: 1 addition & 1 deletion lib/yoga/src/main/cpp/yoga/YGEnums.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ YG_ENUM_DECL(
YGErrata,
YGErrataNone = 0,
YGErrataStretchFlexBasis = 1,
YGErrataAbsolutePositioningIncorrect = 2,
YGErrataAbsolutePositionWithoutInsetsExcludesPadding = 2,
YGErrataAbsolutePercentAgainstInnerSize = 4,
YGErrataAll = 2147483647,
YGErrataClassic = 2147483646)
Expand Down
150 changes: 38 additions & 112 deletions lib/yoga/src/main/cpp/yoga/algorithm/AbsoluteLayout.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,15 @@ static inline void setFlexStartLayoutPosition(
const Direction direction,
const FlexDirection axis,
const float containingBlockWidth) {
child->setLayoutPosition(
child->style().computeFlexStartMargin(
axis, direction, containingBlockWidth) +
parent->getLayout().border(flexStartEdge(axis)) +
parent->getLayout().padding(flexStartEdge(axis)),
flexStartEdge(axis));
float position = child->style().computeFlexStartMargin(
axis, direction, containingBlockWidth) +
parent->getLayout().border(flexStartEdge(axis));

if (!child->hasErrata(Errata::AbsolutePositionWithoutInsetsExcludesPadding)) {
position += parent->getLayout().padding(flexStartEdge(axis));
}

child->setLayoutPosition(position, flexStartEdge(axis));
}

static inline void setFlexEndLayoutPosition(
Expand All @@ -33,15 +36,16 @@ static inline void setFlexEndLayoutPosition(
const Direction direction,
const FlexDirection axis,
const float containingBlockWidth) {
float flexEndPosition = parent->getLayout().border(flexEndEdge(axis)) +
child->style().computeFlexEndMargin(
axis, direction, containingBlockWidth);

if (!child->hasErrata(Errata::AbsolutePositionWithoutInsetsExcludesPadding)) {
flexEndPosition += parent->getLayout().padding(flexEndEdge(axis));
}

child->setLayoutPosition(
getPositionOfOppositeEdge(
parent->getLayout().border(flexEndEdge(axis)) +
parent->getLayout().padding(flexEndEdge(axis)) +
child->style().computeFlexEndMargin(
axis, direction, containingBlockWidth),
axis,
parent,
child),
getPositionOfOppositeEdge(flexEndPosition, axis, parent, child),
flexStartEdge(axis));
}

Expand All @@ -51,22 +55,30 @@ static inline void setCenterLayoutPosition(
const Direction direction,
const FlexDirection axis,
const float containingBlockWidth) {
const float parentContentBoxSize =
float parentContentBoxSize =
parent->getLayout().measuredDimension(dimension(axis)) -
parent->getLayout().border(flexStartEdge(axis)) -
parent->getLayout().border(flexEndEdge(axis)) -
parent->getLayout().padding(flexStartEdge(axis)) -
parent->getLayout().padding(flexEndEdge(axis));
parent->getLayout().border(flexEndEdge(axis));

if (!child->hasErrata(Errata::AbsolutePositionWithoutInsetsExcludesPadding)) {
parentContentBoxSize -= parent->getLayout().padding(flexStartEdge(axis));
parentContentBoxSize -= parent->getLayout().padding(flexEndEdge(axis));
}

const float childOuterSize =
child->getLayout().measuredDimension(dimension(axis)) +
child->style().computeMarginForAxis(axis, containingBlockWidth);
child->setLayoutPosition(
(parentContentBoxSize - childOuterSize) / 2.0f +
parent->getLayout().border(flexStartEdge(axis)) +
parent->getLayout().padding(flexStartEdge(axis)) +
child->style().computeFlexStartMargin(
axis, direction, containingBlockWidth),
flexStartEdge(axis));

float position = (parentContentBoxSize - childOuterSize) / 2.0f +
parent->getLayout().border(flexStartEdge(axis)) +
child->style().computeFlexStartMargin(
axis, direction, containingBlockWidth);

if (!child->hasErrata(Errata::AbsolutePositionWithoutInsetsExcludesPadding)) {
position += parent->getLayout().padding(flexStartEdge(axis));
}

child->setLayoutPosition(position, flexStartEdge(axis));
}

static void justifyAbsoluteChild(
Expand Down Expand Up @@ -133,62 +145,6 @@ static void alignAbsoluteChild(
}
}

// To ensure no breaking changes, we preserve the legacy way of positioning
// absolute children and determine if we should use it using an errata.
static void positionAbsoluteChildLegacy(
const yoga::Node* const containingNode,
const yoga::Node* const parent,
yoga::Node* child,
const Direction direction,
const FlexDirection axis,
const bool isMainAxis,
const float containingBlockWidth,
const float containingBlockHeight) {
const bool isAxisRow = isRow(axis);
const bool shouldCenter = isMainAxis
? parent->style().justifyContent() == Justify::Center
: resolveChildAlignment(parent, child) == Align::Center;
const bool shouldFlexEnd = isMainAxis
? parent->style().justifyContent() == Justify::FlexEnd
: ((resolveChildAlignment(parent, child) == Align::FlexEnd) ^
(parent->style().flexWrap() == Wrap::WrapReverse));

if (child->style().isFlexEndPositionDefined(axis, direction) &&
(!child->style().isFlexStartPositionDefined(axis, direction) ||
child->style().isFlexStartPositionAuto(axis, direction))) {
child->setLayoutPosition(
containingNode->getLayout().measuredDimension(dimension(axis)) -
child->getLayout().measuredDimension(dimension(axis)) -
containingNode->style().computeFlexEndBorder(axis, direction) -
child->style().computeFlexEndMargin(
axis,
direction,
isAxisRow ? containingBlockWidth : containingBlockHeight) -
child->style().computeFlexEndPosition(
axis,
direction,
isAxisRow ? containingBlockWidth : containingBlockHeight),
flexStartEdge(axis));
} else if (
(!child->style().isFlexStartPositionDefined(axis, direction) ||
child->style().isFlexStartPositionAuto(axis, direction)) &&
shouldCenter) {
child->setLayoutPosition(
(parent->getLayout().measuredDimension(dimension(axis)) -
child->getLayout().measuredDimension(dimension(axis))) /
2.0f,
flexStartEdge(axis));
} else if (
(!child->style().isFlexStartPositionDefined(axis, direction) ||
child->style().isFlexStartPositionAuto(axis, direction)) &&
shouldFlexEnd) {
child->setLayoutPosition(
(parent->getLayout().measuredDimension(dimension(axis)) -
child->getLayout().measuredDimension(dimension(axis))),
flexStartEdge(axis));
}
}

/*
* Absolutely positioned nodes do not participate in flex layout and thus their
* positions can be determined independently from the rest of their siblings.
Expand All @@ -205,7 +161,7 @@ static void positionAbsoluteChildLegacy(
* This function does that positioning for the given axis. The spec has more
* information on this topic: https://www.w3.org/TR/css-flexbox-1/#abspos-items
*/
static void positionAbsoluteChildImpl(
static void positionAbsoluteChild(
const yoga::Node* const containingNode,
const yoga::Node* const parent,
yoga::Node* child,
Expand Down Expand Up @@ -267,36 +223,6 @@ static void positionAbsoluteChildImpl(
}
}

static void positionAbsoluteChild(
const yoga::Node* const containingNode,
const yoga::Node* const parent,
yoga::Node* child,
const Direction direction,
const FlexDirection axis,
const bool isMainAxis,
const float containingBlockWidth,
const float containingBlockHeight) {
child->hasErrata(Errata::AbsolutePositioningIncorrect)
? positionAbsoluteChildLegacy(
containingNode,
parent,
child,
direction,
axis,
isMainAxis,
containingBlockWidth,
containingBlockHeight)
: positionAbsoluteChildImpl(
containingNode,
parent,
child,
direction,
axis,
isMainAxis,
containingBlockWidth,
containingBlockHeight);
}

void layoutAbsoluteChild(
const yoga::Node* const containingNode,
const yoga::Node* const node,
Expand Down
Loading

0 comments on commit 37bf399

Please sign in to comment.