Skip to content

Commit 2aa278c

Browse files
authored
fix: remove workaround for removing clipped subviews (software-mansion#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
1 parent 07fc4a0 commit 2aa278c

File tree

2 files changed

+6
-27
lines changed

2 files changed

+6
-27
lines changed

android/src/main/java/com/swmansion/rnscreens/Screen.kt

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,9 @@ import androidx.swiperefreshlayout.widget.SwipeRefreshLayout
1717
import com.facebook.react.bridge.GuardedRunnable
1818
import com.facebook.react.bridge.ReactContext
1919
import com.facebook.react.uimanager.PixelUtil
20-
import com.facebook.react.uimanager.ReactClippingViewGroup
2120
import com.facebook.react.uimanager.UIManagerHelper
2221
import com.facebook.react.uimanager.UIManagerModule
2322
import com.facebook.react.uimanager.events.EventDispatcher
24-
import com.facebook.react.views.scroll.ReactHorizontalScrollView
25-
import com.facebook.react.views.scroll.ReactScrollView
2623
import com.google.android.material.bottomsheet.BottomSheetBehavior
2724
import com.google.android.material.shape.CornerFamily
2825
import com.google.android.material.shape.MaterialShapeDrawable
@@ -404,29 +401,6 @@ class Screen(
404401
}
405402

406403
if (child is ViewGroup) {
407-
// The children are miscounted when there's removeClippedSubviews prop
408-
// set to true (which is the default for FlatLists).
409-
// Unless the child is a ScrollView it's safe to assume that it's true
410-
// and add a simple view for each possibly clipped item to make it work as expected.
411-
// See https://github.com/software-mansion/react-native-screens/pull/2495
412-
413-
if (child is ReactClippingViewGroup &&
414-
child.removeClippedSubviews &&
415-
child !is ReactScrollView &&
416-
child !is ReactHorizontalScrollView
417-
) {
418-
// We need to workaround the issue until our changes land in core.
419-
// Some views do not accept any children or have set amount and they throw
420-
// when we want to brute-forcefully manipulate that.
421-
// Is this ugly? Very. Do we have better option before changes land in core?
422-
// I'm not aware of any.
423-
try {
424-
for (j in 0 until child.childCount) {
425-
child.addView(View(context))
426-
}
427-
} catch (_: Exception) {
428-
}
429-
}
430404
startTransitionRecursive(child)
431405
}
432406
}

apps/src/tests/Test2282.tsx

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,12 +135,17 @@ function ExtraNestedFlatlist(props: Partial<FlatListProps<number>>) {
135135

136136
const Stack = createNativeStackNavigator();
137137

138+
/**
139+
* You can use either the App component with `ListScreen` or `ListScreenSimplified`,
140+
* of `AppSimple` component which has little to no navigation and attempts to reproduce the issue
141+
*/
142+
138143
export default function App(): React.JSX.Element {
139144
return (
140145
<NavigationContainer>
141146
<Stack.Navigator screenOptions={{ animation: 'slide_from_right' }}>
142147
<Stack.Screen name="Home" component={Home} />
143-
<Stack.Screen name="List" component={ListScreenSimplified}/>
148+
<Stack.Screen name="List" component={ListScreenSimplified}/> {/* <- Exchange here for ListScreen for more complex case */}
144149
</Stack.Navigator>
145150
</NavigationContainer>
146151
);

0 commit comments

Comments
 (0)