-
Notifications
You must be signed in to change notification settings - Fork 24.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New file for layout methods related to absolute children #41794
Closed
Closed
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
facebook-github-bot
added
the
CLA Signed
This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
label
Dec 4, 2023
This pull request was exported from Phabricator. Differential Revision: D51824115 |
joevilches
pushed a commit
to joevilches/yoga
that referenced
this pull request
Dec 4, 2023
Summary: X-link: facebook/react-native#41794 This is a copy of D51369722 to make it so that it preserves the file history CalculateLayout.cpp is massive and approaching 3k lines. I added a few large functions dealing with layout of absolute nodes and was thinking it would be nice if that logic was just in its own file so it was more isolated and easier to reason about. So I made AbsoluteLayout.cpp and AbsoluteLayout.h to house this logic. In order for this to work I had to expose calculateLayoutInternal in CalculateLayout.h as layoutAbsoluteChild calls it. This is unideal and I would like to find a better way... I also make LayoutUtils.h to house misc small helper methods as they are called in AbsoluteLayout.cpp and CalculateLayout.cpp Reviewed By: NickGerleman Differential Revision: D51824115
joevilches
pushed a commit
to joevilches/react-native
that referenced
this pull request
Dec 4, 2023
) Summary: This is a copy of D51369722 to make it so that it preserves the file history CalculateLayout.cpp is massive and approaching 3k lines. I added a few large functions dealing with layout of absolute nodes and was thinking it would be nice if that logic was just in its own file so it was more isolated and easier to reason about. So I made AbsoluteLayout.cpp and AbsoluteLayout.h to house this logic. In order for this to work I had to expose calculateLayoutInternal in CalculateLayout.h as layoutAbsoluteChild calls it. This is unideal and I would like to find a better way... I also make LayoutUtils.h to house misc small helper methods as they are called in AbsoluteLayout.cpp and CalculateLayout.cpp Reviewed By: NickGerleman Differential Revision: D51824115
joevilches
force-pushed
the
export-D51824115
branch
from
December 4, 2023 21:54
0e9446e
to
714675b
Compare
This pull request was exported from Phabricator. Differential Revision: D51824115 |
Base commit: 974e584 |
joevilches
pushed a commit
to joevilches/yoga
that referenced
this pull request
Dec 4, 2023
Summary: X-link: facebook/react-native#41794 This is a copy of D51369722 to make it so that it preserves the file history CalculateLayout.cpp is massive and approaching 3k lines. I added a few large functions dealing with layout of absolute nodes and was thinking it would be nice if that logic was just in its own file so it was more isolated and easier to reason about. So I made AbsoluteLayout.cpp and AbsoluteLayout.h to house this logic. In order for this to work I had to expose calculateLayoutInternal in CalculateLayout.h as layoutAbsoluteChild calls it. This is unideal and I would like to find a better way... I also make LayoutUtils.h to house misc small helper methods as they are called in AbsoluteLayout.cpp and CalculateLayout.cpp Reviewed By: NickGerleman Differential Revision: D51824115
joevilches
pushed a commit
to joevilches/react-native
that referenced
this pull request
Dec 4, 2023
) Summary: X-link: facebook/yoga#1495 This is a copy of D51369722 to make it so that it preserves the file history CalculateLayout.cpp is massive and approaching 3k lines. I added a few large functions dealing with layout of absolute nodes and was thinking it would be nice if that logic was just in its own file so it was more isolated and easier to reason about. So I made AbsoluteLayout.cpp and AbsoluteLayout.h to house this logic. In order for this to work I had to expose calculateLayoutInternal in CalculateLayout.h as layoutAbsoluteChild calls it. This is unideal and I would like to find a better way... I also make LayoutUtils.h to house misc small helper methods as they are called in AbsoluteLayout.cpp and CalculateLayout.cpp Reviewed By: NickGerleman Differential Revision: D51824115
joevilches
force-pushed
the
export-D51824115
branch
from
December 4, 2023 22:49
714675b
to
6227696
Compare
This pull request was exported from Phabricator. Differential Revision: D51824115 |
Summary: X-link: facebook/yoga#1454 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
…acebook#41488) Summary: X-link: facebook/yoga#1470 The way we plan on implementing `position: static` is by changing how we lay out absolutely positioned nodes. Instead of letting their direct parent lay them out we are going to let their containing block handle it. This is useful because by the time the containing block gets to this step it will already know its size, which is needed to ensure that absolute nodes can get the right value with percentage units. Additionally, it means that we can "translate" the position of the absolute nodes to be relative to their parent fairly easily, instead of some second pass that would not be possible with a different design. This change just gets the core pieces of this process going. It makes it so that containing blocks will layout out absolute descendants that they contain. We also pass in the containing block size to the owner size args for `layoutAbsoluteChild`. This new path will only happen if we have the errata turned off. If there is no positioned ancestor for a given node we just assume the root is. This is not exactly how it works on the web - there is a notion of an initial containing block - but we are not implementing that as of right now. Reviewed By: NickGerleman Differential Revision: D51182593
…ndants (facebook#41489) Summary: X-link: facebook/yoga#1471 If we are going to allow the containing block to layout its absolute descendants and NOT the direct parent then we need to change step 11 which is concerned with setting the trailing position in the case we are row or column reverse. This is the very last step in the function and is positioned that way because it operates on the assumption that all children have their position set by this time. That is no longer a valid assumption if CBs layout their absolute children. In that case the CB also needs to take care of setting the position here. Because of this problem I moved some things around. It now works like: * If errata is set, the direct parent will set trailing position for all non absolute children in step 11 * If errata is set the CB will set trailing position of absolute descendants after they are laid out inside of layoutAbsoluteDescendants Reviewed By: NickGerleman Differential Revision: D51217291
Summary: X-link: facebook/yoga#1472 This change has most of the logic needed for supporting `position: static`. We do two things here that fix a lot of the broken static test: 1) We pass in the containing node to `layoutAbsoluteChild` and use it to properly position the child in the case that insets are defined. 2) We rewrite the absolute child's position to be relative to it's parent in the event that insets are defined for that child (and thus it is positioned relative to its CB). Yoga's layout position has always be relative to parent, so I feel it is easier to just adjust the coordinates of a node to adhere to that design rather than change the consumers of yoga. The "hard" part of this algorithm is determining how to iterate the offset from the containing block needed to do this translation described above. That is handled in `layoutAbsoluteDescendants`. Reviewed By: NickGerleman Differential Revision: D51224327
Summary: X-link: facebook/yoga#1473 To simplify the logic a bit I introduce a new function called `positionAbsoluteChild`. This function will, eventually, be the **sole function that matters** when determining the layout position of an absolute node. Because [absolute nodes do not participate in flex layout](https://drafts.csswg.org/css-flexbox/#abspos-items), we can determine the position of said node independently of its siblings. The only information we need are the node itself, its parent, and its containing block - which we have all of in `layoutAbsoluteChild`. Right now, however, this is purely a BE change with no functionality different. There was a big set of if statements at the end of `layoutAbsoluteChild` that would position the node on the main and cross axis for certain cases. The old code had it so that the main and cross axis had basically the same logic but the code was repeated. This puts that logic, as is, in `positionAbsoluteChild` and calls that from `layoutAbsoluteChild`. I will soon edit this function to actually do what it is envisioned to do (i.e. be the sole place that position is set for absolute nodes). Reviewed By: NickGerleman Differential Revision: D51272855
…facebook#41682) Summary: X-link: facebook/yoga#1479 There are two ways to get the value of a style for a specific edge right now: 1) From the inline start/end edge which is determined via the writing direction (ltr or rtl), assuming you do not have errata on 2) From the flex start/end edge which is determined via the flex direction (row, row-reverse, column, column-reverse) There is a weird curiosity in the second case: you can define a style to be on the "start" or "end" edge when writing the stylex/css. The physical edge that this refers to is dependent on the writing direction. So `start` would be `left` in `ltr` and `right` in `rtl`, with `end` the opposite. It is **never** determined via the flex direction. Additionally, `start`/`end` takes precedence over the physical edge it corresponds to in the case both are defined. So, all of this means that to actually get the value of a style from the flex start/end edges, we need to account for the case that one of these relative edges was defined and would overwrite any physical edge. Since this mapping is solely determined by the writing direction, we need to pass that in to all the flex start/end getters and do that logic. This is done in `flexStartRelativeEdge`/`flexEndRelativeEdge` which was added earlier but for some reason only being used on border. Reviewed By: NickGerleman Differential Revision: D51293315
) Summary: X-link: facebook/yoga#1495 This is a copy of D51369722 to make it so that it preserves the file history CalculateLayout.cpp is massive and approaching 3k lines. I added a few large functions dealing with layout of absolute nodes and was thinking it would be nice if that logic was just in its own file so it was more isolated and easier to reason about. So I made AbsoluteLayout.cpp and AbsoluteLayout.h to house this logic. In order for this to work I had to expose calculateLayoutInternal in CalculateLayout.h as layoutAbsoluteChild calls it. This is unideal and I would like to find a better way... I also make LayoutUtils.h to house misc small helper methods as they are called in AbsoluteLayout.cpp and CalculateLayout.cpp Reviewed By: NickGerleman Differential Revision: D51824115
joevilches
pushed a commit
to joevilches/yoga
that referenced
this pull request
Dec 4, 2023
Summary: X-link: facebook/react-native#41794 This is a copy of D51369722 to make it so that it preserves the file history CalculateLayout.cpp is massive and approaching 3k lines. I added a few large functions dealing with layout of absolute nodes and was thinking it would be nice if that logic was just in its own file so it was more isolated and easier to reason about. So I made AbsoluteLayout.cpp and AbsoluteLayout.h to house this logic. In order for this to work I had to expose calculateLayoutInternal in CalculateLayout.h as layoutAbsoluteChild calls it. This is unideal and I would like to find a better way... I also make LayoutUtils.h to house misc small helper methods as they are called in AbsoluteLayout.cpp and CalculateLayout.cpp Reviewed By: NickGerleman Differential Revision: D51824115
joevilches
force-pushed
the
export-D51824115
branch
from
December 4, 2023 23:22
6227696
to
e36c11c
Compare
This pull request was exported from Phabricator. Differential Revision: D51824115 |
facebook-github-bot
pushed a commit
to facebook/litho
that referenced
this pull request
Dec 5, 2023
Summary: X-link: facebook/yoga#1495 X-link: facebook/react-native#41794 This is a copy of D51369722 to make it so that it preserves the file history CalculateLayout.cpp is massive and approaching 3k lines. I added a few large functions dealing with layout of absolute nodes and was thinking it would be nice if that logic was just in its own file so it was more isolated and easier to reason about. So I made AbsoluteLayout.cpp and AbsoluteLayout.h to house this logic. In order for this to work I had to expose calculateLayoutInternal in CalculateLayout.h as layoutAbsoluteChild calls it. This is unideal and I would like to find a better way... I also make LayoutUtils.h to house misc small helper methods as they are called in AbsoluteLayout.cpp and CalculateLayout.cpp Reviewed By: NickGerleman Differential Revision: D51824115 fbshipit-source-id: 9b27449e3c1516492c01e6167a6b2c4568a33807
facebook-github-bot
pushed a commit
to facebook/yoga
that referenced
this pull request
Dec 5, 2023
Summary: Pull Request resolved: #1495 X-link: facebook/react-native#41794 This is a copy of D51369722 to make it so that it preserves the file history CalculateLayout.cpp is massive and approaching 3k lines. I added a few large functions dealing with layout of absolute nodes and was thinking it would be nice if that logic was just in its own file so it was more isolated and easier to reason about. So I made AbsoluteLayout.cpp and AbsoluteLayout.h to house this logic. In order for this to work I had to expose calculateLayoutInternal in CalculateLayout.h as layoutAbsoluteChild calls it. This is unideal and I would like to find a better way... I also make LayoutUtils.h to house misc small helper methods as they are called in AbsoluteLayout.cpp and CalculateLayout.cpp Reviewed By: NickGerleman Differential Revision: D51824115 fbshipit-source-id: 9b27449e3c1516492c01e6167a6b2c4568a33807
This pull request has been merged in 0cc09f2. |
Othinn
pushed a commit
to Othinn/react-native
that referenced
this pull request
Jan 9, 2024
) Summary: X-link: facebook/yoga#1495 Pull Request resolved: facebook#41794 This is a copy of D51369722 to make it so that it preserves the file history CalculateLayout.cpp is massive and approaching 3k lines. I added a few large functions dealing with layout of absolute nodes and was thinking it would be nice if that logic was just in its own file so it was more isolated and easier to reason about. So I made AbsoluteLayout.cpp and AbsoluteLayout.h to house this logic. In order for this to work I had to expose calculateLayoutInternal in CalculateLayout.h as layoutAbsoluteChild calls it. This is unideal and I would like to find a better way... I also make LayoutUtils.h to house misc small helper methods as they are called in AbsoluteLayout.cpp and CalculateLayout.cpp Reviewed By: NickGerleman Differential Revision: D51824115 fbshipit-source-id: 9b27449e3c1516492c01e6167a6b2c4568a33807
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
CLA Signed
This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
fb-exported
Merged
This PR has been merged.
p: Facebook
Partner: Facebook
Partner
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:
This is a copy of D51369722 to make it so that it preserves the file history
CalculateLayout.cpp is massive and approaching 3k lines. I added a few large functions dealing with layout of absolute nodes and was thinking it would be nice if that logic was just in its own file so it was more isolated and easier to reason about. So I made AbsoluteLayout.cpp and AbsoluteLayout.h to house this logic. In order for this to work I had to expose calculateLayoutInternal in CalculateLayout.h as layoutAbsoluteChild calls it. This is unideal and I would like to find a better way...
I also make LayoutUtils.h to house misc small helper methods as they are called in AbsoluteLayout.cpp and CalculateLayout.cpp
Differential Revision: D51824115