Skip to content

Commit 6cd7d33

Browse files
NickGerlemanfacebook-github-bot
authored andcommitted
Remove legacy absolute positioning path (#1725)
Summary: X-link: facebook/react-native#46984 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 did not try removing `AbsolutePercentAgainstInnerSize` which I suspect would be pretty breaking. Changelog: [General][Breaking] - More spec compliant absolute positioning Reviewed By: joevilches Differential Revision: D64244949
1 parent 43be588 commit 6cd7d33

File tree

8 files changed

+169
-336
lines changed

8 files changed

+169
-336
lines changed

enums.py

+1-4
Original file line numberDiff line numberDiff line change
@@ -61,12 +61,9 @@
6161
# Allows main-axis flex basis to be stretched without flexGrow being
6262
# set (previously referred to as "UseLegacyStretchBehaviour")
6363
("StretchFlexBasis", 1 << 0),
64-
# Positioning of absolute nodes will have various bugs related to
65-
# justification, alignment, and insets
66-
("AbsolutePositioningIncorrect", 1 << 1),
6764
# Absolute nodes will resolve percentages against the inner size of
6865
# their containing node, not the padding box
69-
("AbsolutePercentAgainstInnerSize", 1 << 2),
66+
("AbsolutePercentAgainstInnerSize", 1 << 1),
7067
# Enable all incorrect behavior (preserve compatibility)
7168
("All", 0x7FFFFFFF),
7269
# Enable all errata except for "StretchFlexBasis" (Defaults behavior

java/com/facebook/yoga/YogaErrata.java

+2-4
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,7 @@
1212
public enum YogaErrata {
1313
NONE(0),
1414
STRETCH_FLEX_BASIS(1),
15-
ABSOLUTE_POSITIONING_INCORRECT(2),
16-
ABSOLUTE_PERCENT_AGAINST_INNER_SIZE(4),
15+
ABSOLUTE_PERCENT_AGAINST_INNER_SIZE(2),
1716
ALL(2147483647),
1817
CLASSIC(2147483646);
1918

@@ -31,8 +30,7 @@ public static YogaErrata fromInt(int value) {
3130
switch (value) {
3231
case 0: return NONE;
3332
case 1: return STRETCH_FLEX_BASIS;
34-
case 2: return ABSOLUTE_POSITIONING_INCORRECT;
35-
case 4: return ABSOLUTE_PERCENT_AGAINST_INNER_SIZE;
33+
case 2: return ABSOLUTE_PERCENT_AGAINST_INNER_SIZE;
3634
case 2147483647: return ALL;
3735
case 2147483646: return CLASSIC;
3836
default: throw new IllegalArgumentException("Unknown enum value: " + value);

javascript/src/generated/YGEnums.ts

+1-3
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,7 @@ export enum Edge {
5555
export enum Errata {
5656
None = 0,
5757
StretchFlexBasis = 1,
58-
AbsolutePositioningIncorrect = 2,
59-
AbsolutePercentAgainstInnerSize = 4,
58+
AbsolutePercentAgainstInnerSize = 2,
6059
All = 2147483647,
6160
Classic = 2147483646,
6261
}
@@ -162,7 +161,6 @@ const constants = {
162161
EDGE_ALL: Edge.All,
163162
ERRATA_NONE: Errata.None,
164163
ERRATA_STRETCH_FLEX_BASIS: Errata.StretchFlexBasis,
165-
ERRATA_ABSOLUTE_POSITIONING_INCORRECT: Errata.AbsolutePositioningIncorrect,
166164
ERRATA_ABSOLUTE_PERCENT_AGAINST_INNER_SIZE: Errata.AbsolutePercentAgainstInnerSize,
167165
ERRATA_ALL: Errata.All,
168166
ERRATA_CLASSIC: Errata.Classic,

yoga/YGEnums.cpp

-2
Original file line numberDiff line numberDiff line change
@@ -105,8 +105,6 @@ const char* YGErrataToString(const YGErrata value) {
105105
return "none";
106106
case YGErrataStretchFlexBasis:
107107
return "stretch-flex-basis";
108-
case YGErrataAbsolutePositioningIncorrect:
109-
return "absolute-positioning-incorrect";
110108
case YGErrataAbsolutePercentAgainstInnerSize:
111109
return "absolute-percent-against-inner-size";
112110
case YGErrataAll:

yoga/YGEnums.h

+1-2
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,7 @@ YG_ENUM_DECL(
6161
YGErrata,
6262
YGErrataNone = 0,
6363
YGErrataStretchFlexBasis = 1,
64-
YGErrataAbsolutePositioningIncorrect = 2,
65-
YGErrataAbsolutePercentAgainstInnerSize = 4,
64+
YGErrataAbsolutePercentAgainstInnerSize = 2,
6665
YGErrataAll = 2147483647,
6766
YGErrataClassic = 2147483646)
6867
YG_DEFINE_ENUM_FLAG_OPERATORS(YGErrata)

yoga/algorithm/AbsoluteLayout.cpp

+1-87
Original file line numberDiff line numberDiff line change
@@ -133,62 +133,6 @@ static void alignAbsoluteChild(
133133
}
134134
}
135135

136-
// To ensure no breaking changes, we preserve the legacy way of positioning
137-
// absolute children and determine if we should use it using an errata.
138-
static void positionAbsoluteChildLegacy(
139-
const yoga::Node* const containingNode,
140-
const yoga::Node* const parent,
141-
yoga::Node* child,
142-
const Direction direction,
143-
const FlexDirection axis,
144-
const bool isMainAxis,
145-
const float containingBlockWidth,
146-
const float containingBlockHeight) {
147-
const bool isAxisRow = isRow(axis);
148-
const bool shouldCenter = isMainAxis
149-
? parent->style().justifyContent() == Justify::Center
150-
: resolveChildAlignment(parent, child) == Align::Center;
151-
const bool shouldFlexEnd = isMainAxis
152-
? parent->style().justifyContent() == Justify::FlexEnd
153-
: ((resolveChildAlignment(parent, child) == Align::FlexEnd) ^
154-
(parent->style().flexWrap() == Wrap::WrapReverse));
155-
156-
if (child->style().isFlexEndPositionDefined(axis, direction) &&
157-
(!child->style().isFlexStartPositionDefined(axis, direction) ||
158-
child->style().isFlexStartPositionAuto(axis, direction))) {
159-
child->setLayoutPosition(
160-
containingNode->getLayout().measuredDimension(dimension(axis)) -
161-
child->getLayout().measuredDimension(dimension(axis)) -
162-
containingNode->style().computeFlexEndBorder(axis, direction) -
163-
child->style().computeFlexEndMargin(
164-
axis,
165-
direction,
166-
isAxisRow ? containingBlockWidth : containingBlockHeight) -
167-
child->style().computeFlexEndPosition(
168-
axis,
169-
direction,
170-
isAxisRow ? containingBlockWidth : containingBlockHeight),
171-
flexStartEdge(axis));
172-
} else if (
173-
(!child->style().isFlexStartPositionDefined(axis, direction) ||
174-
child->style().isFlexStartPositionAuto(axis, direction)) &&
175-
shouldCenter) {
176-
child->setLayoutPosition(
177-
(parent->getLayout().measuredDimension(dimension(axis)) -
178-
child->getLayout().measuredDimension(dimension(axis))) /
179-
2.0f,
180-
flexStartEdge(axis));
181-
} else if (
182-
(!child->style().isFlexStartPositionDefined(axis, direction) ||
183-
child->style().isFlexStartPositionAuto(axis, direction)) &&
184-
shouldFlexEnd) {
185-
child->setLayoutPosition(
186-
(parent->getLayout().measuredDimension(dimension(axis)) -
187-
child->getLayout().measuredDimension(dimension(axis))),
188-
flexStartEdge(axis));
189-
}
190-
}
191-
192136
/*
193137
* Absolutely positioned nodes do not participate in flex layout and thus their
194138
* positions can be determined independently from the rest of their siblings.
@@ -205,7 +149,7 @@ static void positionAbsoluteChildLegacy(
205149
* This function does that positioning for the given axis. The spec has more
206150
* information on this topic: https://www.w3.org/TR/css-flexbox-1/#abspos-items
207151
*/
208-
static void positionAbsoluteChildImpl(
152+
static void positionAbsoluteChild(
209153
const yoga::Node* const containingNode,
210154
const yoga::Node* const parent,
211155
yoga::Node* child,
@@ -267,36 +211,6 @@ static void positionAbsoluteChildImpl(
267211
}
268212
}
269213

270-
static void positionAbsoluteChild(
271-
const yoga::Node* const containingNode,
272-
const yoga::Node* const parent,
273-
yoga::Node* child,
274-
const Direction direction,
275-
const FlexDirection axis,
276-
const bool isMainAxis,
277-
const float containingBlockWidth,
278-
const float containingBlockHeight) {
279-
child->hasErrata(Errata::AbsolutePositioningIncorrect)
280-
? positionAbsoluteChildLegacy(
281-
containingNode,
282-
parent,
283-
child,
284-
direction,
285-
axis,
286-
isMainAxis,
287-
containingBlockWidth,
288-
containingBlockHeight)
289-
: positionAbsoluteChildImpl(
290-
containingNode,
291-
parent,
292-
child,
293-
direction,
294-
axis,
295-
isMainAxis,
296-
containingBlockWidth,
297-
containingBlockHeight);
298-
}
299-
300214
void layoutAbsoluteChild(
301215
const yoga::Node* const containingNode,
302216
const yoga::Node* const node,

0 commit comments

Comments
 (0)