Skip to content

Commit

Permalink
Fix issue where percentages were off of the border box, not padding b…
Browse files Browse the repository at this point in the history
…ox (#1485)

Summary:

X-link: facebook/react-native#41686

The size of the containing block is the size of the padding box of the containing node for absolute nodes. We were looking at  `containingNode->getLayout().measuredDimension(Dimension::Width)` which is the border box. So we need to subtract the border from this.

Added a test that was failing before this change as well

Reviewed By: NickGerleman

Differential Revision: D51330526
  • Loading branch information
Joe Vilches authored and facebook-github-bot committed Dec 5, 2023
1 parent 6782204 commit 8d926d8
Show file tree
Hide file tree
Showing 11 changed files with 279 additions and 16 deletions.
2 changes: 1 addition & 1 deletion gentest/fixtures/YGAbsolutePositionTest.html
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@
<div style="width:20%; height:20%; left:20%; top:20%; position: absolute;"></div>
</div>

<div id="absolute_layout_percentage_height_based_on_padded_parent" style="flex-direction:column; width:100px; height:100px; padding-top: 10px; border-top: 10px;">
<div id="absolute_layout_percentage_height_based_on_padded_parent" style="flex-direction:column; width:100px; height:100px; padding-top: 10px; border-top: 10px solid black;">
<div style="width:100px; height:50%; position: absolute;"></div>
</div>

Expand Down
10 changes: 10 additions & 0 deletions gentest/fixtures/YGStaticPositionTest.html
Original file line number Diff line number Diff line change
Expand Up @@ -324,3 +324,13 @@
</div>
</div>
</div>

<div id="static_position_containing_block_padding_and_border">
<div
style="width: 400px; height: 400px; padding: 8px 1px 4px 9px; border-width: 5px 7px 4px 2px; position: relative">
<div style="height:100px; width: 100px; position: static">
<div style="height: 61%; width: 41%; position: absolute">
</div>
</div>
</div>
</div>
8 changes: 4 additions & 4 deletions java/tests/com/facebook/yoga/YGAbsolutePositionTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -1149,9 +1149,9 @@ public void test_absolute_layout_percentage_height_based_on_padded_parent() {
assertEquals(100f, root.getLayoutHeight(), 0.0f);

assertEquals(0f, root_child0.getLayoutX(), 0.0f);
assertEquals(10f, root_child0.getLayoutY(), 0.0f);
assertEquals(20f, root_child0.getLayoutY(), 0.0f);
assertEquals(100f, root_child0.getLayoutWidth(), 0.0f);
assertEquals(50f, root_child0.getLayoutHeight(), 0.0f);
assertEquals(45f, root_child0.getLayoutHeight(), 0.0f);

root.setDirection(YogaDirection.RTL);
root.calculateLayout(YogaConstants.UNDEFINED, YogaConstants.UNDEFINED);
Expand All @@ -1162,9 +1162,9 @@ public void test_absolute_layout_percentage_height_based_on_padded_parent() {
assertEquals(100f, root.getLayoutHeight(), 0.0f);

assertEquals(0f, root_child0.getLayoutX(), 0.0f);
assertEquals(10f, root_child0.getLayoutY(), 0.0f);
assertEquals(20f, root_child0.getLayoutY(), 0.0f);
assertEquals(100f, root_child0.getLayoutWidth(), 0.0f);
assertEquals(50f, root_child0.getLayoutHeight(), 0.0f);
assertEquals(45f, root_child0.getLayoutHeight(), 0.0f);
}

@Test
Expand Down
79 changes: 79 additions & 0 deletions java/tests/com/facebook/yoga/YGStaticPositionTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -2653,6 +2653,85 @@ public void test_static_position_static_child_containing_block_content_box() {
assertEquals(50f, root_child0_child0.getLayoutHeight(), 0.0f);
}

@Test
public void test_static_position_containing_block_padding_and_border() {
YogaConfig config = YogaConfigFactory.create();
config.setExperimentalFeatureEnabled(YogaExperimentalFeature.ABSOLUTE_PERCENTAGE_AGAINST_PADDING_EDGE, true);

final YogaNode root = createNode(config);
root.setPositionType(YogaPositionType.ABSOLUTE);

final YogaNode root_child0 = createNode(config);
root_child0.setPositionType(YogaPositionType.RELATIVE);
root_child0.setPadding(YogaEdge.LEFT, 9);
root_child0.setPadding(YogaEdge.TOP, 8);
root_child0.setPadding(YogaEdge.RIGHT, 1);
root_child0.setPadding(YogaEdge.BOTTOM, 4);
root_child0.setBorder(YogaEdge.LEFT, 2f);
root_child0.setBorder(YogaEdge.TOP, 5f);
root_child0.setBorder(YogaEdge.RIGHT, 7f);
root_child0.setBorder(YogaEdge.BOTTOM, 4f);
root_child0.setWidth(400f);
root_child0.setHeight(400f);
root.addChildAt(root_child0, 0);

final YogaNode root_child0_child0 = createNode(config);
root_child0_child0.setWidth(100f);
root_child0_child0.setHeight(100f);
root_child0.addChildAt(root_child0_child0, 0);

final YogaNode root_child0_child0_child0 = createNode(config);
root_child0_child0_child0.setPositionType(YogaPositionType.ABSOLUTE);
root_child0_child0_child0.setWidthPercent(41f);
root_child0_child0_child0.setHeightPercent(61f);
root_child0_child0.addChildAt(root_child0_child0_child0, 0);
root.setDirection(YogaDirection.LTR);
root.calculateLayout(YogaConstants.UNDEFINED, YogaConstants.UNDEFINED);

assertEquals(0f, root.getLayoutX(), 0.0f);
assertEquals(0f, root.getLayoutY(), 0.0f);
assertEquals(400f, root.getLayoutWidth(), 0.0f);
assertEquals(400f, root.getLayoutHeight(), 0.0f);

assertEquals(0f, root_child0.getLayoutX(), 0.0f);
assertEquals(0f, root_child0.getLayoutY(), 0.0f);
assertEquals(400f, root_child0.getLayoutWidth(), 0.0f);
assertEquals(400f, root_child0.getLayoutHeight(), 0.0f);

assertEquals(11f, root_child0_child0.getLayoutX(), 0.0f);
assertEquals(13f, root_child0_child0.getLayoutY(), 0.0f);
assertEquals(100f, root_child0_child0.getLayoutWidth(), 0.0f);
assertEquals(100f, root_child0_child0.getLayoutHeight(), 0.0f);

assertEquals(0f, root_child0_child0_child0.getLayoutX(), 0.0f);
assertEquals(0f, root_child0_child0_child0.getLayoutY(), 0.0f);
assertEquals(160f, root_child0_child0_child0.getLayoutWidth(), 0.0f);
assertEquals(239f, root_child0_child0_child0.getLayoutHeight(), 0.0f);

root.setDirection(YogaDirection.RTL);
root.calculateLayout(YogaConstants.UNDEFINED, YogaConstants.UNDEFINED);

assertEquals(0f, root.getLayoutX(), 0.0f);
assertEquals(0f, root.getLayoutY(), 0.0f);
assertEquals(400f, root.getLayoutWidth(), 0.0f);
assertEquals(400f, root.getLayoutHeight(), 0.0f);

assertEquals(0f, root_child0.getLayoutX(), 0.0f);
assertEquals(0f, root_child0.getLayoutY(), 0.0f);
assertEquals(400f, root_child0.getLayoutWidth(), 0.0f);
assertEquals(400f, root_child0.getLayoutHeight(), 0.0f);

assertEquals(292f, root_child0_child0.getLayoutX(), 0.0f);
assertEquals(13f, root_child0_child0.getLayoutY(), 0.0f);
assertEquals(100f, root_child0_child0.getLayoutWidth(), 0.0f);
assertEquals(100f, root_child0_child0.getLayoutHeight(), 0.0f);

assertEquals(-60f, root_child0_child0_child0.getLayoutX(), 0.0f);
assertEquals(0f, root_child0_child0_child0.getLayoutY(), 0.0f);
assertEquals(160f, root_child0_child0_child0.getLayoutWidth(), 0.0f);
assertEquals(239f, root_child0_child0_child0.getLayoutHeight(), 0.0f);
}

private YogaNode createNode(YogaConfig config) {
return mNodeFactory.create(config);
}
Expand Down
8 changes: 4 additions & 4 deletions javascript/tests/generated/YGAbsolutePositionTest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1282,9 +1282,9 @@ test('absolute_layout_percentage_height_based_on_padded_parent', () => {
expect(root.getComputedHeight()).toBe(100);

expect(root_child0.getComputedLeft()).toBe(0);
expect(root_child0.getComputedTop()).toBe(10);
expect(root_child0.getComputedTop()).toBe(20);
expect(root_child0.getComputedWidth()).toBe(100);
expect(root_child0.getComputedHeight()).toBe(50);
expect(root_child0.getComputedHeight()).toBe(45);

root.calculateLayout(undefined, undefined, Direction.RTL);

Expand All @@ -1294,9 +1294,9 @@ test('absolute_layout_percentage_height_based_on_padded_parent', () => {
expect(root.getComputedHeight()).toBe(100);

expect(root_child0.getComputedLeft()).toBe(0);
expect(root_child0.getComputedTop()).toBe(10);
expect(root_child0.getComputedTop()).toBe(20);
expect(root_child0.getComputedWidth()).toBe(100);
expect(root_child0.getComputedHeight()).toBe(50);
expect(root_child0.getComputedHeight()).toBe(45);
} finally {
if (typeof root !== 'undefined') {
root.freeRecursive();
Expand Down
85 changes: 85 additions & 0 deletions javascript/tests/generated/YGStaticPositionTest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2874,3 +2874,88 @@ test('static_position_static_child_containing_block_content_box', () => {
config.free();
}
});
test('static_position_containing_block_padding_and_border', () => {
const config = Yoga.Config.create();
let root;

config.setExperimentalFeatureEnabled(ExperimentalFeature.AbsolutePercentageAgainstPaddingEdge, true);

try {
root = Yoga.Node.create(config);
root.setPositionType(PositionType.Absolute);

const root_child0 = Yoga.Node.create(config);
root_child0.setPositionType(PositionType.Relative);
root_child0.setPadding(Edge.Left, 9);
root_child0.setPadding(Edge.Top, 8);
root_child0.setPadding(Edge.Right, 1);
root_child0.setPadding(Edge.Bottom, 4);
root_child0.setBorder(Edge.Left, 2);
root_child0.setBorder(Edge.Top, 5);
root_child0.setBorder(Edge.Right, 7);
root_child0.setBorder(Edge.Bottom, 4);
root_child0.setWidth(400);
root_child0.setHeight(400);
root.insertChild(root_child0, 0);

const root_child0_child0 = Yoga.Node.create(config);
root_child0_child0.setWidth(100);
root_child0_child0.setHeight(100);
root_child0.insertChild(root_child0_child0, 0);

const root_child0_child0_child0 = Yoga.Node.create(config);
root_child0_child0_child0.setPositionType(PositionType.Absolute);
root_child0_child0_child0.setWidth("41%");
root_child0_child0_child0.setHeight("61%");
root_child0_child0.insertChild(root_child0_child0_child0, 0);
root.calculateLayout(undefined, undefined, Direction.LTR);

expect(root.getComputedLeft()).toBe(0);
expect(root.getComputedTop()).toBe(0);
expect(root.getComputedWidth()).toBe(400);
expect(root.getComputedHeight()).toBe(400);

expect(root_child0.getComputedLeft()).toBe(0);
expect(root_child0.getComputedTop()).toBe(0);
expect(root_child0.getComputedWidth()).toBe(400);
expect(root_child0.getComputedHeight()).toBe(400);

expect(root_child0_child0.getComputedLeft()).toBe(11);
expect(root_child0_child0.getComputedTop()).toBe(13);
expect(root_child0_child0.getComputedWidth()).toBe(100);
expect(root_child0_child0.getComputedHeight()).toBe(100);

expect(root_child0_child0_child0.getComputedLeft()).toBe(0);
expect(root_child0_child0_child0.getComputedTop()).toBe(0);
expect(root_child0_child0_child0.getComputedWidth()).toBe(160);
expect(root_child0_child0_child0.getComputedHeight()).toBe(239);

root.calculateLayout(undefined, undefined, Direction.RTL);

expect(root.getComputedLeft()).toBe(0);
expect(root.getComputedTop()).toBe(0);
expect(root.getComputedWidth()).toBe(400);
expect(root.getComputedHeight()).toBe(400);

expect(root_child0.getComputedLeft()).toBe(0);
expect(root_child0.getComputedTop()).toBe(0);
expect(root_child0.getComputedWidth()).toBe(400);
expect(root_child0.getComputedHeight()).toBe(400);

expect(root_child0_child0.getComputedLeft()).toBe(292);
expect(root_child0_child0.getComputedTop()).toBe(13);
expect(root_child0_child0.getComputedWidth()).toBe(100);
expect(root_child0_child0.getComputedHeight()).toBe(100);

expect(root_child0_child0_child0.getComputedLeft()).toBe(-60);
expect(root_child0_child0_child0.getComputedTop()).toBe(0);
expect(root_child0_child0_child0.getComputedWidth()).toBe(160);
expect(root_child0_child0_child0.getComputedHeight()).toBe(239);
} finally {
if (typeof root !== 'undefined') {
root.freeRecursive();
}

config.free();
}
});
8 changes: 4 additions & 4 deletions tests/generated/YGAbsolutePositionTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1155,9 +1155,9 @@ TEST(YogaTest, absolute_layout_percentage_height_based_on_padded_parent) {
ASSERT_FLOAT_EQ(100, YGNodeLayoutGetHeight(root));

ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root_child0));
ASSERT_FLOAT_EQ(10, YGNodeLayoutGetTop(root_child0));
ASSERT_FLOAT_EQ(20, YGNodeLayoutGetTop(root_child0));
ASSERT_FLOAT_EQ(100, YGNodeLayoutGetWidth(root_child0));
ASSERT_FLOAT_EQ(50, YGNodeLayoutGetHeight(root_child0));
ASSERT_FLOAT_EQ(45, YGNodeLayoutGetHeight(root_child0));

YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionRTL);

Expand All @@ -1167,9 +1167,9 @@ TEST(YogaTest, absolute_layout_percentage_height_based_on_padded_parent) {
ASSERT_FLOAT_EQ(100, YGNodeLayoutGetHeight(root));

ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root_child0));
ASSERT_FLOAT_EQ(10, YGNodeLayoutGetTop(root_child0));
ASSERT_FLOAT_EQ(20, YGNodeLayoutGetTop(root_child0));
ASSERT_FLOAT_EQ(100, YGNodeLayoutGetWidth(root_child0));
ASSERT_FLOAT_EQ(50, YGNodeLayoutGetHeight(root_child0));
ASSERT_FLOAT_EQ(45, YGNodeLayoutGetHeight(root_child0));

YGNodeFreeRecursive(root);

Expand Down
80 changes: 80 additions & 0 deletions tests/generated/YGStaticPositionTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2676,3 +2676,83 @@ TEST(YogaTest, static_position_static_child_containing_block_content_box) {

YGConfigFree(config);
}

TEST(YogaTest, static_position_containing_block_padding_and_border) {
const YGConfigRef config = YGConfigNew();
YGConfigSetExperimentalFeatureEnabled(config, YGExperimentalFeatureAbsolutePercentageAgainstPaddingEdge, true);

const YGNodeRef root = YGNodeNewWithConfig(config);
YGNodeStyleSetPositionType(root, YGPositionTypeAbsolute);

const YGNodeRef root_child0 = YGNodeNewWithConfig(config);
YGNodeStyleSetPositionType(root_child0, YGPositionTypeRelative);
YGNodeStyleSetPadding(root_child0, YGEdgeLeft, 9);
YGNodeStyleSetPadding(root_child0, YGEdgeTop, 8);
YGNodeStyleSetPadding(root_child0, YGEdgeRight, 1);
YGNodeStyleSetPadding(root_child0, YGEdgeBottom, 4);
YGNodeStyleSetBorder(root_child0, YGEdgeLeft, 2);
YGNodeStyleSetBorder(root_child0, YGEdgeTop, 5);
YGNodeStyleSetBorder(root_child0, YGEdgeRight, 7);
YGNodeStyleSetBorder(root_child0, YGEdgeBottom, 4);
YGNodeStyleSetWidth(root_child0, 400);
YGNodeStyleSetHeight(root_child0, 400);
YGNodeInsertChild(root, root_child0, 0);

const YGNodeRef root_child0_child0 = YGNodeNewWithConfig(config);
YGNodeStyleSetWidth(root_child0_child0, 100);
YGNodeStyleSetHeight(root_child0_child0, 100);
YGNodeInsertChild(root_child0, root_child0_child0, 0);

const YGNodeRef root_child0_child0_child0 = YGNodeNewWithConfig(config);
YGNodeStyleSetPositionType(root_child0_child0_child0, YGPositionTypeAbsolute);
YGNodeStyleSetWidthPercent(root_child0_child0_child0, 41);
YGNodeStyleSetHeightPercent(root_child0_child0_child0, 61);
YGNodeInsertChild(root_child0_child0, root_child0_child0_child0, 0);
YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR);

ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root));
ASSERT_FLOAT_EQ(400, YGNodeLayoutGetWidth(root));
ASSERT_FLOAT_EQ(400, YGNodeLayoutGetHeight(root));

ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root_child0));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child0));
ASSERT_FLOAT_EQ(400, YGNodeLayoutGetWidth(root_child0));
ASSERT_FLOAT_EQ(400, YGNodeLayoutGetHeight(root_child0));

ASSERT_FLOAT_EQ(11, YGNodeLayoutGetLeft(root_child0_child0));
ASSERT_FLOAT_EQ(13, YGNodeLayoutGetTop(root_child0_child0));
ASSERT_FLOAT_EQ(100, YGNodeLayoutGetWidth(root_child0_child0));
ASSERT_FLOAT_EQ(100, YGNodeLayoutGetHeight(root_child0_child0));

ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root_child0_child0_child0));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child0_child0_child0));
ASSERT_FLOAT_EQ(160, YGNodeLayoutGetWidth(root_child0_child0_child0));
ASSERT_FLOAT_EQ(239, YGNodeLayoutGetHeight(root_child0_child0_child0));

YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionRTL);

ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root));
ASSERT_FLOAT_EQ(400, YGNodeLayoutGetWidth(root));
ASSERT_FLOAT_EQ(400, YGNodeLayoutGetHeight(root));

ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root_child0));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child0));
ASSERT_FLOAT_EQ(400, YGNodeLayoutGetWidth(root_child0));
ASSERT_FLOAT_EQ(400, YGNodeLayoutGetHeight(root_child0));

ASSERT_FLOAT_EQ(292, YGNodeLayoutGetLeft(root_child0_child0));
ASSERT_FLOAT_EQ(13, YGNodeLayoutGetTop(root_child0_child0));
ASSERT_FLOAT_EQ(100, YGNodeLayoutGetWidth(root_child0_child0));
ASSERT_FLOAT_EQ(100, YGNodeLayoutGetHeight(root_child0_child0));

ASSERT_FLOAT_EQ(-60, YGNodeLayoutGetLeft(root_child0_child0_child0));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child0_child0_child0));
ASSERT_FLOAT_EQ(160, YGNodeLayoutGetWidth(root_child0_child0_child0));
ASSERT_FLOAT_EQ(239, YGNodeLayoutGetHeight(root_child0_child0_child0));

YGNodeFreeRecursive(root);

YGConfigFree(config);
}
9 changes: 6 additions & 3 deletions yoga/algorithm/AbsoluteLayout.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ static void justifyAbsoluteChild(
case Justify::SpaceBetween:
child->setLayoutPosition(
child->getFlexStartMargin(mainAxis, direction, containingBlockWidth) +
parent->getFlexStartBorder(mainAxis, direction),
parent->getLayout().border(flexStartEdge(mainAxis)) +
parent->getLayout().padding(flexStartEdge(mainAxis)),
flexStartEdge(mainAxis));
break;
case Justify::FlexEnd:
Expand Down Expand Up @@ -458,8 +459,10 @@ void layoutAbsoluteDescendants(
containingNode,
currentNode,
child,
containingNode->getLayout().measuredDimension(Dimension::Width),
containingNode->getLayout().measuredDimension(Dimension::Height),
containingNode->getLayout().measuredDimension(Dimension::Width) -
containingNode->getBorderForAxis(FlexDirection::Row),
containingNode->getLayout().measuredDimension(Dimension::Height) -
containingNode->getBorderForAxis(FlexDirection::Column),
widthSizingMode,
currentNodeDirection,
layoutMarkerData,
Expand Down
5 changes: 5 additions & 0 deletions yoga/node/Node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,11 @@ float Node::getFlexEndPaddingAndBorder(
getFlexEndBorder(axis, direction);
}

float Node::getBorderForAxis(FlexDirection axis) const {
return getInlineStartBorder(axis, Direction::LTR) +
getInlineEndBorder(axis, Direction::LTR);
}

float Node::getMarginForAxis(FlexDirection axis, float widthSize) const {
// The total margin for a given axis does not depend on the direction
// so hardcoding LTR here to avoid piping direction to this function
Expand Down
1 change: 1 addition & 0 deletions yoga/node/Node.h
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,7 @@ class YG_EXPORT Node : public ::YGNode {
FlexDirection axis,
Direction direction,
float widthSize) const;
float getBorderForAxis(FlexDirection axis) const;
float getMarginForAxis(FlexDirection axis, float widthSize) const;
float getGapForAxis(FlexDirection axis) const;
// Setters
Expand Down

0 comments on commit 8d926d8

Please sign in to comment.