"flex-direction: column-reverse" doesn't follow the specs

[Sharcoux]

Report

• I have searched existing issues and this is not a duplicate

This might be related, but not a duplicate of #807

Issues and Steps to Reproduce

The issue has been discussed here and was attributed to yoga.

Expected Behavior

This is the expected behaviour for column-reverse.

Actual Behavior

As you can see in the screencast here, the scroll doesn't start at the bottom of the container. This is specified in the specs

Link to Code

You can recreate the bug by cloning this project, and then replacing the content of the file nodes_modules/react-native/Libraries/Lists/VirtualizedList.js by the content of this file

In the FlatList, you now have a list with flex-direction: row-reverse, but it doesn't start at the bottom like it should.

If someone can give me some directions, I might be able to propose a pull-request.

[Sharcoux]

Can't anyone familiar with the project give me some directions? I couldn't locate where the flex-direction properties are interpreted by Yoga.

[Sharcoux]

Ok, thanks to this PR, I found that flex-directions is handled by yoga/src/Layout.c, yoga/src/Layout.js and yoga/src/LayoutEngine.java. I could even find the lines that should be edited to fix the issue. The only remaining problem is: WHERE THE HELL DID THOSE FILES GO???

Searching for 'flex-direction' in master lead me nowhere. The current files structure in master looks more like a maze to me. Could someone tell me where the Layout files ended up?

[danalloway]

@Sharcoux

looks like it uses enums to reference the flex properties

yoga/yoga/YGEnums.h

Lines 91 to 96 in 4840495

	YG_ENUM_SEQ_DECL(
	YGFlexDirection,
	YGFlexDirectionColumn,
	YGFlexDirectionColumnReverse,
	YGFlexDirectionRow,
	YGFlexDirectionRowReverse)

searching for YGFlexDirectionColumnReverse gives these results

https://github.com/facebook/yoga/search?q=YGFlexDirectionColumnReverse&unscoped_q=YGFlexDirectionColumnReverse

you mentioned knowing the changes that needs to happen to fix the issue, would you mind sharing what those might be?

thanks for your sleuthing on this!

[Sharcoux]

Oh cool ! Thanks !

Ok, for what I understand, it should be this part in yoga.cpp:

  // Set the resolved resolution in the node's layout.
  const YGDirection direction = node->resolveDirection(ownerDirection);
  node->setLayoutDirection(direction);

  const YGFlexDirection flexRowDirection =
      YGResolveFlexDirection(YGFlexDirectionRow, direction);
  const YGFlexDirection flexColumnDirection =
      YGResolveFlexDirection(YGFlexDirectionColumn, direction);

  const YGEdge startEdge =
      direction == YGDirectionLTR ? YGEdgeLeft : YGEdgeRight;
  const YGEdge endEdge = direction == YGDirectionLTR ? YGEdgeRight : YGEdgeLeft;
  node->setLayoutMargin(
      node->getLeadingMargin(flexRowDirection, ownerWidth).unwrap(), startEdge);
  node->setLayoutMargin(
      node->getTrailingMargin(flexRowDirection, ownerWidth).unwrap(), endEdge);
  node->setLayoutMargin(
      node->getLeadingMargin(flexColumnDirection, ownerWidth).unwrap(),
      YGEdgeTop);
  node->setLayoutMargin(
      node->getTrailingMargin(flexColumnDirection, ownerWidth).unwrap(),
      YGEdgeBottom);

  node->setLayoutBorder(node->getLeadingBorder(flexRowDirection), startEdge);
  node->setLayoutBorder(node->getTrailingBorder(flexRowDirection), endEdge);
  node->setLayoutBorder(node->getLeadingBorder(flexColumnDirection), YGEdgeTop);
  node->setLayoutBorder(
      node->getTrailingBorder(flexColumnDirection), YGEdgeBottom);

  node->setLayoutPadding(
      node->getLeadingPadding(flexRowDirection, ownerWidth).unwrap(),
      startEdge);
  node->setLayoutPadding(
      node->getTrailingPadding(flexRowDirection, ownerWidth).unwrap(), endEdge);
  node->setLayoutPadding(
      node->getLeadingPadding(flexColumnDirection, ownerWidth).unwrap(),
      YGEdgeTop);
  node->setLayoutPadding(
      node->getTrailingPadding(flexColumnDirection, ownerWidth).unwrap(),
      YGEdge

Or maybe this in YGNode.cpp

YGFloatOptional YGNode::getLeadingPosition(
    const YGFlexDirection axis,
    const float axisSize) const {
  if (YGFlexDirectionIsRow(axis)) {
    auto leadingPosition = YGComputedEdgeValue(
        style_.position(), YGEdgeStart, CompactValue::ofUndefined());
    if (!leadingPosition.isUndefined()) {
      return YGResolveValue(leadingPosition, axisSize);
    }
  }

  auto leadingPosition = YGComputedEdgeValue(
      style_.position(), leading[axis], CompactValue::ofUndefined());

  return leadingPosition.isUndefined()
      ? YGFloatOptional{0}
      : YGResolveValue(leadingPosition, axisSize);
}

YGFloatOptional YGNode::getTrailingPosition(
    const YGFlexDirection axis,
    const float axisSize) const {
  if (YGFlexDirectionIsRow(axis)) {
    auto trailingPosition = YGComputedEdgeValue(
        style_.position(), YGEdgeEnd, CompactValue::ofUndefined());
    if (!trailingPosition.isUndefined()) {
      return YGResolveValue(trailingPosition, axisSize);
    }
  }

  auto trailingPosition = YGComputedEdgeValue(
      style_.position(), trailing[axis], CompactValue::ofUndefined());

  return trailingPosition.isUndefined()
      ? YGFloatOptional{0}
      : YGResolveValue(trailingPosition, axisSize);
}

YGFloatOptional YGNode::getLeadingMargin(
    const YGFlexDirection axis,
    const float widthSize) const {
  if (YGFlexDirectionIsRow(axis) &&
      !style_.margin()[YGEdgeStart].isUndefined()) {
    return YGResolveValueMargin(style_.margin()[YGEdgeStart], widthSize);
  }

  return YGResolveValueMargin(
      YGComputedEdgeValue(
          style_.margin(), leading[axis], CompactValue::ofZero()),
      widthSize);
}

YGFloatOptional YGNode::getTrailingMargin(
    const YGFlexDirection axis,
    const float widthSize) const {
  if (YGFlexDirectionIsRow(axis) && !style_.margin()[YGEdgeEnd].isUndefined()) {
    return YGResolveValueMargin(style_.margin()[YGEdgeEnd], widthSize);
  }

  return YGResolveValueMargin(
      YGComputedEdgeValue(
          style_.margin(), trailing[axis], CompactValue::ofZero()),
      widthSize);
}

...


YGFloatOptional YGNode::getLeadingPadding(
    const YGFlexDirection axis,
    const float widthSize) const {
  const YGFloatOptional paddingEdgeStart =
      YGResolveValue(style_.padding()[YGEdgeStart], widthSize);
  if (YGFlexDirectionIsRow(axis) &&
      !style_.padding()[YGEdgeStart].isUndefined() &&
      !paddingEdgeStart.isUndefined() && paddingEdgeStart.unwrap() >= 0.0f) {
    return paddingEdgeStart;
  }

  YGFloatOptional resolvedValue = YGResolveValue(
      YGComputedEdgeValue(
          style_.padding(), leading[axis], CompactValue::ofZero()),
      widthSize);
  return YGFloatOptionalMax(resolvedValue, YGFloatOptional(0.0f));
}

YGFloatOptional YGNode::getTrailingPadding(
    const YGFlexDirection axis,
    const float widthSize) const {
  const YGFloatOptional paddingEdgeEnd =
      YGResolveValue(style_.padding()[YGEdgeEnd], widthSize);
  if (YGFlexDirectionIsRow(axis) && paddingEdgeEnd >= YGFloatOptional{0.0f}) {
    return paddingEdgeEnd;
  }

  YGFloatOptional resolvedValue = YGResolveValue(
      YGComputedEdgeValue(
          style_.padding(), trailing[axis], CompactValue::ofZero()),
      widthSize);

  return YGFloatOptionalMax(resolvedValue, YGFloatOptional(0.0f));
}

I don't understand this code exactly yet, but I think that this is where we could force the starting edge depending on the value of flexColumnDirection

[Sharcoux]

Or maybe just this in yoga.cpp:

static inline YGAlign YGNodeAlignItem(const YGNode* node, const YGNode* child) {
  const YGAlign align = child->getStyle().alignSelf() == YGAlignAuto
      ? node->getStyle().alignItems()
      : child->getStyle().alignSelf();
  if (align == YGAlignBaseline &&
      YGFlexDirectionIsColumn(node->getStyle().flexDirection())) {
    return YGAlignFlexStart;
  }
  return align;
}

I need to make some tests to understand how all this works...

[Sharcoux]

Would you think that it would be acceptable to just switch YGAlignFlexStart and YGAlignFlexEnd in YGNodeAlignItem when flex-direction is reversed?

[sahrens]

I'm not 100% sure yoga is the problem here - can you make a yoga unit test that demonstrates the problem and will enforce the fix?

[rozele]

This definitely isn't a Yoga problem. This is an implementation detail of browsers. One of two things is likely at play here:

1. The browser anchors to the "end" of the scroll when flex reverse settings are used.
2. DOM elements set overflow-anchor: auto by default, and items are rendered in order that they show up in the DOM tree, the scroll overflow will always be anchored to the first set of items rendered.

I recommend closing here and re-opening in react-native.