Skip to content

Commit

Permalink
fix: remove gap if its last element in line (fix flex gap extra spaci…
Browse files Browse the repository at this point in the history
…ng when children determine parents main axis size) (#1188)

Summary:
Fixes - facebook/react-native#35553

## Approach
We're using `betweenMainDim` to add [gap between](https://github.com/intergalacticspacehighway/yoga/blob/bbeede82d36cd89657faf385aaa704f45443c326/yoga/Yoga.cpp#L2495) items in main axis. This is resulting in increased [main axis](https://github.com/intergalacticspacehighway/yoga/blob/bbeede82d36cd89657faf385aaa704f45443c326/yoga/Yoga.cpp#L2598) dimension of the container as it gets added even for the last element. One solution is to keep using it and subtract the gap when last element is reached.

## Aside
Mutating this value feels weird, but I think `betweenMainDim` gets initialized for every line so should be fine? I did some manual tests to verify. I tried running tests but I'll have to downgrade the java version. Let me know if anything fails. Thanks! 🙏

Pull Request resolved: #1188

Test Plan: Added fixtures which previously failed but now pass.

Reviewed By: necolas

Differential Revision: D42078162

Pulled By: NickGerleman

fbshipit-source-id: 0e535618350422e001141a8786a83fc81651afe9
  • Loading branch information
intergalacticspacehighway authored and facebook-github-bot committed Dec 16, 2022
1 parent 35f3335 commit ba27f9d
Show file tree
Hide file tree
Showing 6 changed files with 561 additions and 0 deletions.
135 changes: 135 additions & 0 deletions csharp/tests/Facebook.Yoga/YGGapTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1597,6 +1597,74 @@ public void Test_column_gap_wrap_align_stretch()
Assert.AreEqual(150f, root_child4.LayoutHeight);
}

[Test]
public void Test_column_gap_determines_parent_width()
{
YogaConfig config = new YogaConfig();

YogaNode root = new YogaNode(config);
root.FlexDirection = YogaFlexDirection.Row;
root.Height = 100;
root.ColumnGap = 10;

YogaNode root_child0 = new YogaNode(config);
root_child0.Width = 10;
root.Insert(0, root_child0);

YogaNode root_child1 = new YogaNode(config);
root_child1.Width = 20;
root.Insert(1, root_child1);

YogaNode root_child2 = new YogaNode(config);
root_child2.Width = 30;
root.Insert(2, root_child2);
root.StyleDirection = YogaDirection.LTR;
root.CalculateLayout();

Assert.AreEqual(0f, root.LayoutX);
Assert.AreEqual(0f, root.LayoutY);
Assert.AreEqual(80f, root.LayoutWidth);
Assert.AreEqual(100f, root.LayoutHeight);

Assert.AreEqual(0f, root_child0.LayoutX);
Assert.AreEqual(0f, root_child0.LayoutY);
Assert.AreEqual(10f, root_child0.LayoutWidth);
Assert.AreEqual(100f, root_child0.LayoutHeight);

Assert.AreEqual(20f, root_child1.LayoutX);
Assert.AreEqual(0f, root_child1.LayoutY);
Assert.AreEqual(20f, root_child1.LayoutWidth);
Assert.AreEqual(100f, root_child1.LayoutHeight);

Assert.AreEqual(50f, root_child2.LayoutX);
Assert.AreEqual(0f, root_child2.LayoutY);
Assert.AreEqual(30f, root_child2.LayoutWidth);
Assert.AreEqual(100f, root_child2.LayoutHeight);

root.StyleDirection = YogaDirection.RTL;
root.CalculateLayout();

Assert.AreEqual(0f, root.LayoutX);
Assert.AreEqual(0f, root.LayoutY);
Assert.AreEqual(80f, root.LayoutWidth);
Assert.AreEqual(100f, root.LayoutHeight);

Assert.AreEqual(70f, root_child0.LayoutX);
Assert.AreEqual(0f, root_child0.LayoutY);
Assert.AreEqual(10f, root_child0.LayoutWidth);
Assert.AreEqual(100f, root_child0.LayoutHeight);

Assert.AreEqual(40f, root_child1.LayoutX);
Assert.AreEqual(0f, root_child1.LayoutY);
Assert.AreEqual(20f, root_child1.LayoutWidth);
Assert.AreEqual(100f, root_child1.LayoutHeight);

Assert.AreEqual(0f, root_child2.LayoutX);
Assert.AreEqual(0f, root_child2.LayoutY);
Assert.AreEqual(30f, root_child2.LayoutWidth);
Assert.AreEqual(100f, root_child2.LayoutHeight);
}

[Test]
public void Test_row_gap_align_items_stretch()
{
Expand Down Expand Up @@ -1981,5 +2049,72 @@ public void Test_row_gap_row_wrap_child_margins()
Assert.AreEqual(0f, root_child2.LayoutHeight);
}

[Test]
public void Test_row_gap_determines_parent_height()
{
YogaConfig config = new YogaConfig();

YogaNode root = new YogaNode(config);
root.Width = 100;
root.RowGap = 10;

YogaNode root_child0 = new YogaNode(config);
root_child0.Height = 10;
root.Insert(0, root_child0);

YogaNode root_child1 = new YogaNode(config);
root_child1.Height = 20;
root.Insert(1, root_child1);

YogaNode root_child2 = new YogaNode(config);
root_child2.Height = 30;
root.Insert(2, root_child2);
root.StyleDirection = YogaDirection.LTR;
root.CalculateLayout();

Assert.AreEqual(0f, root.LayoutX);
Assert.AreEqual(0f, root.LayoutY);
Assert.AreEqual(100f, root.LayoutWidth);
Assert.AreEqual(80f, root.LayoutHeight);

Assert.AreEqual(0f, root_child0.LayoutX);
Assert.AreEqual(0f, root_child0.LayoutY);
Assert.AreEqual(100f, root_child0.LayoutWidth);
Assert.AreEqual(10f, root_child0.LayoutHeight);

Assert.AreEqual(0f, root_child1.LayoutX);
Assert.AreEqual(20f, root_child1.LayoutY);
Assert.AreEqual(100f, root_child1.LayoutWidth);
Assert.AreEqual(20f, root_child1.LayoutHeight);

Assert.AreEqual(0f, root_child2.LayoutX);
Assert.AreEqual(50f, root_child2.LayoutY);
Assert.AreEqual(100f, root_child2.LayoutWidth);
Assert.AreEqual(30f, root_child2.LayoutHeight);

root.StyleDirection = YogaDirection.RTL;
root.CalculateLayout();

Assert.AreEqual(0f, root.LayoutX);
Assert.AreEqual(0f, root.LayoutY);
Assert.AreEqual(100f, root.LayoutWidth);
Assert.AreEqual(80f, root.LayoutHeight);

Assert.AreEqual(0f, root_child0.LayoutX);
Assert.AreEqual(0f, root_child0.LayoutY);
Assert.AreEqual(100f, root_child0.LayoutWidth);
Assert.AreEqual(10f, root_child0.LayoutHeight);

Assert.AreEqual(0f, root_child1.LayoutX);
Assert.AreEqual(20f, root_child1.LayoutY);
Assert.AreEqual(100f, root_child1.LayoutWidth);
Assert.AreEqual(20f, root_child1.LayoutHeight);

Assert.AreEqual(0f, root_child2.LayoutX);
Assert.AreEqual(50f, root_child2.LayoutY);
Assert.AreEqual(100f, root_child2.LayoutWidth);
Assert.AreEqual(30f, root_child2.LayoutHeight);
}

}
}
12 changes: 12 additions & 0 deletions gentest/fixtures/YGGapTest.html
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,12 @@
<div style="min-width: 60px; flex-grow: 1;"></div>
</div>

<div id="column_gap_determines_parent_width" style="flex-direction: row; height: 100px; align-items: 'stretch'; column-gap: 10px;">
<div style="width: 10px;"></div>
<div style="width: 20px;"></div>
<div style="width: 30px;"></div>
</div>

<div id="row_gap_align_items_stretch" style="flex-direction: row; flex-wrap: wrap; width: 100px; height: 200px; column-gap: 10px; row-gap: 20px; align-items:stretch; align-content: stretch">
<div style="width: 20px; "></div>
<div style="width: 20px;"></div>
Expand Down Expand Up @@ -152,3 +158,9 @@
<div style="width: 60px; margin-block: 10px;"></div>
<div style="width: 60px; margin-block: 15px;"></div>
</div>

<div id="row_gap_determines_parent_height" style="flex-direction: column; width: 100px; align-items: 'stretch'; row-gap: 10px;">
<div style="height: 10px;"></div>
<div style="height: 20px"></div>
<div style="height: 30px"></div>
</div>
133 changes: 133 additions & 0 deletions java/tests/com/facebook/yoga/YGGapTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -1589,6 +1589,73 @@ public void test_column_gap_wrap_align_stretch() {
assertEquals(150f, root_child4.getLayoutHeight(), 0.0f);
}

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

final YogaNode root = createNode(config);
root.setFlexDirection(YogaFlexDirection.ROW);
root.setHeight(100f);
root.setGap(YogaGutter.COLUMN, 10f);

final YogaNode root_child0 = createNode(config);
root_child0.setWidth(10f);
root.addChildAt(root_child0, 0);

final YogaNode root_child1 = createNode(config);
root_child1.setWidth(20f);
root.addChildAt(root_child1, 1);

final YogaNode root_child2 = createNode(config);
root_child2.setWidth(30f);
root.addChildAt(root_child2, 2);
root.setDirection(YogaDirection.LTR);
root.calculateLayout(YogaConstants.UNDEFINED, YogaConstants.UNDEFINED);

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

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

assertEquals(20f, root_child1.getLayoutX(), 0.0f);
assertEquals(0f, root_child1.getLayoutY(), 0.0f);
assertEquals(20f, root_child1.getLayoutWidth(), 0.0f);
assertEquals(100f, root_child1.getLayoutHeight(), 0.0f);

assertEquals(50f, root_child2.getLayoutX(), 0.0f);
assertEquals(0f, root_child2.getLayoutY(), 0.0f);
assertEquals(30f, root_child2.getLayoutWidth(), 0.0f);
assertEquals(100f, root_child2.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(80f, root.getLayoutWidth(), 0.0f);
assertEquals(100f, root.getLayoutHeight(), 0.0f);

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

assertEquals(40f, root_child1.getLayoutX(), 0.0f);
assertEquals(0f, root_child1.getLayoutY(), 0.0f);
assertEquals(20f, root_child1.getLayoutWidth(), 0.0f);
assertEquals(100f, root_child1.getLayoutHeight(), 0.0f);

assertEquals(0f, root_child2.getLayoutX(), 0.0f);
assertEquals(0f, root_child2.getLayoutY(), 0.0f);
assertEquals(30f, root_child2.getLayoutWidth(), 0.0f);
assertEquals(100f, root_child2.getLayoutHeight(), 0.0f);
}

@Test
public void test_row_gap_align_items_stretch() {
YogaConfig config = YogaConfigFactory.create();
Expand Down Expand Up @@ -1969,6 +2036,72 @@ public void test_row_gap_row_wrap_child_margins() {
assertEquals(0f, root_child2.getLayoutHeight(), 0.0f);
}

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

final YogaNode root = createNode(config);
root.setWidth(100f);
root.setGap(YogaGutter.ROW, 10f);

final YogaNode root_child0 = createNode(config);
root_child0.setHeight(10f);
root.addChildAt(root_child0, 0);

final YogaNode root_child1 = createNode(config);
root_child1.setHeight(20f);
root.addChildAt(root_child1, 1);

final YogaNode root_child2 = createNode(config);
root_child2.setHeight(30f);
root.addChildAt(root_child2, 2);
root.setDirection(YogaDirection.LTR);
root.calculateLayout(YogaConstants.UNDEFINED, YogaConstants.UNDEFINED);

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

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

assertEquals(0f, root_child1.getLayoutX(), 0.0f);
assertEquals(20f, root_child1.getLayoutY(), 0.0f);
assertEquals(100f, root_child1.getLayoutWidth(), 0.0f);
assertEquals(20f, root_child1.getLayoutHeight(), 0.0f);

assertEquals(0f, root_child2.getLayoutX(), 0.0f);
assertEquals(50f, root_child2.getLayoutY(), 0.0f);
assertEquals(100f, root_child2.getLayoutWidth(), 0.0f);
assertEquals(30f, root_child2.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(100f, root.getLayoutWidth(), 0.0f);
assertEquals(80f, root.getLayoutHeight(), 0.0f);

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

assertEquals(0f, root_child1.getLayoutX(), 0.0f);
assertEquals(20f, root_child1.getLayoutY(), 0.0f);
assertEquals(100f, root_child1.getLayoutWidth(), 0.0f);
assertEquals(20f, root_child1.getLayoutHeight(), 0.0f);

assertEquals(0f, root_child2.getLayoutX(), 0.0f);
assertEquals(50f, root_child2.getLayoutY(), 0.0f);
assertEquals(100f, root_child2.getLayoutWidth(), 0.0f);
assertEquals(30f, root_child2.getLayoutHeight(), 0.0f);
}

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

0 comments on commit ba27f9d

Please sign in to comment.