Skip to content

Commit

Permalink
Update some Justify tests following Chrome changes (facebook#1746)
Browse files Browse the repository at this point in the history 
Summary:
Pull Request resolved: facebook#1746

Chrome made some changes for how overflowed row-reverse containers are laid out which was causing some issues on CI. I updated them here and skipped the new failing tests which we would want to followup on.

For LTR, the differences are seen below
|Before|After|
|--|
|{F1962694149} | {F1962694151}|

The extra space is now extending past the flex start edge vs flex end. RTL is the opposite. NickGerleman had deviated from the spec back in the day to match Chrome and it seems they made the adjustment recently. T208209388 is tracking the followup to align with the spec again. Basically, there is a notion of fallback alignment when certain justification/alignment values cannot actually apply. Right now we are falling back to flex start in all cases but we should fallback to start sometimes.

Changelog: [Internal]

Reviewed By: NickGerleman

Differential Revision: D66138361

fbshipit-source-id: c46d2e9b0cd297069b9cc544e3bded995e4867a6
  • Loading branch information
@joevilches @facebook-github-bot
joevilches authored and facebook-github-bot committed Nov 19, 2024
1 parent 5b962c0 commit 26b21ae
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 47 deletions.
6 changes: 4 additions & 2 deletions gentest/fixtures/YGJustifyContentTest.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,13 +157,15 @@
<div style="width: 40px;"></div>
</div>

<div id="justify_content_overflow_row_reverse_space_around" style="width: 102px; height: 102px; flex-direction: row-reverse; justify-content: space-around;">
<!-- TODO T208209388: fallback to start vs flexstart in overflowed cases -->
<div data-disabled="true" id="justify_content_overflow_row_reverse_space_around" style="width: 102px; height: 102px; flex-direction: row-reverse; justify-content: space-around;">
<div style="width: 40px;"></div>
<div style="width: 40px;"></div>
<div style="width: 40px;"></div>
</div>

<div id="justify_content_overflow_row_reverse_space_evenly" style="width: 102px; height: 102px; flex-direction: row-reverse; justify-content: space-evenly;">
<!-- TODO T208209388: fallback to start vs flexstart in overflowed cases -->
<div data-disabled="true" id="justify_content_overflow_row_reverse_space_evenly" style="width: 102px; height: 102px; flex-direction: row-reverse; justify-content: space-evenly;">
<div style="width: 40px;"></div>
<div style="width: 40px;"></div>
<div style="width: 40px;"></div>
Expand Down
28 changes: 15 additions & 13 deletions java/tests/generated/com/facebook/yoga/YGJustifyContentTest.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<<6aaf6f020cdc74d09e440206957507aa>>
* @generated SignedSource<<ebd50ee9980922e7e6f3beabff5375ea>>
* generated by gentest/gentest-driver.ts from gentest/fixtures/YGJustifyContentTest.html
*/

Expand Down Expand Up @@ -1790,6 +1790,7 @@ public void test_justify_content_overflow_row_space_evenly() {
}

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

Expand Down Expand Up @@ -1819,17 +1820,17 @@ public void test_justify_content_overflow_row_reverse_space_around() {
assertEquals(102f, root.getLayoutWidth(), 0.0f);
assertEquals(102f, root.getLayoutHeight(), 0.0f);

assertEquals(62f, root_child0.getLayoutX(), 0.0f);
assertEquals(80f, root_child0.getLayoutX(), 0.0f);
assertEquals(0f, root_child0.getLayoutY(), 0.0f);
assertEquals(40f, root_child0.getLayoutWidth(), 0.0f);
assertEquals(102f, root_child0.getLayoutHeight(), 0.0f);

assertEquals(22f, root_child1.getLayoutX(), 0.0f);
assertEquals(40f, root_child1.getLayoutX(), 0.0f);
assertEquals(0f, root_child1.getLayoutY(), 0.0f);
assertEquals(40f, root_child1.getLayoutWidth(), 0.0f);
assertEquals(102f, root_child1.getLayoutHeight(), 0.0f);

assertEquals(-18f, root_child2.getLayoutX(), 0.0f);
assertEquals(0f, root_child2.getLayoutX(), 0.0f);
assertEquals(0f, root_child2.getLayoutY(), 0.0f);
assertEquals(40f, root_child2.getLayoutWidth(), 0.0f);
assertEquals(102f, root_child2.getLayoutHeight(), 0.0f);
Expand All @@ -1842,23 +1843,24 @@ public void test_justify_content_overflow_row_reverse_space_around() {
assertEquals(102f, root.getLayoutWidth(), 0.0f);
assertEquals(102f, root.getLayoutHeight(), 0.0f);

assertEquals(0f, root_child0.getLayoutX(), 0.0f);
assertEquals(-18f, root_child0.getLayoutX(), 0.0f);
assertEquals(0f, root_child0.getLayoutY(), 0.0f);
assertEquals(40f, root_child0.getLayoutWidth(), 0.0f);
assertEquals(102f, root_child0.getLayoutHeight(), 0.0f);

assertEquals(40f, root_child1.getLayoutX(), 0.0f);
assertEquals(22f, root_child1.getLayoutX(), 0.0f);
assertEquals(0f, root_child1.getLayoutY(), 0.0f);
assertEquals(40f, root_child1.getLayoutWidth(), 0.0f);
assertEquals(102f, root_child1.getLayoutHeight(), 0.0f);

assertEquals(80f, root_child2.getLayoutX(), 0.0f);
assertEquals(62f, root_child2.getLayoutX(), 0.0f);
assertEquals(0f, root_child2.getLayoutY(), 0.0f);
assertEquals(40f, root_child2.getLayoutWidth(), 0.0f);
assertEquals(102f, root_child2.getLayoutHeight(), 0.0f);
}

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

Expand Down Expand Up @@ -1888,17 +1890,17 @@ public void test_justify_content_overflow_row_reverse_space_evenly() {
assertEquals(102f, root.getLayoutWidth(), 0.0f);
assertEquals(102f, root.getLayoutHeight(), 0.0f);

assertEquals(62f, root_child0.getLayoutX(), 0.0f);
assertEquals(80f, root_child0.getLayoutX(), 0.0f);
assertEquals(0f, root_child0.getLayoutY(), 0.0f);
assertEquals(40f, root_child0.getLayoutWidth(), 0.0f);
assertEquals(102f, root_child0.getLayoutHeight(), 0.0f);

assertEquals(22f, root_child1.getLayoutX(), 0.0f);
assertEquals(40f, root_child1.getLayoutX(), 0.0f);
assertEquals(0f, root_child1.getLayoutY(), 0.0f);
assertEquals(40f, root_child1.getLayoutWidth(), 0.0f);
assertEquals(102f, root_child1.getLayoutHeight(), 0.0f);

assertEquals(-18f, root_child2.getLayoutX(), 0.0f);
assertEquals(0f, root_child2.getLayoutX(), 0.0f);
assertEquals(0f, root_child2.getLayoutY(), 0.0f);
assertEquals(40f, root_child2.getLayoutWidth(), 0.0f);
assertEquals(102f, root_child2.getLayoutHeight(), 0.0f);
Expand All @@ -1911,17 +1913,17 @@ public void test_justify_content_overflow_row_reverse_space_evenly() {
assertEquals(102f, root.getLayoutWidth(), 0.0f);
assertEquals(102f, root.getLayoutHeight(), 0.0f);

assertEquals(0f, root_child0.getLayoutX(), 0.0f);
assertEquals(-18f, root_child0.getLayoutX(), 0.0f);
assertEquals(0f, root_child0.getLayoutY(), 0.0f);
assertEquals(40f, root_child0.getLayoutWidth(), 0.0f);
assertEquals(102f, root_child0.getLayoutHeight(), 0.0f);

assertEquals(40f, root_child1.getLayoutX(), 0.0f);
assertEquals(22f, root_child1.getLayoutX(), 0.0f);
assertEquals(0f, root_child1.getLayoutY(), 0.0f);
assertEquals(40f, root_child1.getLayoutWidth(), 0.0f);
assertEquals(102f, root_child1.getLayoutHeight(), 0.0f);

assertEquals(80f, root_child2.getLayoutX(), 0.0f);
assertEquals(62f, root_child2.getLayoutX(), 0.0f);
assertEquals(0f, root_child2.getLayoutY(), 0.0f);
assertEquals(40f, root_child2.getLayoutWidth(), 0.0f);
assertEquals(102f, root_child2.getLayoutHeight(), 0.0f);
Expand Down
30 changes: 15 additions & 15 deletions javascript/tests/generated/YGJustifyContentTest.test.ts
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<<4d367d399575872c385f254588d91fb5>>
* @generated SignedSource<<d9c3d3430edadef3cd6ba8bbacbb31a2>>
* generated by gentest/gentest-driver.ts from gentest/fixtures/YGJustifyContentTest.html
*/

Expand Down Expand Up @@ -1925,7 +1925,7 @@ test('justify_content_overflow_row_space_evenly', () => {
config.free();
}
});
test('justify_content_overflow_row_reverse_space_around', () => {
test.skip('justify_content_overflow_row_reverse_space_around', () => {
const config = Yoga.Config.create();
let root;

Expand Down Expand Up @@ -1955,17 +1955,17 @@ test('justify_content_overflow_row_reverse_space_around', () => {
expect(root.getComputedWidth()).toBe(102);
expect(root.getComputedHeight()).toBe(102);

expect(root_child0.getComputedLeft()).toBe(62);
expect(root_child0.getComputedLeft()).toBe(80);
expect(root_child0.getComputedTop()).toBe(0);
expect(root_child0.getComputedWidth()).toBe(40);
expect(root_child0.getComputedHeight()).toBe(102);

expect(root_child1.getComputedLeft()).toBe(22);
expect(root_child1.getComputedLeft()).toBe(40);
expect(root_child1.getComputedTop()).toBe(0);
expect(root_child1.getComputedWidth()).toBe(40);
expect(root_child1.getComputedHeight()).toBe(102);

expect(root_child2.getComputedLeft()).toBe(-18);
expect(root_child2.getComputedLeft()).toBe(0);
expect(root_child2.getComputedTop()).toBe(0);
expect(root_child2.getComputedWidth()).toBe(40);
expect(root_child2.getComputedHeight()).toBe(102);
Expand All @@ -1977,17 +1977,17 @@ test('justify_content_overflow_row_reverse_space_around', () => {
expect(root.getComputedWidth()).toBe(102);
expect(root.getComputedHeight()).toBe(102);

expect(root_child0.getComputedLeft()).toBe(0);
expect(root_child0.getComputedLeft()).toBe(-18);
expect(root_child0.getComputedTop()).toBe(0);
expect(root_child0.getComputedWidth()).toBe(40);
expect(root_child0.getComputedHeight()).toBe(102);

expect(root_child1.getComputedLeft()).toBe(40);
expect(root_child1.getComputedLeft()).toBe(22);
expect(root_child1.getComputedTop()).toBe(0);
expect(root_child1.getComputedWidth()).toBe(40);
expect(root_child1.getComputedHeight()).toBe(102);

expect(root_child2.getComputedLeft()).toBe(80);
expect(root_child2.getComputedLeft()).toBe(62);
expect(root_child2.getComputedTop()).toBe(0);
expect(root_child2.getComputedWidth()).toBe(40);
expect(root_child2.getComputedHeight()).toBe(102);
Expand All @@ -1999,7 +1999,7 @@ test('justify_content_overflow_row_reverse_space_around', () => {
config.free();
}
});
test('justify_content_overflow_row_reverse_space_evenly', () => {
test.skip('justify_content_overflow_row_reverse_space_evenly', () => {
const config = Yoga.Config.create();
let root;

Expand Down Expand Up @@ -2029,17 +2029,17 @@ test('justify_content_overflow_row_reverse_space_evenly', () => {
expect(root.getComputedWidth()).toBe(102);
expect(root.getComputedHeight()).toBe(102);

expect(root_child0.getComputedLeft()).toBe(62);
expect(root_child0.getComputedLeft()).toBe(80);
expect(root_child0.getComputedTop()).toBe(0);
expect(root_child0.getComputedWidth()).toBe(40);
expect(root_child0.getComputedHeight()).toBe(102);

expect(root_child1.getComputedLeft()).toBe(22);
expect(root_child1.getComputedLeft()).toBe(40);
expect(root_child1.getComputedTop()).toBe(0);
expect(root_child1.getComputedWidth()).toBe(40);
expect(root_child1.getComputedHeight()).toBe(102);

expect(root_child2.getComputedLeft()).toBe(-18);
expect(root_child2.getComputedLeft()).toBe(0);
expect(root_child2.getComputedTop()).toBe(0);
expect(root_child2.getComputedWidth()).toBe(40);
expect(root_child2.getComputedHeight()).toBe(102);
Expand All @@ -2051,17 +2051,17 @@ test('justify_content_overflow_row_reverse_space_evenly', () => {
expect(root.getComputedWidth()).toBe(102);
expect(root.getComputedHeight()).toBe(102);

expect(root_child0.getComputedLeft()).toBe(0);
expect(root_child0.getComputedLeft()).toBe(-18);
expect(root_child0.getComputedTop()).toBe(0);
expect(root_child0.getComputedWidth()).toBe(40);
expect(root_child0.getComputedHeight()).toBe(102);

expect(root_child1.getComputedLeft()).toBe(40);
expect(root_child1.getComputedLeft()).toBe(22);
expect(root_child1.getComputedTop()).toBe(0);
expect(root_child1.getComputedWidth()).toBe(40);
expect(root_child1.getComputedHeight()).toBe(102);

expect(root_child2.getComputedLeft()).toBe(80);
expect(root_child2.getComputedLeft()).toBe(62);
expect(root_child2.getComputedTop()).toBe(0);
expect(root_child2.getComputedWidth()).toBe(40);
expect(root_child2.getComputedHeight()).toBe(102);
Expand Down
30 changes: 17 additions & 13 deletions tests/generated/YGJustifyContentTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* LICENSE file in the root directory of this source tree.
*
* clang-format off
* @generated SignedSource<<acb28ab0d3486089ff07a8763f92e22f>>
* @generated SignedSource<<07092062f14f233462a4678c392df76a>>
* generated by gentest/gentest-driver.ts from gentest/fixtures/YGJustifyContentTest.html
*/

Expand Down Expand Up @@ -1803,6 +1803,8 @@ TEST(YogaTest, justify_content_overflow_row_space_evenly) {
}

TEST(YogaTest, justify_content_overflow_row_reverse_space_around) {
GTEST_SKIP();

YGConfigRef config = YGConfigNew();

YGNodeRef root = YGNodeNewWithConfig(config);
Expand Down Expand Up @@ -1830,17 +1832,17 @@ TEST(YogaTest, justify_content_overflow_row_reverse_space_around) {
ASSERT_FLOAT_EQ(102, YGNodeLayoutGetWidth(root));
ASSERT_FLOAT_EQ(102, YGNodeLayoutGetHeight(root));

ASSERT_FLOAT_EQ(62, YGNodeLayoutGetLeft(root_child0));
ASSERT_FLOAT_EQ(80, YGNodeLayoutGetLeft(root_child0));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child0));
ASSERT_FLOAT_EQ(40, YGNodeLayoutGetWidth(root_child0));
ASSERT_FLOAT_EQ(102, YGNodeLayoutGetHeight(root_child0));

ASSERT_FLOAT_EQ(22, YGNodeLayoutGetLeft(root_child1));
ASSERT_FLOAT_EQ(40, YGNodeLayoutGetLeft(root_child1));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child1));
ASSERT_FLOAT_EQ(40, YGNodeLayoutGetWidth(root_child1));
ASSERT_FLOAT_EQ(102, YGNodeLayoutGetHeight(root_child1));

ASSERT_FLOAT_EQ(-18, YGNodeLayoutGetLeft(root_child2));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root_child2));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child2));
ASSERT_FLOAT_EQ(40, YGNodeLayoutGetWidth(root_child2));
ASSERT_FLOAT_EQ(102, YGNodeLayoutGetHeight(root_child2));
Expand All @@ -1852,17 +1854,17 @@ TEST(YogaTest, justify_content_overflow_row_reverse_space_around) {
ASSERT_FLOAT_EQ(102, YGNodeLayoutGetWidth(root));
ASSERT_FLOAT_EQ(102, YGNodeLayoutGetHeight(root));

ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root_child0));
ASSERT_FLOAT_EQ(-18, YGNodeLayoutGetLeft(root_child0));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child0));
ASSERT_FLOAT_EQ(40, YGNodeLayoutGetWidth(root_child0));
ASSERT_FLOAT_EQ(102, YGNodeLayoutGetHeight(root_child0));

ASSERT_FLOAT_EQ(40, YGNodeLayoutGetLeft(root_child1));
ASSERT_FLOAT_EQ(22, YGNodeLayoutGetLeft(root_child1));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child1));
ASSERT_FLOAT_EQ(40, YGNodeLayoutGetWidth(root_child1));
ASSERT_FLOAT_EQ(102, YGNodeLayoutGetHeight(root_child1));

ASSERT_FLOAT_EQ(80, YGNodeLayoutGetLeft(root_child2));
ASSERT_FLOAT_EQ(62, YGNodeLayoutGetLeft(root_child2));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child2));
ASSERT_FLOAT_EQ(40, YGNodeLayoutGetWidth(root_child2));
ASSERT_FLOAT_EQ(102, YGNodeLayoutGetHeight(root_child2));
Expand All @@ -1873,6 +1875,8 @@ TEST(YogaTest, justify_content_overflow_row_reverse_space_around) {
}

TEST(YogaTest, justify_content_overflow_row_reverse_space_evenly) {
GTEST_SKIP();

YGConfigRef config = YGConfigNew();

YGNodeRef root = YGNodeNewWithConfig(config);
Expand Down Expand Up @@ -1900,17 +1904,17 @@ TEST(YogaTest, justify_content_overflow_row_reverse_space_evenly) {
ASSERT_FLOAT_EQ(102, YGNodeLayoutGetWidth(root));
ASSERT_FLOAT_EQ(102, YGNodeLayoutGetHeight(root));

ASSERT_FLOAT_EQ(62, YGNodeLayoutGetLeft(root_child0));
ASSERT_FLOAT_EQ(80, YGNodeLayoutGetLeft(root_child0));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child0));
ASSERT_FLOAT_EQ(40, YGNodeLayoutGetWidth(root_child0));
ASSERT_FLOAT_EQ(102, YGNodeLayoutGetHeight(root_child0));

ASSERT_FLOAT_EQ(22, YGNodeLayoutGetLeft(root_child1));
ASSERT_FLOAT_EQ(40, YGNodeLayoutGetLeft(root_child1));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child1));
ASSERT_FLOAT_EQ(40, YGNodeLayoutGetWidth(root_child1));
ASSERT_FLOAT_EQ(102, YGNodeLayoutGetHeight(root_child1));

ASSERT_FLOAT_EQ(-18, YGNodeLayoutGetLeft(root_child2));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root_child2));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child2));
ASSERT_FLOAT_EQ(40, YGNodeLayoutGetWidth(root_child2));
ASSERT_FLOAT_EQ(102, YGNodeLayoutGetHeight(root_child2));
Expand All @@ -1922,17 +1926,17 @@ TEST(YogaTest, justify_content_overflow_row_reverse_space_evenly) {
ASSERT_FLOAT_EQ(102, YGNodeLayoutGetWidth(root));
ASSERT_FLOAT_EQ(102, YGNodeLayoutGetHeight(root));

ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root_child0));
ASSERT_FLOAT_EQ(-18, YGNodeLayoutGetLeft(root_child0));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child0));
ASSERT_FLOAT_EQ(40, YGNodeLayoutGetWidth(root_child0));
ASSERT_FLOAT_EQ(102, YGNodeLayoutGetHeight(root_child0));

ASSERT_FLOAT_EQ(40, YGNodeLayoutGetLeft(root_child1));
ASSERT_FLOAT_EQ(22, YGNodeLayoutGetLeft(root_child1));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child1));
ASSERT_FLOAT_EQ(40, YGNodeLayoutGetWidth(root_child1));
ASSERT_FLOAT_EQ(102, YGNodeLayoutGetHeight(root_child1));

ASSERT_FLOAT_EQ(80, YGNodeLayoutGetLeft(root_child2));
ASSERT_FLOAT_EQ(62, YGNodeLayoutGetLeft(root_child2));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child2));
ASSERT_FLOAT_EQ(40, YGNodeLayoutGetWidth(root_child2));
ASSERT_FLOAT_EQ(102, YGNodeLayoutGetHeight(root_child2));
Expand Down
8 changes: 4 additions & 4 deletions yoga/algorithm/Align.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ constexpr Align fallbackAlignment(Align align) {
case Align::Stretch:
return Align::FlexStart;

// Fallback to safe center. TODO: This should be aligned to Start
// instead of FlexStart (for row-reverse containers)
// Fallback to safe center. TODO (T208209388): This should be aligned to
// Start instead of FlexStart (for row-reverse containers)
case Align::SpaceAround:
case Align::SpaceEvenly:
return Align::FlexStart;
Expand All @@ -59,8 +59,8 @@ constexpr Justify fallbackAlignment(Justify align) {
// case Justify::Stretch:
return Justify::FlexStart;

// Fallback to safe center. TODO: This should be aligned to Start
// instead of FlexStart (for row-reverse containers)
// Fallback to safe center. TODO (T208209388): This should be aligned to
// Start instead of FlexStart (for row-reverse containers)
case Justify::SpaceAround:
case Justify::SpaceEvenly:
return Justify::FlexStart;
Expand Down

0 comments on commit 26b21ae

Please sign in to comment.