Rename ambiguous leading/trailingEdge functions (#41017)

Summary:
X-link: https://github.com/facebook/yoga/pull/1423

Pull Request resolved: https://github.com/facebook/react-native/pull/41017

Before resolving https://github.com/facebook/yoga/issues/1208 yoga was in a state where "leading" and "trailing" only referred to the main-start and main-end directions ([definition in spec](https://drafts.csswg.org/css-flexbox/#box-model)). That is, the start/end of the layout of flex items in a container. This is distinct from something like inline-start/inline-end which is the [start of text layout as defined by direction](https://drafts.csswg.org/css-writing-modes-3/#inline-start).

The bug linked above happened because "leading" and "trailing" functions are referring to the wrong directions in certain cases. So in order to fix this we added a new set of functions to get the "leading" and "trailing" edges according to what inline-start/inline-end would refer to - i.e. those defined by the direction (ltr | rtl). In this state I think it is confusing to understand which function refers to which direction and more specific names could help that.

This diff just renames the following 4 FlexDirection.h functions:

* **leadingEdge** -> **flexStartEdge**
* **trailingEdge** -> **flexEndEdge**
* **leadingLayoutEdge** -> **inlineStartEdge**
* **trailingLayoutEdge** -> **inlineEndEdge**

The spec calls the start/end directions as dictated by the flex-direction attribute "main-start" and "main-end" respectively, but mainStartEdge might be a bit confusing given it will be compared to a non-flexbox-specific name in inlineStartEdge. As a result I landed on flexStart/flexEnd similar to what values are used with alignment attributes (justify-content, align-content).

I chose to get rid of the "leading" and "trailing" descriptors to be more in line with what terminology the spec uses.

Next diff will be to rename the functions in Node.cpp to adhere to the above patterns.

Reviewed By: NickGerleman

Differential Revision: D50342254

fbshipit-source-id: 1e83a885876af9cf363822ebdbb64537f4784520
This commit is contained in:
Joe Vilches 2023-10-17 20:30:16 -07:00 коммит произвёл Facebook GitHub Bot
Родитель ae0632d01d
Коммит 594b586c7d
3 изменённых файлов: 54 добавлений и 52 удалений

Просмотреть файл

@ -88,8 +88,8 @@ static void setChildTrailingPosition(
const float size = child->getLayout().measuredDimension(dimension(axis)); const float size = child->getLayout().measuredDimension(dimension(axis));
child->setLayoutPosition( child->setLayoutPosition(
node->getLayout().measuredDimension(dimension(axis)) - size - node->getLayout().measuredDimension(dimension(axis)) - size -
child->getLayout().position[leadingEdge(axis)], child->getLayout().position[flexStartEdge(axis)],
trailingEdge(axis)); flexEndEdge(axis));
} }
static void constrainMaxSizeForMode( static void constrainMaxSizeForMode(
@ -456,7 +456,7 @@ static void layoutAbsoluteChild(
mainAxis, direction, isMainAxisRow ? width : height) - mainAxis, direction, isMainAxisRow ? width : height) -
child->getTrailingPosition( child->getTrailingPosition(
mainAxis, isMainAxisRow ? width : height), mainAxis, isMainAxisRow ? width : height),
leadingEdge(mainAxis)); flexStartEdge(mainAxis));
} else if ( } else if (
!child->isLeadingPositionDefined(mainAxis) && !child->isLeadingPositionDefined(mainAxis) &&
node->getStyle().justifyContent() == Justify::Center) { node->getStyle().justifyContent() == Justify::Center) {
@ -464,14 +464,14 @@ static void layoutAbsoluteChild(
(node->getLayout().measuredDimension(dimension(mainAxis)) - (node->getLayout().measuredDimension(dimension(mainAxis)) -
child->getLayout().measuredDimension(dimension(mainAxis))) / child->getLayout().measuredDimension(dimension(mainAxis))) /
2.0f, 2.0f,
leadingEdge(mainAxis)); flexStartEdge(mainAxis));
} else if ( } else if (
!child->isLeadingPositionDefined(mainAxis) && !child->isLeadingPositionDefined(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)) -
child->getLayout().measuredDimension(dimension(mainAxis))), child->getLayout().measuredDimension(dimension(mainAxis))),
leadingEdge(mainAxis)); flexStartEdge(mainAxis));
} else if ( } else if (
node->getConfig()->isExperimentalFeatureEnabled( node->getConfig()->isExperimentalFeatureEnabled(
ExperimentalFeature::AbsolutePercentageAgainstPaddingEdge) && ExperimentalFeature::AbsolutePercentageAgainstPaddingEdge) &&
@ -485,7 +485,7 @@ static void layoutAbsoluteChild(
mainAxis, mainAxis,
direction, direction,
node->getLayout().measuredDimension(dimension(mainAxis))), node->getLayout().measuredDimension(dimension(mainAxis))),
leadingEdge(mainAxis)); flexStartEdge(mainAxis));
} }
if (child->isTrailingPosDefined(crossAxis) && if (child->isTrailingPosDefined(crossAxis) &&
@ -498,7 +498,7 @@ static void layoutAbsoluteChild(
crossAxis, direction, isMainAxisRow ? height : width) - crossAxis, direction, isMainAxisRow ? height : width) -
child->getTrailingPosition( child->getTrailingPosition(
crossAxis, isMainAxisRow ? height : width), crossAxis, isMainAxisRow ? height : width),
leadingEdge(crossAxis)); flexStartEdge(crossAxis));
} else if ( } else if (
!child->isLeadingPositionDefined(crossAxis) && !child->isLeadingPositionDefined(crossAxis) &&
@ -507,7 +507,7 @@ static void layoutAbsoluteChild(
(node->getLayout().measuredDimension(dimension(crossAxis)) - (node->getLayout().measuredDimension(dimension(crossAxis)) -
child->getLayout().measuredDimension(dimension(crossAxis))) / child->getLayout().measuredDimension(dimension(crossAxis))) /
2.0f, 2.0f,
leadingEdge(crossAxis)); flexStartEdge(crossAxis));
} else if ( } else if (
!child->isLeadingPositionDefined(crossAxis) && !child->isLeadingPositionDefined(crossAxis) &&
((resolveChildAlignment(node, child) == Align::FlexEnd) ^ ((resolveChildAlignment(node, child) == Align::FlexEnd) ^
@ -515,7 +515,7 @@ static void layoutAbsoluteChild(
child->setLayoutPosition( child->setLayoutPosition(
(node->getLayout().measuredDimension(dimension(crossAxis)) - (node->getLayout().measuredDimension(dimension(crossAxis)) -
child->getLayout().measuredDimension(dimension(crossAxis))), child->getLayout().measuredDimension(dimension(crossAxis))),
leadingEdge(crossAxis)); flexStartEdge(crossAxis));
} else if ( } else if (
node->getConfig()->isExperimentalFeatureEnabled( node->getConfig()->isExperimentalFeatureEnabled(
ExperimentalFeature::AbsolutePercentageAgainstPaddingEdge) && ExperimentalFeature::AbsolutePercentageAgainstPaddingEdge) &&
@ -529,7 +529,7 @@ static void layoutAbsoluteChild(
crossAxis, crossAxis,
direction, direction,
node->getLayout().measuredDimension(dimension(crossAxis))), node->getLayout().measuredDimension(dimension(crossAxis))),
leadingEdge(crossAxis)); flexStartEdge(crossAxis));
} }
} }
@ -1308,7 +1308,7 @@ static void justifyMainAxis(
node->getLeadingBorder(mainAxis) + node->getLeadingBorder(mainAxis) +
child->getLeadingMargin( child->getLeadingMargin(
mainAxis, direction, availableInnerWidth), mainAxis, direction, availableInnerWidth),
leadingEdge(mainAxis)); flexStartEdge(mainAxis));
} }
} else { } else {
// Now that we placed the element, we need to update the variables. // Now that we placed the element, we need to update the variables.
@ -1322,9 +1322,9 @@ static void justifyMainAxis(
if (performLayout) { if (performLayout) {
child->setLayoutPosition( child->setLayoutPosition(
childLayout.position[leadingEdge(mainAxis)] + childLayout.position[flexStartEdge(mainAxis)] +
flexLine.layout.mainDim, flexLine.layout.mainDim,
leadingEdge(mainAxis)); flexStartEdge(mainAxis));
} }
if (child != flexLine.itemsInFlow.back()) { if (child != flexLine.itemsInFlow.back()) {
@ -1378,9 +1378,9 @@ static void justifyMainAxis(
} }
} else if (performLayout) { } else if (performLayout) {
child->setLayoutPosition( child->setLayoutPosition(
childLayout.position[leadingEdge(mainAxis)] + childLayout.position[flexStartEdge(mainAxis)] +
node->getLeadingBorder(mainAxis) + leadingMainDim, node->getLeadingBorder(mainAxis) + leadingMainDim,
leadingEdge(mainAxis)); flexStartEdge(mainAxis));
} }
} }
} }
@ -1856,18 +1856,18 @@ static void calculateLayoutImpl(
node->getLeadingBorder(crossAxis) + node->getLeadingBorder(crossAxis) +
child->getLeadingMargin( child->getLeadingMargin(
crossAxis, direction, availableInnerWidth), crossAxis, direction, availableInnerWidth),
leadingEdge(crossAxis)); flexStartEdge(crossAxis));
} }
// If leading position is not defined or calculations result in Nan, // If leading position is not defined or calculations result in Nan,
// default to border + margin // default to border + margin
if (!isChildLeadingPosDefined || if (!isChildLeadingPosDefined ||
yoga::isUndefined( yoga::isUndefined(
child->getLayout().position[leadingEdge(crossAxis)])) { child->getLayout().position[flexStartEdge(crossAxis)])) {
child->setLayoutPosition( child->setLayoutPosition(
node->getLeadingBorder(crossAxis) + node->getLeadingBorder(crossAxis) +
child->getLeadingMargin( child->getLeadingMargin(
crossAxis, direction, availableInnerWidth), crossAxis, direction, availableInnerWidth),
leadingEdge(crossAxis)); flexStartEdge(crossAxis));
} }
} else { } else {
float leadingCrossDim = leadingPaddingAndBorderCross; float leadingCrossDim = leadingPaddingAndBorderCross;
@ -1975,9 +1975,9 @@ static void calculateLayoutImpl(
} }
// And we apply the position // And we apply the position
child->setLayoutPosition( child->setLayoutPosition(
child->getLayout().position[leadingEdge(crossAxis)] + child->getLayout().position[flexStartEdge(crossAxis)] +
totalLineCrossDim + leadingCrossDim, totalLineCrossDim + leadingCrossDim,
leadingEdge(crossAxis)); flexStartEdge(crossAxis));
} }
} }
} }
@ -2092,7 +2092,7 @@ static void calculateLayoutImpl(
currentLead + currentLead +
child->getLeadingMargin( child->getLeadingMargin(
crossAxis, direction, availableInnerWidth), crossAxis, direction, availableInnerWidth),
leadingEdge(crossAxis)); flexStartEdge(crossAxis));
break; break;
} }
case Align::FlexEnd: { case Align::FlexEnd: {
@ -2102,7 +2102,7 @@ static void calculateLayoutImpl(
crossAxis, direction, availableInnerWidth) - crossAxis, direction, availableInnerWidth) -
child->getLayout().measuredDimension( child->getLayout().measuredDimension(
dimension(crossAxis)), dimension(crossAxis)),
leadingEdge(crossAxis)); flexStartEdge(crossAxis));
break; break;
} }
case Align::Center: { case Align::Center: {
@ -2111,7 +2111,7 @@ static void calculateLayoutImpl(
child->setLayoutPosition( child->setLayoutPosition(
currentLead + (lineHeight - childHeight) / 2, currentLead + (lineHeight - childHeight) / 2,
leadingEdge(crossAxis)); flexStartEdge(crossAxis));
break; break;
} }
case Align::Stretch: { case Align::Stretch: {
@ -2119,7 +2119,7 @@ static void calculateLayoutImpl(
currentLead + currentLead +
child->getLeadingMargin( child->getLeadingMargin(
crossAxis, direction, availableInnerWidth), crossAxis, direction, availableInnerWidth),
leadingEdge(crossAxis)); flexStartEdge(crossAxis));
// Remeasure child with the line height as it as been only // Remeasure child with the line height as it as been only
// measured with the owners height yet. // measured with the owners height yet.
@ -2275,9 +2275,9 @@ static void calculateLayoutImpl(
if (child->getStyle().positionType() != PositionType::Absolute) { if (child->getStyle().positionType() != PositionType::Absolute) {
child->setLayoutPosition( child->setLayoutPosition(
node->getLayout().measuredDimension(dimension(crossAxis)) - node->getLayout().measuredDimension(dimension(crossAxis)) -
child->getLayout().position[leadingEdge(crossAxis)] - child->getLayout().position[flexStartEdge(crossAxis)] -
child->getLayout().measuredDimension(dimension(crossAxis)), child->getLayout().measuredDimension(dimension(crossAxis)),
leadingEdge(crossAxis)); flexStartEdge(crossAxis));
} }
} }
} }

Просмотреть файл

@ -47,7 +47,7 @@ inline FlexDirection resolveCrossDirection(
: FlexDirection::Column; : FlexDirection::Column;
} }
inline YGEdge leadingEdge(const FlexDirection flexDirection) { inline YGEdge flexStartEdge(const FlexDirection flexDirection) {
switch (flexDirection) { switch (flexDirection) {
case FlexDirection::Column: case FlexDirection::Column:
return YGEdgeTop; return YGEdgeTop;
@ -62,7 +62,7 @@ inline YGEdge leadingEdge(const FlexDirection flexDirection) {
fatalWithMessage("Invalid FlexDirection"); fatalWithMessage("Invalid FlexDirection");
} }
inline YGEdge trailingEdge(const FlexDirection flexDirection) { inline YGEdge flexEndEdge(const FlexDirection flexDirection) {
switch (flexDirection) { switch (flexDirection) {
case FlexDirection::Column: case FlexDirection::Column:
return YGEdgeBottom; return YGEdgeBottom;
@ -77,7 +77,7 @@ inline YGEdge trailingEdge(const FlexDirection flexDirection) {
fatalWithMessage("Invalid FlexDirection"); fatalWithMessage("Invalid FlexDirection");
} }
inline YGEdge leadingLayoutEdge( inline YGEdge inlineStartEdge(
const FlexDirection flexDirection, const FlexDirection flexDirection,
const Direction direction) { const Direction direction) {
if (isRow(flexDirection)) { if (isRow(flexDirection)) {
@ -87,7 +87,7 @@ inline YGEdge leadingLayoutEdge(
return YGEdgeTop; return YGEdgeTop;
} }
inline YGEdge trailingLayoutEdge( inline YGEdge inlineEndEdge(
const FlexDirection flexDirection, const FlexDirection flexDirection,
const Direction direction) { const Direction direction) {
if (isRow(flexDirection)) { if (isRow(flexDirection)) {

Просмотреть файл

@ -87,31 +87,31 @@ YGEdge Node::getLeadingLayoutEdgeUsingErrata(
FlexDirection flexDirection, FlexDirection flexDirection,
Direction direction) const { Direction direction) const {
return hasErrata(Errata::StartingEndingEdgeFromFlexDirection) return hasErrata(Errata::StartingEndingEdgeFromFlexDirection)
? leadingEdge(flexDirection) ? flexStartEdge(flexDirection)
: leadingLayoutEdge(flexDirection, direction); : inlineStartEdge(flexDirection, direction);
} }
YGEdge Node::getTrailingLayoutEdgeUsingErrata( YGEdge Node::getTrailingLayoutEdgeUsingErrata(
FlexDirection flexDirection, FlexDirection flexDirection,
Direction direction) const { Direction direction) const {
return hasErrata(Errata::StartingEndingEdgeFromFlexDirection) return hasErrata(Errata::StartingEndingEdgeFromFlexDirection)
? trailingEdge(flexDirection) ? flexEndEdge(flexDirection)
: trailingLayoutEdge(flexDirection, direction); : inlineEndEdge(flexDirection, direction);
} }
bool Node::isLeadingPositionDefined(FlexDirection axis) const { bool Node::isLeadingPositionDefined(FlexDirection axis) const {
auto leadingPosition = isRow(axis) auto leadingPosition = isRow(axis)
? computeEdgeValueForRow( ? computeEdgeValueForRow(
style_.position(), YGEdgeStart, leadingEdge(axis)) style_.position(), YGEdgeStart, flexStartEdge(axis))
: computeEdgeValueForColumn(style_.position(), leadingEdge(axis)); : computeEdgeValueForColumn(style_.position(), flexStartEdge(axis));
return !leadingPosition.isUndefined(); return !leadingPosition.isUndefined();
} }
bool Node::isTrailingPosDefined(FlexDirection axis) const { bool Node::isTrailingPosDefined(FlexDirection axis) const {
auto trailingPosition = isRow(axis) auto trailingPosition = isRow(axis)
? computeEdgeValueForRow(style_.position(), YGEdgeEnd, trailingEdge(axis)) ? computeEdgeValueForRow(style_.position(), YGEdgeEnd, flexEndEdge(axis))
: computeEdgeValueForColumn(style_.position(), trailingEdge(axis)); : computeEdgeValueForColumn(style_.position(), flexEndEdge(axis));
return !trailingPosition.isUndefined(); return !trailingPosition.isUndefined();
} }
@ -119,16 +119,16 @@ bool Node::isTrailingPosDefined(FlexDirection axis) const {
float Node::getLeadingPosition(FlexDirection axis, float axisSize) const { float Node::getLeadingPosition(FlexDirection axis, float axisSize) const {
auto leadingPosition = isRow(axis) auto leadingPosition = isRow(axis)
? computeEdgeValueForRow( ? computeEdgeValueForRow(
style_.position(), YGEdgeStart, leadingEdge(axis)) style_.position(), YGEdgeStart, flexStartEdge(axis))
: computeEdgeValueForColumn(style_.position(), leadingEdge(axis)); : computeEdgeValueForColumn(style_.position(), flexStartEdge(axis));
return resolveValue(leadingPosition, axisSize).unwrapOrDefault(0.0f); return resolveValue(leadingPosition, axisSize).unwrapOrDefault(0.0f);
} }
float Node::getTrailingPosition(FlexDirection axis, float axisSize) const { float Node::getTrailingPosition(FlexDirection axis, float axisSize) const {
auto trailingPosition = isRow(axis) auto trailingPosition = isRow(axis)
? computeEdgeValueForRow(style_.position(), YGEdgeEnd, trailingEdge(axis)) ? computeEdgeValueForRow(style_.position(), YGEdgeEnd, flexEndEdge(axis))
: computeEdgeValueForColumn(style_.position(), trailingEdge(axis)); : computeEdgeValueForColumn(style_.position(), flexEndEdge(axis));
return resolveValue(trailingPosition, axisSize).unwrapOrDefault(0.0f); return resolveValue(trailingPosition, axisSize).unwrapOrDefault(0.0f);
} }
@ -159,32 +159,34 @@ float Node::getTrailingMargin(
float Node::getLeadingBorder(FlexDirection axis) const { float Node::getLeadingBorder(FlexDirection axis) const {
YGValue leadingBorder = isRow(axis) YGValue leadingBorder = isRow(axis)
? computeEdgeValueForRow(style_.border(), YGEdgeStart, leadingEdge(axis)) ? computeEdgeValueForRow(
: computeEdgeValueForColumn(style_.border(), leadingEdge(axis)); style_.border(), YGEdgeStart, flexStartEdge(axis))
: computeEdgeValueForColumn(style_.border(), flexStartEdge(axis));
return maxOrDefined(leadingBorder.value, 0.0f); return maxOrDefined(leadingBorder.value, 0.0f);
} }
float Node::getTrailingBorder(FlexDirection axis) const { float Node::getTrailingBorder(FlexDirection axis) const {
YGValue trailingBorder = isRow(axis) YGValue trailingBorder = isRow(axis)
? computeEdgeValueForRow(style_.border(), YGEdgeEnd, trailingEdge(axis)) ? computeEdgeValueForRow(style_.border(), YGEdgeEnd, flexEndEdge(axis))
: computeEdgeValueForColumn(style_.border(), trailingEdge(axis)); : computeEdgeValueForColumn(style_.border(), flexEndEdge(axis));
return maxOrDefined(trailingBorder.value, 0.0f); return maxOrDefined(trailingBorder.value, 0.0f);
} }
float Node::getLeadingPadding(FlexDirection axis, float widthSize) const { float Node::getLeadingPadding(FlexDirection axis, float widthSize) const {
auto leadingPadding = isRow(axis) auto leadingPadding = isRow(axis)
? computeEdgeValueForRow(style_.padding(), YGEdgeStart, leadingEdge(axis)) ? computeEdgeValueForRow(
: computeEdgeValueForColumn(style_.padding(), leadingEdge(axis)); style_.padding(), YGEdgeStart, flexStartEdge(axis))
: computeEdgeValueForColumn(style_.padding(), flexStartEdge(axis));
return maxOrDefined(resolveValue(leadingPadding, widthSize).unwrap(), 0.0f); return maxOrDefined(resolveValue(leadingPadding, widthSize).unwrap(), 0.0f);
} }
float Node::getTrailingPadding(FlexDirection axis, float widthSize) const { float Node::getTrailingPadding(FlexDirection axis, float widthSize) const {
auto trailingPadding = isRow(axis) auto trailingPadding = isRow(axis)
? computeEdgeValueForRow(style_.padding(), YGEdgeEnd, trailingEdge(axis)) ? computeEdgeValueForRow(style_.padding(), YGEdgeEnd, flexEndEdge(axis))
: computeEdgeValueForColumn(style_.padding(), trailingEdge(axis)); : computeEdgeValueForColumn(style_.padding(), flexEndEdge(axis));
return maxOrDefined(resolveValue(trailingPadding, widthSize).unwrap(), 0.0f); return maxOrDefined(resolveValue(trailingPadding, widthSize).unwrap(), 0.0f);
} }
@ -418,7 +420,7 @@ YGValue Node::marginLeadingValue(FlexDirection axis) const {
if (isRow(axis) && !style_.margin()[YGEdgeStart].isUndefined()) { if (isRow(axis) && !style_.margin()[YGEdgeStart].isUndefined()) {
return style_.margin()[YGEdgeStart]; return style_.margin()[YGEdgeStart];
} else { } else {
return style_.margin()[leadingEdge(axis)]; return style_.margin()[flexStartEdge(axis)];
} }
} }
@ -426,7 +428,7 @@ YGValue Node::marginTrailingValue(FlexDirection axis) const {
if (isRow(axis) && !style_.margin()[YGEdgeEnd].isUndefined()) { if (isRow(axis) && !style_.margin()[YGEdgeEnd].isUndefined()) {
return style_.margin()[YGEdgeEnd]; return style_.margin()[YGEdgeEnd];
} else { } else {
return style_.margin()[trailingEdge(axis)]; return style_.margin()[flexEndEdge(axis)];
} }
} }