From 2aa278c8c5c39d697f636fa8c3781fc96f1a2e1b Mon Sep 17 00:00:00 2001
From: Kacper Kafara <kacper.kafara@swmansion.com>
Date: Thu, 16 Jan 2025 16:41:25 +0100
Subject: [PATCH] fix: remove workaround for removing clipped subviews (#2596)

## Description

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

1. https://github.com/software-mansion/react-native-screens/pull/2307
2. https://github.com/software-mansion/react-native-screens/pull/2383
3. https://github.com/software-mansion/react-native-screens/pull/2495
4. https://github.com/software-mansion/react-native-screens/pull/2531

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

1. [my comment on
#2495](https://github.com/software-mansion/react-native-screens/pull/2495#issuecomment-2478915818),
2. [my fix to RN
core](https://github.com/facebook/react-native/pull/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.

https://github.com/facebook/react-native/pull/47634 is supposed to fix
the underlying issue in `react-native's` core.

## Changes

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

The

* https://github.com/facebook/react-native/pull/47634

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

* https://github.com/facebook/react-native/pull/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
---
 .../java/com/swmansion/rnscreens/Screen.kt    | 26 -------------------
 apps/src/tests/Test2282.tsx                   |  7 ++++-
 2 files changed, 6 insertions(+), 27 deletions(-)

diff --git a/android/src/main/java/com/swmansion/rnscreens/Screen.kt b/android/src/main/java/com/swmansion/rnscreens/Screen.kt
index c2761f5f4..a04a6bef6 100644
--- a/android/src/main/java/com/swmansion/rnscreens/Screen.kt
+++ b/android/src/main/java/com/swmansion/rnscreens/Screen.kt
@@ -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
@@ -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)
                 }
             }
diff --git a/apps/src/tests/Test2282.tsx b/apps/src/tests/Test2282.tsx
index f41ba4b00..2745af4ff 100644
--- a/apps/src/tests/Test2282.tsx
+++ b/apps/src/tests/Test2282.tsx
@@ -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>
   );