Skip to content

Commit

Permalink
Insets no longer apply to statically positioned nodes (facebook#1454)
Browse files Browse the repository at this point in the history
Summary:

X-link: facebook/react-native#41369

One of the most basic aspects of statically positioned nodes is that [insets do not apply to them](https://developer.mozilla.org/en-US/docs/Web/CSS/position#static). So I put a guard inside `Node::relativePosition` where we take that into account when setting the position.

Reviewed By: NickGerleman

Differential Revision: D50507808
  • Loading branch information
Joe Vilches authored and facebook-github-bot committed Nov 14, 2023
1 parent f2c8aca commit ac5c7d5
Show file tree
Hide file tree
Showing 5 changed files with 11 additions and 13 deletions.
4 changes: 2 additions & 2 deletions gentest/fixtures/YGStaticPositionTest.html
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
<!-- The top level divs in each test are needed so that each div overlays each
other and the top measurement is accurate -->
<div id="static_position_insets_have_no_effect_left_top" data-disabled="true">
<div id="static_position_insets_have_no_effect_left_top">
<div style="width: 100px; height: 100px; position: static; top: 50px; left: 50px;">
</div>
</div>

<div id="static_position_insets_have_no_effect_right_bottom" data-disabled="true">
<div id="static_position_insets_have_no_effect_right_bottom">
<div style="width: 100px; height: 100px; position: static; bottom: 50px; right: 50px;">
</div>
</div>
Expand Down
2 changes: 0 additions & 2 deletions java/tests/com/facebook/yoga/YGStaticPositionTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ public static Iterable<TestParametrization.NodeFactory> nodeFactories() {
@Parameterized.Parameter public TestParametrization.NodeFactory mNodeFactory;

@Test
@Ignore
public void test_static_position_insets_have_no_effect_left_top() {
YogaConfig config = YogaConfigFactory.create();
config.setExperimentalFeatureEnabled(YogaExperimentalFeature.ABSOLUTE_PERCENTAGE_AGAINST_PADDING_EDGE, true);
Expand Down Expand Up @@ -68,7 +67,6 @@ public void test_static_position_insets_have_no_effect_left_top() {
}

@Test
@Ignore
public void test_static_position_insets_have_no_effect_right_bottom() {
YogaConfig config = YogaConfigFactory.create();
config.setExperimentalFeatureEnabled(YogaExperimentalFeature.ABSOLUTE_PERCENTAGE_AGAINST_PADDING_EDGE, true);
Expand Down
4 changes: 2 additions & 2 deletions javascript/tests/generated/YGStaticPositionTest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import {
Wrap,
} from 'yoga-layout';

test.skip('static_position_insets_have_no_effect_left_top', () => {
test('static_position_insets_have_no_effect_left_top', () => {
const config = Yoga.Config.create();
let root;

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

Expand Down
4 changes: 0 additions & 4 deletions tests/generated/YGStaticPositionTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
#include <yoga/Yoga.h>

TEST(YogaTest, static_position_insets_have_no_effect_left_top) {
GTEST_SKIP();

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

Expand Down Expand Up @@ -56,8 +54,6 @@ TEST(YogaTest, static_position_insets_have_no_effect_left_top) {
}

TEST(YogaTest, static_position_insets_have_no_effect_right_bottom) {
GTEST_SKIP();

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

Expand Down
10 changes: 7 additions & 3 deletions yoga/node/Node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -502,11 +502,16 @@ void Node::setLayoutDimension(float dimensionValue, Dimension dimension) {
}

// If both left and right are defined, then use left. Otherwise return +left or
// -right depending on which is defined.
// -right depending on which is defined. Ignore statically positioned nodes as
// insets do not apply to them.
float Node::relativePosition(
FlexDirection axis,
Direction direction,
float axisSize) const {
if (style_.positionType() == PositionType::Static &&
!hasErrata(Errata::PositionStaticBehavesLikeRelative)) {
return 0;
}
if (isInlineStartPositionDefined(axis, direction)) {
return getInlineStartPosition(axis, direction, axisSize);
}
Expand All @@ -528,8 +533,7 @@ void Node::setPosition(
const FlexDirection crossAxis =
yoga::resolveCrossDirection(mainAxis, directionRespectingRoot);

// Here we should check for `PositionType::Static` and in this case zero inset
// properties (left, right, top, bottom, begin, end).
// In the case of position static these are just 0. See:
// https://www.w3.org/TR/css-position-3/#valdef-position-static
const float relativePositionMain =
relativePosition(mainAxis, directionRespectingRoot, mainSize);
Expand Down

0 comments on commit ac5c7d5

Please sign in to comment.