Skip to content

Commit

Permalink
fix(Android): incorrect childCount in removeViewAt when using flatlis…
Browse files Browse the repository at this point in the history
…t on fabric (software-mansion#2307)

## Description

This PR intents to fix the crash when navigating back from a screen with
FlatList on the new architecture. The crash was caused by miscalculated
`childCount` of the list.
Earlier on I found out that setting the
[removeClippedSubviews](https://reactnative.dev/docs/flatlist#removeclippedsubviews)
option to false (defaults to true on Android) in the FlatList fixes the
problem.

This PR is rather a quick fix with an extra condition, that adds simple
views in place of the miscalculated ones in `startTransitionRecursive`
function if there's a FlatList with `removeClippedSubviews` option set.

Fixes software-mansion#2282.

## Changes

- added `Test2282.tsx` repro
- added extra condition in `startTransitionRecursive` function

<!--

## Screenshots / GIFs

Here you can add screenshots / GIFs documenting your change.

You can add before / after section if you're changing some behavior.

### Before

### After

-->

## Test code and steps to reproduce

- added `Test2282.tsx` repro

## Checklist

- [x] Ensured that CI passes
  • Loading branch information
alduzy authored and ja1ns committed Oct 9, 2024
1 parent f5c1b53 commit a6f2ba3
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 0 deletions.
10 changes: 10 additions & 0 deletions android/src/main/java/com/swmansion/rnscreens/Screen.kt
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import com.facebook.react.uimanager.PixelUtil
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.ReactScrollView
import com.google.android.material.bottomsheet.BottomSheetBehavior
import com.swmansion.rnscreens.events.HeaderHeightChangeEvent
import com.swmansion.rnscreens.events.SheetDetentChangedEvent
Expand Down Expand Up @@ -384,6 +385,15 @@ class Screen(
startTransitionRecursive(child.toolbar)
}
if (child is ViewGroup) {
// The children are miscounted when there's a FlatList with
// removeCLippedSubviews set to true (default).
// We add a simple view for each item in the list to make it work as expected.
// See https://github.com/software-mansion/react-native-screens/issues/2282
if (it is ReactScrollView && it.removeClippedSubviews) {
for (j in 0 until child.childCount) {
child.addView(View(context))
}
}
startTransitionRecursive(child)
}
}
Expand Down
32 changes: 32 additions & 0 deletions apps/src/tests/Test2282.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import React from 'react';
import { FlatList, Button, Text } from 'react-native';
import { createNativeStackNavigator } from '@react-navigation/native-stack';
import { NavigationContainer } from '@react-navigation/native';

const Stack = createNativeStackNavigator();

const First = ({ navigation }: any) => (
<Button onPress={() => navigation.navigate('Second')} title="Navigate" />
);

const Second = ({ navigation }: any) => (
<>
<FlatList
renderItem={({ item }) => <Text>{item}</Text>}
data={[1,2,3,4,5,6]}
keyExtractor={item => item.toString()}
/>
<Button onPress={navigation.goBack} title="Go back" />
</>
);

export default function App() {
return (
<NavigationContainer>
<Stack.Navigator>
<Stack.Screen name="First" component={First} />
<Stack.Screen name="Second" component={Second} />
</Stack.Navigator>
</NavigationContainer>
);
}
1 change: 1 addition & 0 deletions apps/src/tests/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ export { default as Test2232 } from './Test2232';
export { default as Test2235 } from './Test2235';
export { default as Test2252 } from './Test2252';
export { default as Test2271 } from './Test2271';
export { default as Test2282 } from './Test2282';
export { default as TestScreenAnimation } from './TestScreenAnimation';
export { default as TestHeader } from './TestHeader';

0 comments on commit a6f2ba3

Please sign in to comment.