Rename ambiguous getLeading/Trailing... functions in Node.cpp #1424
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: D50344874 |
joevilches
added a commit
to joevilches/react-native
that referenced
this pull request
Oct 16, 2023
Summary: X-link: facebook/yoga#1424 See D50344874 Differential Revision: D50344874 fbshipit-source-id: bb4d0c474e3b8f7e1a087e17cdb5e7eedb648bca
|
A huge +1 for this change. I can read those new names and know exactly what they refer to. Whereas the old ones were indeed ambiguous to me. |
|
This pull request was exported from Phabricator. Differential Revision: D50344874 |
joevilches
added a commit
to joevilches/yoga
that referenced
this pull request
Oct 17, 2023
…ok#1424) Summary: X-link: facebook/react-native#41018 Pull Request resolved: facebook#1424 See D50344874 Reviewed By: NickGerleman Differential Revision: D50344874 fbshipit-source-id: 1a24ae1cdb9374cd7b84895fb7d5ea6bc9507bb1
joevilches
force-pushed
the
export-D50344874
branch
from
a00ffa7 to
3b4169a
Compare
joevilches
added a commit
to joevilches/react-native
that referenced
this pull request
Oct 17, 2023
…ok#41018) Summary: Pull Request resolved: facebook#41018 X-link: facebook/yoga#1424 See D50344874 Reviewed By: NickGerleman Differential Revision: D50344874 fbshipit-source-id: 995bb1a6cd2e9a9d2b79caf8aab7e09152a9294c
joevilches
pushed a commit
to joevilches/yoga
that referenced
this pull request
Oct 17, 2023
…ok#1424) Summary: X-link: facebook/react-native#41018 See D50344874 Reviewed By: NickGerleman Differential Revision: D50344874
joevilches
force-pushed
the
export-D50344874
branch
from
3b4169a to
f2510ee
Compare
joevilches
pushed a commit
to joevilches/react-native
that referenced
this pull request
Oct 17, 2023
…ok#41018) Summary: X-link: facebook/yoga#1424 See D50344874 Reviewed By: NickGerleman Differential Revision: D50344874
|
This pull request was exported from Phabricator. Differential Revision: D50344874 |
1 similar comment
|
This pull request was exported from Phabricator. Differential Revision: D50344874 |
joevilches
force-pushed
the
export-D50344874
branch
from
f2510ee to
0912018
Compare
joevilches
added a commit
to joevilches/yoga
that referenced
this pull request
Oct 17, 2023
…ok#1424) Summary: X-link: facebook/react-native#41018 Pull Request resolved: facebook#1424 See D50344874 Reviewed By: NickGerleman Differential Revision: D50344874 fbshipit-source-id: a423a7f8cefbc541c1b0bb1dae11fe81cbedfe96
joevilches
added a commit
to joevilches/react-native
that referenced
this pull request
Oct 17, 2023
…ok#41018) Summary: Pull Request resolved: facebook#41018 X-link: facebook/yoga#1424 See D50344874 Reviewed By: NickGerleman Differential Revision: D50344874 fbshipit-source-id: fdff690712d7740072257afaa931a3b09ad79c16
joevilches
pushed a commit
to joevilches/react-native
that referenced
this pull request
Oct 17, 2023
…ok#41018) Summary: X-link: facebook/yoga#1424 See D50344874 Reviewed By: NickGerleman Differential Revision: D50344874
joevilches
pushed a commit
to joevilches/react-native
that referenced
this pull request
Oct 17, 2023
…ok#41018) Summary: X-link: facebook/yoga#1424 See D50344874 Reviewed By: NickGerleman Differential Revision: D50344874
joevilches
pushed a commit
to joevilches/react-native
that referenced
this pull request
Oct 17, 2023
…ok#41018) Summary: X-link: facebook/yoga#1424 See D50344874 Reviewed By: NickGerleman Differential Revision: D50344874
joevilches
pushed a commit
to joevilches/yoga
that referenced
this pull request
Oct 18, 2023
…ok#1424) Summary: X-link: facebook/react-native#41018 See D50344874 Reviewed By: NickGerleman Differential Revision: D50344874
joevilches
force-pushed
the
export-D50344874
branch
from
0912018 to
98cc7c6
Compare
joevilches
pushed a commit
to joevilches/react-native
that referenced
this pull request
Oct 18, 2023
…ok#41018) Summary: X-link: facebook/yoga#1424 See D50344874 Reviewed By: NickGerleman Differential Revision: D50344874
|
This pull request was exported from Phabricator. Differential Revision: D50344874 |
joevilches
pushed a commit
to joevilches/yoga
that referenced
this pull request
Oct 18, 2023
…ok#1424) Summary: X-link: facebook/react-native#41018 See D50344874 Reviewed By: NickGerleman Differential Revision: D50344874
joevilches
pushed a commit
to joevilches/react-native
that referenced
this pull request
Oct 18, 2023
…ok#41018) Summary: X-link: facebook/yoga#1424 See D50344874 Reviewed By: NickGerleman Differential Revision: D50344874
added 2 commits
October 17, 2023 18:10
Summary: X-link: facebook/react-native#41017 Before resolving facebook#1208 yoga was in a state where "leading" and "trailing" only referred to the main-start and main-end directions ([definition in spec](https://drafts.csswg.org/css-flexbox/#box-model)). That is, the start/end of the layout of flex items in a container. This is distinct from something like inline-start/inline-end which is the [start of text layout as defined by direction](https://drafts.csswg.org/css-writing-modes-3/#inline-start). The bug linked above happened because "leading" and "trailing" functions are referring to the wrong directions in certain cases. So in order to fix this we added a new set of functions to get the "leading" and "trailing" edges according to what inline-start/inline-end would refer to - i.e. those defined by the direction (ltr | rtl). In this state I think it is confusing to understand which function refers to which direction and more specific names could help that. This diff just renames the following 4 FlexDirection.h functions: * **leadingEdge** -> **flexStartEdge** * **trailingEdge** -> **flexEndEdge** * **leadingLayoutEdge** -> **inlineStartEdge** * **trailingLayoutEdge** -> **inlineEndEdge** The spec calls the start/end directions as dictated by the flex-direction attribute "main-start" and "main-end" respectively, but mainStartEdge might be a bit confusing given it will be compared to a non-flexbox-specific name in inlineStartEdge. As a result I landed on flexStart/flexEnd similar to what values are used with alignment attributes (justify-content, align-content). I chose to get rid of the "leading" and "trailing" descriptors to be more in line with what terminology the spec uses. Next diff will be to rename the functions in Node.cpp to adhere to the above patterns. Reviewed By: NickGerleman Differential Revision: D50342254
…ok#1424) Summary: X-link: facebook/react-native#41018 See D50344874 Reviewed By: NickGerleman Differential Revision: D50344874
joevilches
force-pushed
the
export-D50344874
branch
from
98cc7c6 to
94fe535
Compare
joevilches
pushed a commit
to joevilches/react-native
that referenced
this pull request
Oct 18, 2023
…ok#41018) Summary: X-link: facebook/yoga#1424 See D50344874 Reviewed By: NickGerleman Differential Revision: D50344874
|
This pull request was exported from Phabricator. Differential Revision: D50344874 |
joevilches
pushed a commit
to joevilches/yoga
that referenced
this pull request
Oct 18, 2023
…ok#1424) Summary: X-link: facebook/react-native#41018 See D50344874 Reviewed By: NickGerleman Differential Revision: D50344874
joevilches
pushed a commit
to joevilches/react-native
that referenced
this pull request
Oct 18, 2023
…ok#41018) Summary: X-link: facebook/yoga#1424 See D50344874 Reviewed By: NickGerleman Differential Revision: D50344874
facebook-github-bot
pushed a commit
to facebook/litho
that referenced
this pull request
Oct 18, 2023
Summary: X-link: facebook/react-native#41018 X-link: facebook/yoga#1424 See D50344874 Reviewed By: NickGerleman Differential Revision: D50344874 fbshipit-source-id: 32fc35df674eb854c682a5e387c031a94c1c4f68
|
This pull request has been merged in e170059. |
facebook-github-bot
pushed a commit
to facebook/react-native
that referenced
this pull request
Oct 18, 2023
Summary: Pull Request resolved: #41018 X-link: facebook/yoga#1424 See D50344874 Reviewed By: NickGerleman Differential Revision: D50344874 fbshipit-source-id: 32fc35df674eb854c682a5e387c031a94c1c4f68
Othinn
pushed a commit
to Othinn/react-native
that referenced
this pull request
Oct 30, 2023
…ok#41018) Summary: Pull Request resolved: facebook#41018 X-link: facebook/yoga#1424 See D50344874 Reviewed By: NickGerleman Differential Revision: D50344874 fbshipit-source-id: 32fc35df674eb854c682a5e387c031a94c1c4f68
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: See D50344874
Differential Revision: D50344874