Skip to content

Commit

Permalink
Fix mismatched cases of inlineStart/End and flexStart/End (#1561)
Browse files Browse the repository at this point in the history 
Summary:
Pull Request resolved: #1561

Back when I introduced the inline functions that would get the edge according to the writing direction I swapped some instances of `setLayoutPosition` which wrote to the flexStart edge erroneously. We should basically never read from some inline style and write to the flex edge. This changes them all to use the flex values.

Reviewed By: NickGerleman

Differential Revision: D52921401

fbshipit-source-id: 92b74d652018596134c91827806272ed7418ef6c
  • Loading branch information
@joevilches @facebook-github-bot
joevilches authored and facebook-github-bot committed Jan 22, 2024
1 parent 67154d4 commit 8fe38fc
Show file tree
Hide file tree
Showing 9 changed files with 623 additions and 64 deletions.
60 changes: 41 additions & 19 deletions gentest/fixtures/YGAbsolutePositionTest.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,28 +14,33 @@
<div style="width:10px; height: 10px; position: absolute; start: 10px; top: 10px; end: 10px; bottom: 10px;"></div>
</div>

<div id="do_not_clamp_height_of_absolute_node_to_height_of_its_overflow_hidden_parent" style="height: 50px; width: 50px; overflow: hidden; flex-direction: row;">
<div id="do_not_clamp_height_of_absolute_node_to_height_of_its_overflow_hidden_parent"
style="height: 50px; width: 50px; overflow: hidden; flex-direction: row;">
<div style="position: absolute; start: 0px; top: 0px;">
<div style="width: 100px; height: 100px;"></div>
</div>
</div>

<div id="absolute_layout_within_border" style="height:100px; width:100px; border-width: 10px; margin: 10px; padding: 10px;">
<div id="absolute_layout_within_border"
style="height:100px; width:100px; border-width: 10px; margin: 10px; padding: 10px;">
<div style="position: absolute; width: 50px; height: 50px; left: 0px; top: 0px;"></div>
<div style="position: absolute; width: 50px; height: 50px; right: 0px; bottom: 0px;"></div>
<div style="position: absolute; width: 50px; height: 50px; left: 0px; top: 0px; margin: 10px;"></div>
<div style="position: absolute; width: 50px; height: 50px; right: 0px; bottom: 0px; margin: 10px;"></div>
</div>

<div id="absolute_layout_align_items_and_justify_content_center" style="height: 100px; width: 110px; flex-grow: 1; align-items: center; justify-content: center;">
<div id="absolute_layout_align_items_and_justify_content_center"
style="height: 100px; width: 110px; flex-grow: 1; align-items: center; justify-content: center;">
<div style="position: absolute; width: 60px; height: 40px;"></div>
</div>

<div id="absolute_layout_align_items_and_justify_content_flex_end" style="height: 100px; width: 110px; flex-grow: 1; align-items: flex-end; justify-content: flex-end;">
<div id="absolute_layout_align_items_and_justify_content_flex_end"
style="height: 100px; width: 110px; flex-grow: 1; align-items: flex-end; justify-content: flex-end;">
<div style="position: absolute; width: 60px; height: 40px;"></div>
</div>

<div id="absolute_layout_justify_content_center" style="height: 100px; width: 110px; flex-grow: 1; justify-content: center;">
<div id="absolute_layout_justify_content_center"
style="height: 100px; width: 110px; flex-grow: 1; justify-content: center;">
<div style="position: absolute; width: 60px; height: 40px;"></div>
</div>

Expand All @@ -47,44 +52,52 @@
<div style="position: absolute; width: 60px; height: 40px;align-self: center;"></div>
</div>

<div id="absolute_layout_align_items_and_justify_content_center_and_top_position" style="height: 100px; width: 110px; flex-grow: 1; align-items: center; justify-content: center;">
<div id="absolute_layout_align_items_and_justify_content_center_and_top_position"
style="height: 100px; width: 110px; flex-grow: 1; align-items: center; justify-content: center;">
<div style="position: absolute; width: 60px; height: 40px;top:10px;"></div>
</div>

<div id="absolute_layout_align_items_and_justify_content_center_and_bottom_position" style="height: 100px; width: 110px; flex-grow: 1; align-items: center; justify-content: center;">
<div id="absolute_layout_align_items_and_justify_content_center_and_bottom_position"
style="height: 100px; width: 110px; flex-grow: 1; align-items: center; justify-content: center;">
<div style="position: absolute; width: 60px; height: 40px;bottom:10px;"></div>
</div>

<div id="absolute_layout_align_items_and_justify_content_center_and_left_position" style="height: 100px; width: 110px; flex-grow: 1; align-items: center; justify-content: center;">
<div id="absolute_layout_align_items_and_justify_content_center_and_left_position"
style="height: 100px; width: 110px; flex-grow: 1; align-items: center; justify-content: center;">
<div style="position: absolute; width: 60px; height: 40px;left:5px;"></div>
</div>

<div id="absolute_layout_align_items_and_justify_content_center_and_right_position" style="height: 100px; width: 110px; flex-grow: 1; align-items: center; justify-content: center;">
<div id="absolute_layout_align_items_and_justify_content_center_and_right_position"
style="height: 100px; width: 110px; flex-grow: 1; align-items: center; justify-content: center;">
<div style="position: absolute; width: 60px; height: 40px;right:5px;"></div>
</div>

<div id="position_root_with_rtl_should_position_withoutdirection" style="height: 52px; width: 52px; left: 72px; ">
</div>

<div id="absolute_layout_percentage_bottom_based_on_parent_height" style="width: 100px; height: 200px;">
<div style="position: absolute; top: 50%; width: 10px; height: 10px;"></div>
<div style="position: absolute; bottom: 50%; width: 10px; height: 10px;"></div>
<div style="position: absolute; top: 10%; width: 10px; bottom: 10%;"></div>
<div style="position: absolute; top: 50%; width: 10px; height: 10px;"></div>
<div style="position: absolute; bottom: 50%; width: 10px; height: 10px;"></div>
<div style="position: absolute; top: 10%; width: 10px; bottom: 10%;"></div>
</div>

<div id="absolute_layout_in_wrap_reverse_column_container" style="flex-direction:column; width:100px; height:100px; flex-wrap: wrap-reverse;">
<div id="absolute_layout_in_wrap_reverse_column_container"
style="flex-direction:column; width:100px; height:100px; flex-wrap: wrap-reverse;">
<div style="width:20px; height:20px; position: absolute;"></div>
</div>

<div id="absolute_layout_in_wrap_reverse_row_container" style="flex-direction:row; width:100px; height:100px; flex-wrap: wrap-reverse;">
<div id="absolute_layout_in_wrap_reverse_row_container"
style="flex-direction:row; width:100px; height:100px; flex-wrap: wrap-reverse;">
<div style="width:20px; height:20px; position: absolute;"></div>
</div>

<div id="absolute_layout_in_wrap_reverse_column_container_flex_end" style="flex-direction:column; width:100px; height:100px; flex-wrap: wrap-reverse;">
<div id="absolute_layout_in_wrap_reverse_column_container_flex_end"
style="flex-direction:column; width:100px; height:100px; flex-wrap: wrap-reverse;">
<div style="width:20px; height:20px; position: absolute; align-self: flex-end;"></div>
</div>

<div id="absolute_layout_in_wrap_reverse_row_container_flex_end" style="flex-direction:row; width:100px; height:100px; flex-wrap: wrap-reverse;">
<div id="absolute_layout_in_wrap_reverse_row_container_flex_end"
style="flex-direction:row; width:100px; height:100px; flex-wrap: wrap-reverse;">
<div style="width:20px; height:20px; position: absolute; align-self: flex-end;"></div>
</div>

Expand All @@ -93,12 +106,14 @@
<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 solid black;">
<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>

<div id="absolute_layout_percentage_height_based_on_padded_parent_and_align_items_center" style="position: relative; flex-direction:column; align-items:center; justify-content:center; width:100px; height:100px; padding-top:20px; padding-bottom:20px; ">
<div style="position:absolute; width:100px; height:50%;"></div>
<div id="absolute_layout_percentage_height_based_on_padded_parent_and_align_items_center"
style="position: relative; flex-direction:column; align-items:center; justify-content:center; width:100px; height:100px; padding-top:20px; padding-bottom:20px; ">
<div style="position:absolute; width:100px; height:50%;"></div>
</div>

<div id="absolute_layout_padding_left" style="width:200px; height:200px; padding-left:100px;">
Expand All @@ -116,3 +131,10 @@
<div id="absolute_layout_padding_bottom" style="width:200px; height:200px; padding-bottom:100px;">
<div style="position:absolute; width:50px; height:50px;"></div>
</div>

<div id="absolute_layout_column_reverse_margin_border"
style="width:200px; height:200px; flex-direction: column-reverse;">
<div
style="position:absolute; width:50px; height:50px; left: 5px; right: 3px; margin-right: 4px; margin-left: 3px; border-right: 7px solid black; border-left: 1px solid black">
</div>
</div>
61 changes: 46 additions & 15 deletions gentest/fixtures/YGAlignItemsTest.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@
</div>
</div>

<div id="align_baseline_child_multiline" style="width: 100px; height: 100px; flex-direction:row; align-items: baseline;">
<div id="align_baseline_child_multiline"
style="width: 100px; height: 100px; flex-direction:row; align-items: baseline;">
<div style="width: 50px; height: 60px;"></div>
<div style="width: 50px; height: 25px;flex-wrap:wrap;flex-direction:row;">
<div style="width: 25px; height: 20px;"></div>
Expand All @@ -36,7 +37,8 @@
</div>
</div>

<div id="align_baseline_child_multiline_override" style="width: 100px; height: 100px; flex-direction:row; align-items: baseline;">
<div id="align_baseline_child_multiline_override"
style="width: 100px; height: 100px; flex-direction:row; align-items: baseline;">
<div style="width: 50px; height: 60px;"></div>
<div style="width: 50px; height: 25px;flex-wrap:wrap;flex-direction:row;">
<div style="width: 25px; height: 20px;"></div>
Expand All @@ -46,7 +48,8 @@
</div>
</div>

<div id="align_baseline_child_multiline_no_override_on_secondline" style="width: 100px; height: 100px; flex-direction:row; align-items: baseline;">
<div id="align_baseline_child_multiline_no_override_on_secondline"
style="width: 100px; height: 100px; flex-direction:row; align-items: baseline;">
<div style="width: 50px; height: 60px;"></div>
<div style="width: 50px; height: 25px;flex-wrap:wrap;flex-direction:row;">
<div style="width: 25px; height: 20px;"></div>
Expand All @@ -71,7 +74,8 @@
</div>
</div>

<div id="align_baseline_double_nested_child" style="width: 100px; height: 100px; flex-direction:row; align-items: baseline;">
<div id="align_baseline_double_nested_child"
style="width: 100px; height: 100px; flex-direction:row; align-items: baseline;">
<div style="width: 50px; height: 50px;">
<div style="width: 50px; height: 20px;"></div>
</div>
Expand All @@ -93,14 +97,16 @@
</div>
</div>

<div id="align_baseline_child_padding" style="width: 100px; height: 100px; padding:5px; flex-direction:row; align-items: baseline;">
<div id="align_baseline_child_padding"
style="width: 100px; height: 100px; padding:5px; flex-direction:row; align-items: baseline;">
<div style="width: 50px; height: 50px;"></div>
<div style="width: 50px; height: 20px;padding:5px;">
<div style="width: 50px; height: 10px;"></div>
</div>
</div>

<div id="align_baseline_multiline" style="width: 100px; height: 100px; flex-direction:row; align-items: baseline;flex-wrap:wrap;">
<div id="align_baseline_multiline"
style="width: 100px; height: 100px; flex-direction:row; align-items: baseline;flex-wrap:wrap;">
<div style="width: 50px; height: 50px;"></div>
<div style="width: 50px; height: 20px;">
<div style="width: 50px; height: 10px;"></div>
Expand All @@ -112,7 +118,8 @@
</div>

<!-- TODO: Yoga's behavior is no longer in line with Chromium -->
<div id="align_baseline_multiline_column" data-disabled="true" style="width: 100px; height: 100px; flex-direction:column; align-items: baseline;flex-wrap:wrap;">
<div id="align_baseline_multiline_column" data-disabled="true"
style="width: 100px; height: 100px; flex-direction:column; align-items: baseline;flex-wrap:wrap;">
<div style="width: 50px; height: 50px;"></div>
<div style="width: 30px; height: 50px;">
<div style="width: 20px; height: 20px;"></div>
Expand All @@ -124,7 +131,8 @@
</div>

<!-- TODO: Yoga's behavior is no longer in line with Chromium -->
<div id="align_baseline_multiline_column2" data-disabled="true" style="width: 100px; height: 100px; flex-direction:column; align-items: baseline;flex-wrap:wrap;">
<div id="align_baseline_multiline_column2" data-disabled="true"
style="width: 100px; height: 100px; flex-direction:column; align-items: baseline;flex-wrap:wrap;">
<div style="width: 50px; height: 50px;flex-direction:column;"></div>
<div style="width: 30px; height: 50px;flex-direction:column;">
<div style="width: 20px; height: 20px;flex-direction:column;"></div>
Expand All @@ -135,7 +143,8 @@
<div style="width: 50px; height: 20px;flex-direction:column;"></div>
</div>

<div id="align_baseline_multiline_row_and_column" style="width: 100px; height: 100px; flex-direction:row; align-items: baseline;flex-wrap:wrap;">
<div id="align_baseline_multiline_row_and_column"
style="width: 100px; height: 100px; flex-direction:row; align-items: baseline;flex-wrap:wrap;">
<div style="width: 50px; height: 50px;"></div>
<div style="width: 50px; height: 50px;flex-direction:column;">
<div style="width: 50px; height: 10px;"></div>
Expand All @@ -146,39 +155,45 @@
<div style="width: 50px; height: 20px;"></div>
</div>

<div id="align_items_center_child_with_margin_bigger_than_parent" style="height: 52px; width: 52px; align-items: center; justify-content: center;">
<div id="align_items_center_child_with_margin_bigger_than_parent"
style="height: 52px; width: 52px; align-items: center; justify-content: center;">
<div style="align-items: center;">
<div style="width: 52px; height: 52px; margin-left: 10px; margin-right: 10px;"></div>
</div>
</div>

<div id="align_items_flex_end_child_with_margin_bigger_than_parent" style="height: 52px; width: 52px; align-items: center; justify-content: center;">
<div id="align_items_flex_end_child_with_margin_bigger_than_parent"
style="height: 52px; width: 52px; align-items: center; justify-content: center;">
<div style="align-items: flex-end;">
<div style="width: 52px; height: 52px; margin-left: 10px; margin-right: 10px;"></div>
</div>
</div>

<div id="align_items_center_child_without_margin_bigger_than_parent" style="height: 52px; width: 52px; align-items: center; justify-content: center;">
<div id="align_items_center_child_without_margin_bigger_than_parent"
style="height: 52px; width: 52px; align-items: center; justify-content: center;">
<div style="align-items: center;">
<div style="width: 72px; height: 72px;"></div>
</div>
</div>

<div id="align_items_flex_end_child_without_margin_bigger_than_parent" style="height: 52px; width: 52px; align-items: center; justify-content: center;">
<div id="align_items_flex_end_child_without_margin_bigger_than_parent"
style="height: 52px; width: 52px; align-items: center; justify-content: center;">
<div style="align-items: flex-end;">
<div style="width: 72px; height: 72px;"></div>
</div>
</div>

<div id="align_center_should_size_based_on_content" style="width: 100px; height: 100px; align-items: center; margin-top: 20px;">
<div id="align_center_should_size_based_on_content"
style="width: 100px; height: 100px; align-items: center; margin-top: 20px;">
<div style="flex-grow: 0; flex-shrink: 1; justify-content: center;">
<div style="flex-grow: 1; flex-shrink: 1;">
<div style="width: 20px; height: 20px;"></div>
</div>
</div>
</div>

<div id="align_stretch_should_size_based_on_parent" style="width: 100px; height: 100px; align-items: stretch; margin-top: 20px;">
<div id="align_stretch_should_size_based_on_parent"
style="width: 100px; height: 100px; align-items: stretch; margin-top: 20px;">
<div style="flex-grow: 0; flex-shrink: 1; justify-content: center;">
<div style="flex-grow: 1; flex-shrink: 1;">
<div style="width: 20px; height: 20px;"></div>
Expand Down Expand Up @@ -209,3 +224,19 @@
</div>
</div>
</div>

<div id="align_flex_end_with_row_reverse"
style="width: 100px; height: 75px; align-items: flex-end; flex-direction: column; flex-wrap: wrap;">
<div style="position:relative; width:50px; height:50px; margin-right: 5px; margin-left: 3px;">
</div>
<div style="position:relative; width:50px; height:50px;">
</div>
</div>

<div id="align_stretch_with_row_reverse"
style="width: 100px; height: 75px; align-items: stretch; flex-direction: column; flex-wrap: wrap;">
<div style="position:relative; width:50px; height:50px; margin-right: 5px; margin-left: 3px;">
</div>
<div style="position:relative; width:50px; height:50px;">
</div>
</div>
50 changes: 49 additions & 1 deletion java/tests/com/facebook/yoga/YGAbsolutePositionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<655bad05f0830b5ea39b80b01b0e5e9c>>
* @generated SignedSource<<ff8c3bfd84ae0fa4cc6ce4b728200f82>>
* generated by gentest/gentest-driver.ts from gentest/fixtures/YGAbsolutePositionTest.html
*/

Expand Down Expand Up @@ -1357,6 +1357,54 @@ public void test_absolute_layout_padding_bottom() {
assertEquals(50f, root_child0.getLayoutHeight(), 0.0f);
}

@Test
public void test_absolute_layout_column_reverse_margin_border() {
YogaConfig config = YogaConfigFactory.create();

final YogaNode root = createNode(config);
root.setFlexDirection(YogaFlexDirection.COLUMN_REVERSE);
root.setPositionType(YogaPositionType.ABSOLUTE);
root.setWidth(200f);
root.setHeight(200f);

final YogaNode root_child0 = createNode(config);
root_child0.setPositionType(YogaPositionType.ABSOLUTE);
root_child0.setPosition(YogaEdge.LEFT, 5f);
root_child0.setPosition(YogaEdge.RIGHT, 3f);
root_child0.setMargin(YogaEdge.LEFT, 3f);
root_child0.setMargin(YogaEdge.RIGHT, 4f);
root_child0.setBorder(YogaEdge.LEFT, 1f);
root_child0.setBorder(YogaEdge.RIGHT, 7f);
root_child0.setWidth(50f);
root_child0.setHeight(50f);
root.addChildAt(root_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(200f, root.getLayoutWidth(), 0.0f);
assertEquals(200f, root.getLayoutHeight(), 0.0f);

assertEquals(8f, root_child0.getLayoutX(), 0.0f);
assertEquals(150f, root_child0.getLayoutY(), 0.0f);
assertEquals(50f, root_child0.getLayoutWidth(), 0.0f);
assertEquals(50f, root_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(200f, root.getLayoutWidth(), 0.0f);
assertEquals(200f, root.getLayoutHeight(), 0.0f);

assertEquals(143f, root_child0.getLayoutX(), 0.0f);
assertEquals(150f, root_child0.getLayoutY(), 0.0f);
assertEquals(50f, root_child0.getLayoutWidth(), 0.0f);
assertEquals(50f, root_child0.getLayoutHeight(), 0.0f);
}

private YogaNode createNode(YogaConfig config) {
return mNodeFactory.create(config);
}
Expand Down
Loading

0 comments on commit 8fe38fc

Please sign in to comment.