Fix issue where absolute children of row-reverse containers would inset on the wrong side #1446
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This pull request was exported from Phabricator. Differential Revision: D50945475 |
joevilches
added a commit
to joevilches/yoga
that referenced
this pull request
Nov 3, 2023
…et on the wrong side (facebook#1446) Summary: NickGerleman pointed out that my recent changes to fix the slew of row-reverse problems in Yoga actually ended up regressing some parts. Specifically, absolute children of row-reverse containers would have their insets set to the wrong side. So if you set left: 10 it would apply it to the right. Turns out, in `layoutAbsoluteChild` there were cases where we were applying inlineStart/End values to the flexStart/End edge, which can never be right. So I changed the values to also be flexStart/End as the fix here. Differential Revision: D50945475
joevilches
force-pushed
the
export-D50945475
branch
from
092d63b to
8e1a9cb
Compare
|
This pull request was exported from Phabricator. Differential Revision: D50945475 |
joevilches
added a commit
to joevilches/react-native
that referenced
this pull request
Nov 3, 2023
…et on the wrong side Summary: X-link: facebook/yoga#1446 NickGerleman pointed out that my recent changes to fix the slew of row-reverse problems in Yoga actually ended up regressing some parts. Specifically, absolute children of row-reverse containers would have their insets set to the wrong side. So if you set left: 10 it would apply it to the right. Turns out, in `layoutAbsoluteChild` there were cases where we were applying inlineStart/End values to the flexStart/End edge, which can never be right. So I changed the values to also be flexStart/End as the fix here. Differential Revision: D50945475
|
One thing I've found to be super-helpful to get good test coverage of issues like this without having to create a million tests is to use uneven padding/border/margin/inset sizes. For examples |
joevilches
added a commit
to joevilches/yoga
that referenced
this pull request
Nov 6, 2023
…et on the wrong side (facebook#1446) Summary: X-link: facebook/react-native#41293 NickGerleman pointed out that my recent changes to fix the slew of row-reverse problems in Yoga actually ended up regressing some parts. Specifically, absolute children of row-reverse containers would have their insets set to the wrong side. So if you set left: 10 it would apply it to the right. Turns out, in `layoutAbsoluteChild` there were cases where we were applying inlineStart/End values to the flexStart/End edge, which can never be right. So I changed the values to also be flexStart/End as the fix here. Reviewed By: NickGerleman Differential Revision: D50945475
joevilches
force-pushed
the
export-D50945475
branch
from
8e1a9cb to
f2a221f
Compare
joevilches
added a commit
to joevilches/react-native
that referenced
this pull request
Nov 6, 2023
…et on the wrong side (facebook#41293) Summary: X-link: facebook/yoga#1446 NickGerleman pointed out that my recent changes to fix the slew of row-reverse problems in Yoga actually ended up regressing some parts. Specifically, absolute children of row-reverse containers would have their insets set to the wrong side. So if you set left: 10 it would apply it to the right. Turns out, in `layoutAbsoluteChild` there were cases where we were applying inlineStart/End values to the flexStart/End edge, which can never be right. So I changed the values to also be flexStart/End as the fix here. Reviewed By: NickGerleman Differential Revision: D50945475
|
This pull request was exported from Phabricator. Differential Revision: D50945475 |
joevilches
added a commit
to joevilches/yoga
that referenced
this pull request
Nov 6, 2023
…et on the wrong side (facebook#1446) Summary: X-link: facebook/react-native#41293 NickGerleman pointed out that my recent changes to fix the slew of row-reverse problems in Yoga actually ended up regressing some parts. Specifically, absolute children of row-reverse containers would have their insets set to the wrong side. So if you set left: 10 it would apply it to the right. Turns out, in `layoutAbsoluteChild` there were cases where we were applying inlineStart/End values to the flexStart/End edge, which can never be right. So I changed the values to also be flexStart/End as the fix here. Reviewed By: NickGerleman Differential Revision: D50945475
joevilches
force-pushed
the
export-D50945475
branch
from
f2a221f to
6b1726d
Compare
joevilches
added a commit
to joevilches/react-native
that referenced
this pull request
Nov 6, 2023
…et on the wrong side (facebook#41293) Summary: X-link: facebook/yoga#1446 NickGerleman pointed out that my recent changes to fix the slew of row-reverse problems in Yoga actually ended up regressing some parts. Specifically, absolute children of row-reverse containers would have their insets set to the wrong side. So if you set left: 10 it would apply it to the right. Turns out, in `layoutAbsoluteChild` there were cases where we were applying inlineStart/End values to the flexStart/End edge, which can never be right. So I changed the values to also be flexStart/End as the fix here. Reviewed By: NickGerleman Differential Revision: D50945475
|
This pull request was exported from Phabricator. Differential Revision: D50945475 |
joevilches
force-pushed
the
export-D50945475
branch
from
6b1726d to
a5227a0
Compare
joevilches
added a commit
to joevilches/yoga
that referenced
this pull request
Nov 6, 2023
…et on the wrong side (facebook#1446) Summary: X-link: facebook/react-native#41293 NickGerleman pointed out that my recent changes to fix the slew of row-reverse problems in Yoga actually ended up regressing some parts. Specifically, absolute children of row-reverse containers would have their insets set to the wrong side. So if you set left: 10 it would apply it to the right. Turns out, in `layoutAbsoluteChild` there were cases where we were applying inlineStart/End values to the flexStart/End edge, which can never be right. So I changed the values to also be flexStart/End as the fix here. Reviewed By: NickGerleman Differential Revision: D50945475
joevilches
added a commit
to joevilches/react-native
that referenced
this pull request
Nov 6, 2023
…et on the wrong side (facebook#41293) Summary: X-link: facebook/yoga#1446 NickGerleman pointed out that my recent changes to fix the slew of row-reverse problems in Yoga actually ended up regressing some parts. Specifically, absolute children of row-reverse containers would have their insets set to the wrong side. So if you set left: 10 it would apply it to the right. Turns out, in `layoutAbsoluteChild` there were cases where we were applying inlineStart/End values to the flexStart/End edge, which can never be right. So I changed the values to also be flexStart/End as the fix here. Reviewed By: NickGerleman Differential Revision: D50945475
|
This pull request was exported from Phabricator. Differential Revision: D50945475 |
joevilches
added a commit
to joevilches/yoga
that referenced
this pull request
Nov 7, 2023
…et on the wrong side (facebook#1446) Summary: X-link: facebook/react-native#41293 NickGerleman pointed out that my recent changes to fix the slew of row-reverse problems in Yoga actually ended up regressing some parts. Specifically, absolute children of row-reverse containers would have their insets set to the wrong side. So if you set left: 10 it would apply it to the right. Turns out, in `layoutAbsoluteChild` there were cases where we were applying inlineStart/End values to the flexStart/End edge, which can never be right. So I changed the values to also be flexStart/End as the fix here. Reviewed By: NickGerleman Differential Revision: D50945475
joevilches
force-pushed
the
export-D50945475
branch
from
a5227a0 to
5bd93ac
Compare
joevilches
added a commit
to joevilches/react-native
that referenced
this pull request
Nov 7, 2023
…et on the wrong side (facebook#41293) Summary: X-link: facebook/yoga#1446 NickGerleman pointed out that my recent changes to fix the slew of row-reverse problems in Yoga actually ended up regressing some parts. Specifically, absolute children of row-reverse containers would have their insets set to the wrong side. So if you set left: 10 it would apply it to the right. Turns out, in `layoutAbsoluteChild` there were cases where we were applying inlineStart/End values to the flexStart/End edge, which can never be right. So I changed the values to also be flexStart/End as the fix here. Reviewed By: NickGerleman Differential Revision: D50945475
|
This pull request was exported from Phabricator. Differential Revision: D50945475 |
…ok#1437) Summary: X-link: facebook/react-native#41208 Reading through the sizing logic and this seemed a bit redundant/confusing. Lets use the same function we just used for the main axis for the cross axis as well so people do not think its special. Also we will need one less variable. The reason this was done it seems is because we need the leading padding + border elsewhere so this is technically a few less steps but this is cleaner Reviewed By: NickGerleman Differential Revision: D50704177
Summary: X-link: facebook/react-native#41209 There are so many instances in this code base where we use the double negative of `!yoga::isUndefined(<something>)`. This is not as easy to read since because of this double negative imo. Additionally, sometimes we have really long chains like `!longVariableName.longFunctionName(longArgumentName).isUndefined()` and it is hard to see that this undefined is inverted. This just replaces all instances of inverted `isUndefined()` with `isDefined()` so its easier to read. Reviewed By: NickGerleman Differential Revision: D50705523
…et on the wrong side (facebook#1446) Summary: X-link: facebook/react-native#41293 NickGerleman pointed out that my recent changes to fix the slew of row-reverse problems in Yoga actually ended up regressing some parts. Specifically, absolute children of row-reverse containers would have their insets set to the wrong side. So if you set left: 10 it would apply it to the right. Turns out, in `layoutAbsoluteChild` there were cases where we were applying inlineStart/End values to the flexStart/End edge, which can never be right. So I changed the values to also be flexStart/End as the fix here. Reviewed By: NickGerleman Differential Revision: D50945475
joevilches
force-pushed
the
export-D50945475
branch
from
5bd93ac to
c06d4a1
Compare
joevilches
added a commit
to joevilches/react-native
that referenced
this pull request
Nov 7, 2023
…et on the wrong side (facebook#41293) Summary: X-link: facebook/yoga#1446 NickGerleman pointed out that my recent changes to fix the slew of row-reverse problems in Yoga actually ended up regressing some parts. Specifically, absolute children of row-reverse containers would have their insets set to the wrong side. So if you set left: 10 it would apply it to the right. Turns out, in `layoutAbsoluteChild` there were cases where we were applying inlineStart/End values to the flexStart/End edge, which can never be right. So I changed the values to also be flexStart/End as the fix here. Reviewed By: NickGerleman Differential Revision: D50945475
|
This pull request was exported from Phabricator. Differential Revision: D50945475 |
facebook-github-bot
pushed a commit
to facebook/litho
that referenced
this pull request
Nov 7, 2023
…et on the wrong side Summary: X-link: facebook/react-native#41293 X-link: facebook/yoga#1446 NickGerleman pointed out that my recent changes to fix the slew of row-reverse problems in Yoga actually ended up regressing some parts. Specifically, absolute children of row-reverse containers would have their insets set to the wrong side. So if you set left: 10 it would apply it to the right. Turns out, in `layoutAbsoluteChild` there were cases where we were applying inlineStart/End values to the flexStart/End edge, which can never be right. So I changed the values to also be flexStart/End as the fix here. Reviewed By: NickGerleman Differential Revision: D50945475 fbshipit-source-id: 290de06dcc04e8e644a3a32c127af12fdabb2f75
|
This pull request has been merged in 283e320. |
facebook-github-bot
pushed a commit
to facebook/react-native
that referenced
this pull request
Nov 7, 2023
…et on the wrong side (#41293) Summary: Pull Request resolved: #41293 X-link: facebook/yoga#1446 NickGerleman pointed out that my recent changes to fix the slew of row-reverse problems in Yoga actually ended up regressing some parts. Specifically, absolute children of row-reverse containers would have their insets set to the wrong side. So if you set left: 10 it would apply it to the right. Turns out, in `layoutAbsoluteChild` there were cases where we were applying inlineStart/End values to the flexStart/End edge, which can never be right. So I changed the values to also be flexStart/End as the fix here. Reviewed By: NickGerleman Differential Revision: D50945475 fbshipit-source-id: 290de06dcc04e8e644a3a32c127af12fdabb2f75
Othinn
pushed a commit
to Othinn/react-native
that referenced
this pull request
Jan 9, 2024
…et on the wrong side (facebook#41293) Summary: Pull Request resolved: facebook#41293 X-link: facebook/yoga#1446 NickGerleman pointed out that my recent changes to fix the slew of row-reverse problems in Yoga actually ended up regressing some parts. Specifically, absolute children of row-reverse containers would have their insets set to the wrong side. So if you set left: 10 it would apply it to the right. Turns out, in `layoutAbsoluteChild` there were cases where we were applying inlineStart/End values to the flexStart/End edge, which can never be right. So I changed the values to also be flexStart/End as the fix here. Reviewed By: NickGerleman Differential Revision: D50945475 fbshipit-source-id: 290de06dcc04e8e644a3a32c127af12fdabb2f75
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
NickGerleman pointed out that my recent changes to fix the slew of row-reverse problems in Yoga actually ended up regressing some parts. Specifically, absolute children of row-reverse containers would have their insets set to the wrong side. So if you set left: 10 it would apply it to the right.
Turns out, in
layoutAbsoluteChildthere were cases where we were applying inlineStart/End values to the flexStart/End edge, which can never be right. So I changed the values to also be flexStart/End as the fix here.Differential Revision: D50945475