Skip to content

Commit

Permalink
Make it so that aspect ratio behaves like auto if it is 0 or inf (#1696)
Browse files Browse the repository at this point in the history
Summary:
X-link: facebook/react-native#46428

Pull Request resolved: #1696

We do not validate the aspect ratio to ensure it is non zero and non inf in a lot of places. Per the spec, these values should act like auto. There is no auto keyword, but it is the default so I just set the style to a default FloatOptional in this case

Changelog: [Internal]

Reviewed By: NickGerleman

Differential Revision: D62473161

fbshipit-source-id: 6857de819538a7a87ce0a652e99f5a49992921ae
  • Loading branch information
joevilches authored and facebook-github-bot committed Sep 12, 2024
1 parent dc4ab5a commit a112a07
Show file tree
Hide file tree
Showing 5 changed files with 142 additions and 8 deletions.
12 changes: 8 additions & 4 deletions gentest/fixtures/YGAspectRatioTest.html
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,18 @@
<div id="aspect_ratio_does_not_stretch_cross_axis_dim" data-disabled="true" style="width: 300px; height: 300px;">
<div style="flex: 1; overflow: scroll;">
<div style="flex-direction: row;">
<div style="flex: 2; aspect-ratio: 1;"></div>
<div style="width: 5px"></div>
<div style="flex: 1">
<div style="flex: 1; aspect-ratio: 1;">
<div style="flex: 2; aspect-ratio: 1;"></div>
<div style="width: 5px"></div>
<div style="flex: 1">
<div style="flex: 1; aspect-ratio: 1;">
<div style="width: 5px"></div>
<div style="flex: 1; aspect-ratio: 1;"></div>
</div>
</div>
</div>
</div>
</div>

<div id="zero_aspect_ratio_behaves_like_auto" style="width: 300px; height: 300px;">
<div style="aspect-ratio: 0; width: 50px"></div>
</div>
42 changes: 41 additions & 1 deletion java/tests/generated/com/facebook/yoga/YGAspectRatioTest.java
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<<e5092fdd5b7f67021254ae2bbc5a3684>>
* @generated SignedSource<<37a01c67158df025b1b43b8378071746>>
* generated by gentest/gentest-driver.ts from gentest/fixtures/YGAspectRatioTest.html
*/

Expand Down Expand Up @@ -179,6 +179,46 @@ public void test_aspect_ratio_does_not_stretch_cross_axis_dim() {
assertEquals(197f, root_child0_child0_child2_child0_child1.getLayoutHeight(), 0.0f);
}

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

final YogaNode root = createNode(config);
root.setPositionType(YogaPositionType.ABSOLUTE);
root.setWidth(300f);
root.setHeight(300f);

final YogaNode root_child0 = createNode(config);
root_child0.setWidth(50f);
root_child0.setAspectRatio(0 / 1f);
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(300f, root.getLayoutWidth(), 0.0f);
assertEquals(300f, root.getLayoutHeight(), 0.0f);

assertEquals(0f, root_child0.getLayoutX(), 0.0f);
assertEquals(0f, root_child0.getLayoutY(), 0.0f);
assertEquals(50f, root_child0.getLayoutWidth(), 0.0f);
assertEquals(0f, 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(300f, root.getLayoutWidth(), 0.0f);
assertEquals(300f, root.getLayoutHeight(), 0.0f);

assertEquals(250f, root_child0.getLayoutX(), 0.0f);
assertEquals(0f, root_child0.getLayoutY(), 0.0f);
assertEquals(50f, root_child0.getLayoutWidth(), 0.0f);
assertEquals(0f, root_child0.getLayoutHeight(), 0.0f);
}

private YogaNode createNode(YogaConfig config) {
return mNodeFactory.create(config);
}
Expand Down
47 changes: 46 additions & 1 deletion javascript/tests/generated/YGAspectRatioTest.test.ts
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<<acd05459fdaeace7681295ee1812c3cb>>
* @generated SignedSource<<c9a86707a0d8554afa020ec319688655>>
* generated by gentest/gentest-driver.ts from gentest/fixtures/YGAspectRatioTest.html
*/

Expand Down Expand Up @@ -183,3 +183,48 @@ test.skip('aspect_ratio_does_not_stretch_cross_axis_dim', () => {
config.free();
}
});
test('zero_aspect_ratio_behaves_like_auto', () => {
const config = Yoga.Config.create();
let root;

try {
root = Yoga.Node.create(config);
root.setPositionType(PositionType.Absolute);
root.setWidth(300);
root.setHeight(300);

const root_child0 = Yoga.Node.create(config);
root_child0.setWidth(50);
root_child0.setAspectRatio(0 / 1);
root.insertChild(root_child0, 0);
root.calculateLayout(undefined, undefined, Direction.LTR);

expect(root.getComputedLeft()).toBe(0);
expect(root.getComputedTop()).toBe(0);
expect(root.getComputedWidth()).toBe(300);
expect(root.getComputedHeight()).toBe(300);

expect(root_child0.getComputedLeft()).toBe(0);
expect(root_child0.getComputedTop()).toBe(0);
expect(root_child0.getComputedWidth()).toBe(50);
expect(root_child0.getComputedHeight()).toBe(0);

root.calculateLayout(undefined, undefined, Direction.RTL);

expect(root.getComputedLeft()).toBe(0);
expect(root.getComputedTop()).toBe(0);
expect(root.getComputedWidth()).toBe(300);
expect(root.getComputedHeight()).toBe(300);

expect(root_child0.getComputedLeft()).toBe(250);
expect(root_child0.getComputedTop()).toBe(0);
expect(root_child0.getComputedWidth()).toBe(50);
expect(root_child0.getComputedHeight()).toBe(0);
} finally {
if (typeof root !== 'undefined') {
root.freeRecursive();
}

config.free();
}
});
43 changes: 42 additions & 1 deletion tests/generated/YGAspectRatioTest.cpp
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<<bca5b9517b7a728b30eb070a33de0bdf>>
* @generated SignedSource<<0894aa78d01d5194e4c042491128cd1c>>
* generated by gentest/gentest-driver.ts from gentest/fixtures/YGAspectRatioTest.html
*/

Expand Down Expand Up @@ -166,3 +166,44 @@ TEST(YogaTest, aspect_ratio_does_not_stretch_cross_axis_dim) {

YGConfigFree(config);
}

TEST(YogaTest, zero_aspect_ratio_behaves_like_auto) {
YGConfigRef config = YGConfigNew();

YGNodeRef root = YGNodeNewWithConfig(config);
YGNodeStyleSetPositionType(root, YGPositionTypeAbsolute);
YGNodeStyleSetWidth(root, 300);
YGNodeStyleSetHeight(root, 300);

YGNodeRef root_child0 = YGNodeNewWithConfig(config);
YGNodeStyleSetWidth(root_child0, 50);
YGNodeStyleSetAspectRatio(root_child0, 0 / 1);
YGNodeInsertChild(root, root_child0, 0);
YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR);

ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root));
ASSERT_FLOAT_EQ(300, YGNodeLayoutGetWidth(root));
ASSERT_FLOAT_EQ(300, YGNodeLayoutGetHeight(root));

ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root_child0));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child0));
ASSERT_FLOAT_EQ(50, YGNodeLayoutGetWidth(root_child0));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetHeight(root_child0));

YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionRTL);

ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root));
ASSERT_FLOAT_EQ(300, YGNodeLayoutGetWidth(root));
ASSERT_FLOAT_EQ(300, YGNodeLayoutGetHeight(root));

ASSERT_FLOAT_EQ(250, YGNodeLayoutGetLeft(root_child0));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child0));
ASSERT_FLOAT_EQ(50, YGNodeLayoutGetWidth(root_child0));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetHeight(root_child0));

YGNodeFreeRecursive(root);

YGConfigFree(config);
}
6 changes: 5 additions & 1 deletion yoga/style/Style.h
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,11 @@ class YG_EXPORT Style {
return pool_.getNumber(aspectRatio_);
}
void setAspectRatio(FloatOptional value) {
pool_.store(aspectRatio_, value);
// degenerate aspect ratios act as auto.
// see https://drafts.csswg.org/css-sizing-4/#valdef-aspect-ratio-ratio
pool_.store(
aspectRatio_,
value == 0.0f || std::isinf(value.unwrap()) ? FloatOptional{} : value);
}

bool horizontalInsetsDefined() const {
Expand Down

0 comments on commit a112a07

Please sign in to comment.