Skip to content

Commit

Permalink
Fix issue in gentest where border-<edge> would add a border to test (#…
Browse files Browse the repository at this point in the history 
…1496)

Summary:
Pull Request resolved: #1496

Gentest code has a problem where we try to apply a border in our test when the web browser is not actually adding one. This happens when we do something like `border-top: 10px`. This will actually set the style of the border to `initial` which is just `none`, so nothing renders. This is causing at least 1 test to pass when it actually fails.

I changed it so we ignore setting this value if the style is one of these values. I then re-ran the gentest code and excluded the now failing test (which gets fixed in my static stack).

Reviewed By: NickGerleman

Differential Revision: D51831754

fbshipit-source-id: a325e4a42b2d7cd6f19efc6cd5a2445574467fb7
  • Loading branch information
@joevilches @facebook-github-bot
joevilches authored and facebook-github-bot committed Dec 5, 2023
1 parent 7b3b66d commit c93734f
Show file tree
Hide file tree
Showing 6 changed files with 79 additions and 50 deletions.
2 changes: 1 addition & 1 deletion gentest/fixtures/YGAbsolutePositionTest.html
View file
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" data-disabled="true" 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
12 changes: 6 additions & 6 deletions gentest/fixtures/YGFlexDirectionTest.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -307,47 +307,47 @@

<div id="flex_direction_row_reverse_inner_border_left" style="height: 100px; width: 100px;">
<div style="height: 100px; width: 100px; flex-direction: row-reverse; ">
<div style="width: 10px; height: 10px; position: absolute; border-left: 10px"></div>
<div style="width: 10px; height: 10px; position: absolute; border-left: 10px solid black"></div>
<div style="width: 10px;"></div>
<div style="width: 10px;"></div>
</div>
</div>

<div id="flex_direction_row_reverse_inner_border_right" style="height: 100px; width: 100px;">
<div style="height: 100px; width: 100px; flex-direction: row-reverse; ">
<div style="width: 10px; height: 10px; position: absolute; border-right: 10px"></div>
<div style="width: 10px; height: 10px; position: absolute; border-right: 10px solid black"></div>
<div style="width: 10px;"></div>
<div style="width: 10px;"></div>
</div>
</div>

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

<div id="flex_direction_col_reverse_inner_border_bottom" style="height: 100px; width: 100px;">
<div style="height: 100px; width: 100px; flex-direction: column-reverse; ">
<div style="width: 10px; height: 10px; position: absolute; border-bottom: 10px"></div>
<div style="width: 10px; height: 10px; position: absolute; border-bottom: 10px solid black"></div>
<div style="width: 10px;"></div>
<div style="width: 10px;"></div>
</div>
</div>

<div id="flex_direction_row_reverse_inner_border_start" style="height: 100px; width: 100px;">
<div style="height: 100px; width: 100px; flex-direction: row-reverse; ">
<div style="width: 10px; height: 10px; position: absolute; border-start: 10px"></div>
<div style="width: 10px; height: 10px; position: absolute; border-start: 10px solid black"></div>
<div style="width: 10px;"></div>
<div style="width: 10px;"></div>
</div>
</div>

<div id="flex_direction_row_reverse_inner_border_end" style="height: 100px; width: 100px;">
<div style="height: 100px; width: 100px; flex-direction: row-reverse; ">
<div style="width: 10px; height: 10px; position: absolute; border-end: 10px"></div>
<div style="width: 10px; height: 10px; position: absolute; border-end: 10px solid black"></div>
<div style="width: 10px;"></div>
<div style="width: 10px;"></div>
</div>
Expand Down
86 changes: 56 additions & 30 deletions gentest/gentest.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@

const DEFAULT_EXPERIMENTS = ['AbsolutePercentageAgainstPaddingEdge'];

const INVISIBLE_BORDER_STYLES = ['none', 'initial'];

window.onload = function () {
checkDefaultValues();

Expand Down Expand Up @@ -432,49 +434,73 @@ function setupTestTree(
);
break;
case 'border-left-width':
if (genericNode.rawStyle.indexOf('border-start-width:') >= 0) {
e.YGNodeStyleSetBorder(
nodeName,
e.YGEdgeStart,
pointValue(e, node.style[style]),
);
} else {
e.YGNodeStyleSetBorder(
nodeName,
e.YGEdgeLeft,
pointValue(e, node.style[style]),
);
if (
!INVISIBLE_BORDER_STYLES.includes(
node.declaredStyle['border-left-style'],
)
) {
if (genericNode.rawStyle.indexOf('border-start-width:') >= 0) {
e.YGNodeStyleSetBorder(
nodeName,
e.YGEdgeStart,
pointValue(e, node.style[style]),
);
} else {
e.YGNodeStyleSetBorder(
nodeName,
e.YGEdgeLeft,
pointValue(e, node.style[style]),
);
}
}
break;
case 'border-top-width':
e.YGNodeStyleSetBorder(
nodeName,
e.YGEdgeTop,
pointValue(e, node.style[style]),
);
break;
case 'border-right-width':
if (genericNode.rawStyle.indexOf('border-end-width:') >= 0) {
if (
!INVISIBLE_BORDER_STYLES.includes(
node.declaredStyle['border-top-style'],
)
) {
e.YGNodeStyleSetBorder(
nodeName,
e.YGEdgeEnd,
e.YGEdgeTop,
pointValue(e, node.style[style]),
);
} else {
}
break;
case 'border-right-width':
if (
!INVISIBLE_BORDER_STYLES.includes(
node.declaredStyle['border-right-style'],
)
) {
if (genericNode.rawStyle.indexOf('border-end-width:') >= 0) {
e.YGNodeStyleSetBorder(
nodeName,
e.YGEdgeEnd,
pointValue(e, node.style[style]),
);
} else {
e.YGNodeStyleSetBorder(
nodeName,
e.YGEdgeRight,
pointValue(e, node.style[style]),
);
}
}
break;
case 'border-bottom-width':
if (
!INVISIBLE_BORDER_STYLES.includes(
node.declaredStyle['border-bottom-style'],
)
) {
e.YGNodeStyleSetBorder(
nodeName,
e.YGEdgeRight,
e.YGEdgeBottom,
pointValue(e, node.style[style]),
);
}
break;
case 'border-bottom-width':
e.YGNodeStyleSetBorder(
nodeName,
e.YGEdgeBottom,
pointValue(e, node.style[style]),
);
break;
case 'width':
e.YGNodeStyleSetWidth(nodeName, pointValue(e, node.style[style]));
break;
Expand Down
9 changes: 5 additions & 4 deletions java/tests/com/facebook/yoga/YGAbsolutePositionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1124,6 +1124,7 @@ public void test_percent_absolute_position_infinite_height() {
}

@Test
@Ignore
public void test_absolute_layout_percentage_height_based_on_padded_parent() {
YogaConfig config = YogaConfigFactory.create();
config.setExperimentalFeatureEnabled(YogaExperimentalFeature.ABSOLUTE_PERCENTAGE_AGAINST_PADDING_EDGE, true);
Expand All @@ -1149,9 +1150,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 +1163,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
10 changes: 5 additions & 5 deletions javascript/tests/generated/YGAbsolutePositionTest.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1255,7 +1255,7 @@ test('percent_absolute_position_infinite_height', () => {
config.free();
}
});
test('absolute_layout_percentage_height_based_on_padded_parent', () => {
test.skip('absolute_layout_percentage_height_based_on_padded_parent', () => {
const config = Yoga.Config.create();
let root;

Expand All @@ -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
10 changes: 6 additions & 4 deletions tests/generated/YGAbsolutePositionTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1132,6 +1132,8 @@ TEST(YogaTest, percent_absolute_position_infinite_height) {
}

TEST(YogaTest, absolute_layout_percentage_height_based_on_padded_parent) {
GTEST_SKIP();

const YGConfigRef config = YGConfigNew();
YGConfigSetExperimentalFeatureEnabled(config, YGExperimentalFeatureAbsolutePercentageAgainstPaddingEdge, true);

Expand All @@ -1155,9 +1157,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 +1169,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

0 comments on commit c93734f

Please sign in to comment.