Enable previously broken absolute positioning tests #1488
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
|
This pull request was exported from Phabricator. Differential Revision: D51404875 |
|
This pull request was exported from Phabricator. Differential Revision: D51404875 |
joevilches
force-pushed
the
export-D51404875
branch
from
3d09e2e to
ec51c55
Compare
joevilches
added a commit
to joevilches/yoga
that referenced
this pull request
Dec 4, 2023
Summary: Pull Request resolved: facebook#1488 These were disabled when they were written because they were broken. The recent changes made them pass now so lets enable them. I also added another test that is already passing Reviewed By: NickGerleman Differential Revision: D51404875 fbshipit-source-id: dee9e88ff5e3036e7522061380889cd130d58587
|
This pull request was exported from Phabricator. Differential Revision: D51404875 |
joevilches
force-pushed
the
export-D51404875
branch
from
ec51c55 to
12d3468
Compare
joevilches
added a commit
to joevilches/yoga
that referenced
this pull request
Dec 5, 2023
Summary: Pull Request resolved: facebook#1488 These were disabled when they were written because they were broken. The recent changes made them pass now so lets enable them. I also added another test that is already passing Reviewed By: NickGerleman Differential Revision: D51404875 fbshipit-source-id: 7c38fafa6e47678d027c438df0671f9a0b6f26bf
|
This pull request was exported from Phabricator. Differential Revision: D51404875 |
joevilches
added a commit
to joevilches/yoga
that referenced
this pull request
Dec 6, 2023
Summary: Pull Request resolved: facebook#1488 These were disabled when they were written because they were broken. The recent changes made them pass now so lets enable them. I also added another test that is already passing Reviewed By: NickGerleman Differential Revision: D51404875 fbshipit-source-id: 755c28fc8aa9c6b0819f4992122e7922842cf44d
joevilches
force-pushed
the
export-D51404875
branch
from
12d3468 to
b0a4700
Compare
|
This pull request was exported from Phabricator. Differential Revision: D51404875 |
joevilches
force-pushed
the
export-D51404875
branch
from
b0a4700 to
f1aaac7
Compare
joevilches
added a commit
to joevilches/yoga
that referenced
this pull request
Dec 7, 2023
Summary: Pull Request resolved: facebook#1488 These were disabled when they were written because they were broken. The recent changes made them pass now so lets enable them. I also added another test that is already passing Reviewed By: NickGerleman Differential Revision: D51404875 fbshipit-source-id: cfdd5bb97569e08caacbd92c2939d74256c5e41e
…ng absolute node's position Summary: Absolute nodes can be laid out by themselves and do not have to care about what is happening to their siblings. Because of this we can make `positionAbsoluteChild` the sole place where we handle this logic. Right now that is scattered around algorithm with many `if (child is absolute)` cases everywhere. This makes implementing position static a lot harder since we are relying on the CB to do all this work, not the parent. With this change the only time we set position for an absolute node and it matter (i.e. not overwritten) is in `positionAbsoluteChild` Reviewed By: NickGerleman Differential Revision: D51290723 fbshipit-source-id: 26f2d1b4daf7e2ab9ee8edc92f591cdc66bb177e
Summary: Pull Request resolved: facebook#1482 X-link: facebook/react-native#41685 This is the final step (that I know of) to get the core features of static working. Here we turn on all of the tests and pass down the correct owner size for the call to `calculateLayoutInternal` that is in `layoutAbsoluteChild` Differential Revision: https://www.internalfb.com/diff/D51293606?entry_point=27 fbshipit-source-id: 1059ea53aeef7ec242834c62d933abee5ba69742
…ox (facebook#1485) Summary: Pull Request resolved: facebook#1485 X-link: facebook/react-native#41686 The size of the containing block is the size of the padding box of the containing node for absolute nodes. We were looking at `containingNode->getLayout().measuredDimension(Dimension::Width)` which is the border box. So we need to subtract the border from this. Added a test that was failing before this change as well Differential Revision: https://www.internalfb.com/diff/D51330526?entry_point=27 fbshipit-source-id: 9bf0c515264359557f168465bc0daf060f665a0d
Differential Revision: https://www.internalfb.com/diff/D51333812?entry_point=27 fbshipit-source-id: 90f59dc4d274f9b50a9dc105a3d989cd3fe61999
Summary: Tsia. Added test and accounted for parent padding Differential Revision: https://www.internalfb.com/diff/D51374086?entry_point=27 fbshipit-source-id: ed28788491323f71b8e657673c97e8f6c5b4b8d6
…rtain case Differential Revision: https://www.internalfb.com/diff/D51376309?entry_point=27 fbshipit-source-id: 62e3df0ceba855251edcb376c74fb54fa917d292
…ustifying Differential Revision: https://www.internalfb.com/diff/D51383625?entry_point=27 fbshipit-source-id: fb44e7a24210fcad7e01f93d010d93ffece899c7
Differential Revision: https://www.internalfb.com/diff/D51383792?entry_point=27 fbshipit-source-id: 0c6be88a6ec57a38b3478dd14b1ae618d6745e54
Summary: In the previous diffs I fixed problems with justifying absolute nodes. The same issues plague aligning so I fixed them in the same way. Added tests that were failing before but now passing Differential Revision: https://www.internalfb.com/diff/D51404489?entry_point=27 fbshipit-source-id: 4d0e7779eb0af2996b119b569ac9e73c1bf11929
Summary: Pull Request resolved: facebook#1488 These were disabled when they were written because they were broken. The recent changes made them pass now so lets enable them. I also added another test that is already passing Reviewed By: NickGerleman Differential Revision: D51404875 fbshipit-source-id: 83161b40177e5af0469d55a93477bc0846c060f4
|
This pull request was exported from Phabricator. Differential Revision: D51404875 |
joevilches
force-pushed
the
export-D51404875
branch
from
f1aaac7 to
bc1f716
Compare
|
This pull request has been merged in 1ea5756. |
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: These were disabled when they were written because they were broken. The recent changes made them pass now so lets enable them. I also added another test that is already passing
Reviewed By: NickGerleman
Differential Revision: D51404875