Skip to content

Commit

Permalink
Remove row-reverse errata (#42251)
Browse files Browse the repository at this point in the history
Summary:
X-link: facebook/yoga#1547


Yoga has an odd behavior, where `start`/`end` edges under row-reverse are relative to flex-direction, instead of writing direction.

While Yoga doesn't actually document what this behavior is supposed to be, it goes against CK documentation, historic RN documentation, and the behavior valid on the web. It is also applied inconsistently (e.g. sometimes only on container, sometimes on child). It really is a bug, instead of an intended behavior.

We changed the default behavior for Yoga, but left the existing one behind an errata (so existing fbsource users got old behavior). We have previously seen this behavior show up in product code, including CK when running on FlexLayout.

`row-reverse` is surprisingly uncommon though:
1. Litho has <40 usages
2. RN has ~40 usages in `RKJSModules`,~30 in `arvr/js`, ~6 in `xplat/archon`
3. CK has ~80 usages
4. NT has ~40 usages

These components are pretty static, and few enough that we can do this style of refactor, where we just fix everything we find. But there is some risk of missing complonents.

CK accounts for 10/13 usages that I could tell would trigger the issue, since it only exposes start/end edge, and not left/right. It might make sense to make it preserve behavior instead, to reduce risk a bit.

FlexLayout is now separately powering Bloks, which wasn't surveryed, do I didn't touch CK behavior under Bloks.

There could also be other usages in other frameworks/bespoke usages, and this has implications for OSS users. But based on our own usage, of many, many components, this seems rare.

Changelog:
[General][Breaking] - Make `start/end` in styles always refer to writing direction

Reviewed By: joevilches

Differential Revision: D52698130
  • Loading branch information
NickGerleman authored and facebook-github-bot committed Jan 11, 2024
1 parent 3630138 commit 6f54e66
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -920,13 +920,9 @@ static void justifyMainAxis(
const auto& style = node->getStyle();

const float leadingPaddingAndBorderMain =
node->hasErrata(Errata::StartingEndingEdgeFromFlexDirection)
? node->getInlineStartPaddingAndBorder(mainAxis, direction, ownerWidth)
: node->getFlexStartPaddingAndBorder(mainAxis, direction, ownerWidth);
node->getFlexStartPaddingAndBorder(mainAxis, direction, ownerWidth);
const float trailingPaddingAndBorderMain =
node->hasErrata(Errata::StartingEndingEdgeFromFlexDirection)
? node->getInlineEndPaddingAndBorder(mainAxis, direction, ownerWidth)
: node->getFlexEndPaddingAndBorder(mainAxis, direction, ownerWidth);
node->getFlexEndPaddingAndBorder(mainAxis, direction, ownerWidth);

const float gap = node->getGapForAxis(mainAxis);
// If we are using "at most" rules in the main axis, make sure that
Expand Down
16 changes: 4 additions & 12 deletions packages/react-native/ReactCommon/yoga/yoga/node/Node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,33 +84,25 @@ Style::Length Node::computeEdgeValueForColumn(Edge edge) const {
Edge Node::getInlineStartEdgeUsingErrata(
FlexDirection flexDirection,
Direction direction) const {
return hasErrata(Errata::StartingEndingEdgeFromFlexDirection)
? flexStartEdge(flexDirection)
: inlineStartEdge(flexDirection, direction);
return inlineStartEdge(flexDirection, direction);
}

Edge Node::getInlineEndEdgeUsingErrata(
FlexDirection flexDirection,
Direction direction) const {
return hasErrata(Errata::StartingEndingEdgeFromFlexDirection)
? flexEndEdge(flexDirection)
: inlineEndEdge(flexDirection, direction);
return inlineEndEdge(flexDirection, direction);
}

Edge Node::getFlexStartRelativeEdgeUsingErrata(
FlexDirection flexDirection,
Direction direction) const {
return hasErrata(Errata::StartingEndingEdgeFromFlexDirection)
? Edge::Start
: flexStartRelativeEdge(flexDirection, direction);
return flexStartRelativeEdge(flexDirection, direction);
}

Edge Node::getFlexEndRelativeEdgeUsingErrata(
FlexDirection flexDirection,
Direction direction) const {
return hasErrata(Errata::StartingEndingEdgeFromFlexDirection)
? Edge::End
: flexEndRelativeEdge(flexDirection, direction);
return flexEndRelativeEdge(flexDirection, direction);
}

bool Node::isFlexStartPositionDefined(FlexDirection axis, Direction direction)
Expand Down

0 comments on commit 6f54e66

Please sign in to comment.