Skip to content

Commit

Permalink
fix: remove workaround for removing clipped subviews (software-mansio…
Browse files Browse the repository at this point in the history
…n#2596)

## Description

This PR removes the workaround introduced in series of PRs (listed
chronologically here):

1. software-mansion#2307
2. software-mansion#2383
3. software-mansion#2495
4. software-mansion#2531

For detailed description of error mechanism and broader discussion
please refer to:

1. [my comment on
software-mansion#2495](software-mansion#2495 (comment)),
2. [my fix to RN
core](facebook/react-native#47634)

tldr: When popping screen on Fabric we marked the views as
"transitioning" and this led to view being effectively miscounted
during removal by view groups that supported react-native's subview
clipping mechanism.

The issue has been present most likely in every version of the library
when running on Android & Fabric, but it arose few months ago due to
broader
adoption of the new architecture.

facebook/react-native#47634 is supposed to fix
the underlying issue in `react-native's` core.

## Changes

Removed the workaround code from `Screen` implementation on Android.

The 

* facebook/react-native#47634

has been released with 0.77.0-rc.3 and followup small fixup:

* facebook/react-native#48329

has been released with 0.77.0-rc.4.

Therefore, with landing this PR we should limit our support on Fabric to
0.77.0.

## Test code and steps to reproduce

`Test2282` - note that there are few testing variants available there,
you just need to comment (out) some parts of the code.

## Checklist

- [x] Included code example that can be used to test this change
- [ ] Ensured that CI passes
  • Loading branch information
kkafar authored Jan 16, 2025
1 parent 07fc4a0 commit 2aa278c
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 27 deletions.
26 changes: 0 additions & 26 deletions android/src/main/java/com/swmansion/rnscreens/Screen.kt
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,9 @@ import androidx.swiperefreshlayout.widget.SwipeRefreshLayout
import com.facebook.react.bridge.GuardedRunnable
import com.facebook.react.bridge.ReactContext
import com.facebook.react.uimanager.PixelUtil
import com.facebook.react.uimanager.ReactClippingViewGroup
import com.facebook.react.uimanager.UIManagerHelper
import com.facebook.react.uimanager.UIManagerModule
import com.facebook.react.uimanager.events.EventDispatcher
import com.facebook.react.views.scroll.ReactHorizontalScrollView
import com.facebook.react.views.scroll.ReactScrollView
import com.google.android.material.bottomsheet.BottomSheetBehavior
import com.google.android.material.shape.CornerFamily
import com.google.android.material.shape.MaterialShapeDrawable
Expand Down Expand Up @@ -404,29 +401,6 @@ class Screen(
}

if (child is ViewGroup) {
// The children are miscounted when there's removeClippedSubviews prop
// set to true (which is the default for FlatLists).
// Unless the child is a ScrollView it's safe to assume that it's true
// and add a simple view for each possibly clipped item to make it work as expected.
// See https://github.com/software-mansion/react-native-screens/pull/2495

if (child is ReactClippingViewGroup &&
child.removeClippedSubviews &&
child !is ReactScrollView &&
child !is ReactHorizontalScrollView
) {
// We need to workaround the issue until our changes land in core.
// Some views do not accept any children or have set amount and they throw
// when we want to brute-forcefully manipulate that.
// Is this ugly? Very. Do we have better option before changes land in core?
// I'm not aware of any.
try {
for (j in 0 until child.childCount) {
child.addView(View(context))
}
} catch (_: Exception) {
}
}
startTransitionRecursive(child)
}
}
Expand Down
7 changes: 6 additions & 1 deletion apps/src/tests/Test2282.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -135,12 +135,17 @@ function ExtraNestedFlatlist(props: Partial<FlatListProps<number>>) {

const Stack = createNativeStackNavigator();

/**
* You can use either the App component with `ListScreen` or `ListScreenSimplified`,
* of `AppSimple` component which has little to no navigation and attempts to reproduce the issue
*/

export default function App(): React.JSX.Element {
return (
<NavigationContainer>
<Stack.Navigator screenOptions={{ animation: 'slide_from_right' }}>
<Stack.Screen name="Home" component={Home} />
<Stack.Screen name="List" component={ListScreenSimplified}/>
<Stack.Screen name="List" component={ListScreenSimplified}/> {/* <- Exchange here for ListScreen for more complex case */}
</Stack.Navigator>
</NavigationContainer>
);
Expand Down

0 comments on commit 2aa278c

Please sign in to comment.