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 (facebook#1485)

Summary:
Pull Request resolved: facebook#1485

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

Differential Revision: https://www.internalfb.com/diff/D51330526?entry_point=27

fbshipit-source-id: 891758632b33e88c176fa5b4023c3495849119f7
  • Loading branch information
Joe Vilches authored and facebook-github-bot committed Dec 5, 2023
1 parent 9a09d2f commit fd9657d
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 fd9657d

Please sign in to comment.