Skip to content

Commit

Permalink
Stop applying zPosition for views (#42021)
Browse files Browse the repository at this point in the history
Summary:

# Context
A while ago D48905010 was committed that set the CALayer's zPosition equal to the zIndex passed in via props. This was to avoid an issue like:  https://pxl.cl/40F72
where the rotating view would clip into the background.

There are a few issues here. 
* The current zIndex code is designed to be cross platform on the C++ layer and not the native layer. This diverges from the Android behavior and adds a special case for this platform when we have the ability to share all of this logic
* Static nodes will apply a zIndex when they shouldn't. This code pre-empts the static check [here](https://www.internalfb.com/code/fbsource/[cf8ad268b4cf]/xplat/js/react-native-github/packages/react-native/ReactCommon/react/renderer/components/view/ConcreteViewShadowNode.h?lines=98) that zeroes out the "order index" for static nodes
* As a result of the above, static nodes can eclipse their children which should never happen because static nodes ignore zIndex values

# Reason for the clipping
The reason this clipping is happening is because the red/blue views share the same stacking context as the white background (as indicated by the vertical black line). {F1175070418}
This in combination with the fact that our zIndex implementation will NOT set iOS's zPosition means that these three views (red view, blue view, white background view) all have the same zPosition (0) and will be laid out in the order described by the orderIndex mentioned earlier. This index essentially just changes the document order that is used for tiebreakers when zPosition is tied. 

So, all the views are on the same stacking context and they all have no zPosition set. Add the rotation that the colored views are doing and you get this clipping. Apple will change the "zPosition" in a sense for the parts of the view that should be perceived as "further away" due to the rotation. So, we clip into our background which has a lower order index but the same zPosition.

# This change
The fix here just makes it so that the rotating views are not on the same stacking context as the background so the changing zPosition from the rotation does not matter. This can be achieved by setting the zIndex of the container to any number (among other things). Note that this is only the case because the default position type is relative in this stack. Otherwise you would also need to set the position type as well. Now the stacking context looks like: {F1175083828} and the problem is solved!

Changelog: [Internal]

Reviewed By: NickGerleman

Differential Revision: D52181701
  • Loading branch information
joevilches authored and facebook-github-bot committed Dec 20, 2023
1 parent d9cd4db commit 633f522
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -374,11 +374,6 @@ - (void)updateProps:(const Props::Shared &)props oldProps:(const Props::Shared &
self.accessibilityIdentifier = RCTNSStringFromString(newViewProps.testId);
}

// `zIndex`
if (oldViewProps.zIndex != newViewProps.zIndex) {
self.layer.zPosition = newViewProps.zIndex.value_or(0);
}

_needsInvalidateLayer = _needsInvalidateLayer || needsInvalidateLayer;

_props = std::static_pointer_cast<const ViewProps>(props);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,7 @@ const styles = StyleSheet.create({
marginVertical: 40,
flex: 1,
alignSelf: 'center',
zIndex: 0,
},
flipCard: {
width: 200,
Expand All @@ -255,7 +256,6 @@ const styles = StyleSheet.create({
justifyContent: 'center',
backgroundColor: 'blue',
backfaceVisibility: 'hidden',
zIndex: 1024,
},
flipCard1: {
position: 'absolute',
Expand Down

0 comments on commit 633f522

Please sign in to comment.