Make it so that static is considered in zIndex tiebreakers#42064
Closed
joevilches wants to merge 1 commit into
Closed
Make it so that static is considered in zIndex tiebreakers#42064joevilches wants to merge 1 commit into
joevilches wants to merge 1 commit into
Conversation
facebook-github-bot
added
CLA Signed
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D52103057 |
Base commit: 0c37f8c |
joevilches
force-pushed
the
export-D52103057
branch
from
52f0218 to
899866e
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D52103057 |
joevilches
force-pushed
the
export-D52103057
branch
from
899866e to
8cb5542
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D52103057 |
joevilches
force-pushed
the
export-D52103057
branch
from
8cb5542 to
3d28651
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D52103057 |
joevilches
force-pushed
the
export-D52103057
branch
from
3d28651 to
596eed9
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D52103057 |
…42064) Summary: # Context Dealing with how we should stack Views in a world with static is actually a bit complicated, despite the fact that static cannot get zIndex. The outline of everything can be found in this spec: https://www.w3.org/TR/CSS21/zindex.html. That is a bit dense, but mainly because there is much more functionality to consider in the browser than we need to in RN (like block layout, inline, floats, etc). Basically it comes down to this order, with larger numbers stacked on top of smaller ones: 1) Root 2) Negative zIndex positioned nodes (ties in document order) 3) Non positioned nodes in document order 4) Positioned nodes with no zIndex (or 0) in document order* 5) Positioned nodes with positive zIndex (ties in document order) Document order is a pre-order traversal of the tree. The asterisk on 4 is where it gets a bit complicated. Even though there is no zIndex set, you should still treat these nodes as if they form a stacking context - with their descendants stacked relative to the parent stacking context like normal if they have zIndex set. Essentially what this means in our world is that a static child will not come before (under) a positioned parent if they share the same stacking context. Without this, you would need to form a stacking context on all positioned nodes with static children if you wanted them to appear at all since static comes before (under) positioned nodes. # Implementation Implementing this was a bit tricky. We had to go in pre-order traversal of the tree, but if we see a relative node it needs to form a "pseudo-stacking-context" to allow static to show over it, but if any of those descendants have a zIndex set, that should be relative to the parent stacking context like normal, not this "pseudo-stacking-context". Without static we take care of this just fine because it just ends up being a pre-order traversal of the tree, then sort by zIndex. My approach was to ignore zIndex's at first and gather all nodes that share the same stacking context in an array as if none of them had any zIndex set. After that was gathered, then sort by zIndex to get the final order for said stacking context. The second part was already implemented. For the first part I just did a pre-order traversal of the tree (stopping at nodes that form stacking contexts) and: * If a node was non static, add to the end of the list * If a node was static, insert into the list right after the previously inserted static node that shares the same "psuedo-stacking-context", or the parent if there were none. * Since inserting into arrays is slow, I opted to use `std::list` In effect this was a pre-order traversal where static nodes are "sorted" to come first in the list of children, which is what we want since these nodes should come under non-positioned nodes that have the same "pseudo-stacking-context". My implementation is a bit slower. The previous one just visits all nodes once with a pre-order traversal. This will also do that and thanks to `std::list`, we have no non-constant time operations while coming in that order. After the fact, however, we need to convert back to `std::vector` which is the type used across the mounting layer, so we have to iterate over this list again. I think this is the fastest we can do this and it is optimized for a world where most nodes are relative (since it will be the new default). Changelog: [Internal] Reviewed By: NickGerleman Differential Revision: D52103057
joevilches
force-pushed
the
export-D52103057
branch
from
596eed9 to
801dd6f
Compare
joevilches
added a commit
to joevilches/react-native
that referenced
this pull request
Jan 5, 2024
…acebook#42064) Summary: # Context Dealing with how we should stack Views in a world with static is actually a bit complicated, despite the fact that static cannot get zIndex. The outline of everything can be found in this spec: https://www.w3.org/TR/CSS21/zindex.html. That is a bit dense, but mainly because there is much more functionality to consider in the browser than we need to in RN (like block layout, inline, floats, etc). Basically it comes down to this order, with larger numbers stacked on top of smaller ones: 1) Root 2) Negative zIndex positioned nodes (ties in document order) 3) Non positioned nodes in document order 4) Positioned nodes with no zIndex (or 0) in document order* 5) Positioned nodes with positive zIndex (ties in document order) Document order is a pre-order traversal of the tree. The asterisk on 4 is where it gets a bit complicated. Even though there is no zIndex set, you should still treat these nodes as if they form a stacking context - with their descendants stacked relative to the parent stacking context like normal if they have zIndex set. Essentially what this means in our world is that a static child will not come before (under) a positioned parent if they share the same stacking context. Without this, you would need to form a stacking context on all positioned nodes with static children if you wanted them to appear at all since static comes before (under) positioned nodes. # Implementation Implementing this was a bit tricky. We had to go in pre-order traversal of the tree, but if we see a relative node it needs to form a "pseudo-stacking-context" to allow static to show over it, but if any of those descendants have a zIndex set, that should be relative to the parent stacking context like normal, not this "pseudo-stacking-context". Without static we take care of this just fine because it just ends up being a pre-order traversal of the tree, then sort by zIndex. My approach was to ignore zIndex's at first and gather all nodes that share the same stacking context in an array as if none of them had any zIndex set. After that was gathered, then sort by zIndex to get the final order for said stacking context. The second part was already implemented. For the first part I just did a pre-order traversal of the tree (stopping at nodes that form stacking contexts) and: * If a node was non static, add to the end of the list * If a node was static, insert into the list right after the previously inserted static node that shares the same "psuedo-stacking-context", or the parent if there were none. * Since inserting into arrays is slow, I opted to use `std::list` In effect this was a pre-order traversal where static nodes are "sorted" to come first in the list of children, which is what we want since these nodes should come under non-positioned nodes that have the same "pseudo-stacking-context". My implementation is a bit slower. The previous one just visits all nodes once with a pre-order traversal. This will also do that and thanks to `std::list`, we have no non-constant time operations while coming in that order. After the fact, however, we need to convert back to `std::vector` which is the type used across the mounting layer, so we have to iterate over this list again. I think this is the fastest we can do this and it is optimized for a world where most nodes are relative (since it will be the new default). Changelog: [Internal] Reviewed By: NickGerleman Differential Revision: D52103057
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D52103057 |
|
This pull request was successfully merged by @joevilches in 8ad0839. When will my fix make it into a release? | Upcoming Releases |
Contributor
|
This pull request has been merged in 8ad0839. |
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
3 participants
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:
In the event that zIndex is not set, static views should always come behind non-static: https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_positioned_layout/Understanding_z-index/Stacking_without_z-index. Right now we were always doing document order. This changes our differentiator to consider position in this case.
Changelog: [Internal]
Differential Revision: D52103057