Fix issue where absolute children of row-reverse containers would inset on the wrong side#41293
Closed
joevilches wants to merge 3 commits intofacebook:mainfrom
Closed
Fix issue where absolute children of row-reverse containers would inset on the wrong side#41293joevilches wants to merge 3 commits intofacebook:mainfrom
joevilches wants to merge 3 commits intofacebook:mainfrom
Conversation
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D50945475 |
Base commit: d11d5f3 |
bfb4e1e to
545f13e
Compare
Contributor
|
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
545f13e to
75faa3d
Compare
Contributor
|
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
75faa3d to
08f81a2
Compare
Contributor
|
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
08f81a2 to
1f5c4a4
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D50945475 |
Error ReferenceErrorDangerfile |
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
1f5c4a4 to
39e86c3
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D50945475 |
…ok#41208) Summary: X-link: facebook/yoga#1437 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/yoga#1439 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#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
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
39e86c3 to
91b47c3
Compare
Contributor
|
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
facebook-github-bot
pushed a commit
to facebook/yoga
that referenced
this pull request
Nov 7, 2023
…et on the wrong side (#1446) Summary: X-link: facebook/react-native#41293 Pull Request resolved: #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
Contributor
|
This pull request has been merged in 9847bca. |
This file contains hidden or 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
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