Skip to content

Commit d83d4b7

Browse files
alduzykkafar
alduzy
authored and
kkafar
committed
fix(Android): going back on fabric with nested list (#2383)
This PR intents to fix crash happening on Android (fabric) when navigating back from a screen with nested list. The crash happens whenever the nested list is visible on the screen, as shown below: | ✅ | ❌ | ❌ | ❌ | ✅ | |----------|----------|----------|----------|----------| | <img width="247" alt="Screenshot 2024-10-03 at 16 12 16" src="https://github.com/user-attachments/assets/55f36653-9451-48a1-b4fc-9d419622fe2b"> | <img width="247" alt="Screenshot 2024-10-03 at 16 12 25" src="https://github.com/user-attachments/assets/5cc1af46-4418-40de-9391-6dc5168e08af"> | <img width="247" alt="Screenshot 2024-10-03 at 16 12 34" src="https://github.com/user-attachments/assets/d2ec614a-fbeb-4717-8aeb-328e3a890cd6"> | <img width="247" alt="Screenshot 2024-10-03 at 16 12 40" src="https://github.com/user-attachments/assets/5d634fbd-45c9-4612-9dcf-25cf8b9295a0"> | <img width="247" alt="Screenshot 2024-10-03 at 16 12 47" src="https://github.com/user-attachments/assets/0881dd80-ff98-4ba2-992e-933d549eed19"> | https://github.com/user-attachments/assets/fb36d030-ef53-493d-a8cc-38be670ee504 Nested lists are rendered as ViewGroups, thus the check for ReactScrollView fails. This PR fixes the issue by checking wether the view is an indirect child of a ScrollView with [removeClippedSubviews](https://reactnative.dev/docs/optimizing-flatlist-configuration#removeclippedsubviews) enabled. Fixes: #2341 - added `isInsideScrollViewWithRemoveClippedSubviews` check to `startTransitionRecursive` - modified `Test2282.tsx` repro <!-- Here you can add screenshots / GIFs documenting your change. You can add before / after section if you're changing some behavior. --> - use `Test2282.tsx` repro - [x] Included code example that can be used to test this change - [x] Ensured that CI passes (cherry picked from commit b67af86)
1 parent 173110d commit d83d4b7

File tree

3 files changed

+161
-21
lines changed

3 files changed

+161
-21
lines changed

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ import com.facebook.react.bridge.GuardedRunnable
1717
import com.facebook.react.bridge.ReactContext
1818
import com.facebook.react.uimanager.UIManagerHelper
1919
import com.facebook.react.uimanager.UIManagerModule
20-
import com.facebook.react.views.scroll.ReactScrollView
2120
import com.swmansion.rnscreens.events.HeaderHeightChangeEvent
21+
import com.swmansion.rnscreens.ext.isInsideScrollViewWithRemoveClippedSubviews
2222

2323
@SuppressLint("ViewConstructor") // Only we construct this view, it is never inflated.
2424
class Screen(
@@ -311,10 +311,10 @@ class Screen(
311311
}
312312
if (child is ViewGroup) {
313313
// The children are miscounted when there's a FlatList with
314-
// removeCLippedSubviews set to true (default).
314+
// removeClippedSubviews set to true (default).
315315
// We add a simple view for each item in the list to make it work as expected.
316-
// See https://github.com/software-mansion/react-native-screens/issues/2282
317-
if (it is ReactScrollView && it.removeClippedSubviews) {
316+
// See https://github.com/software-mansion/react-native-screens/pull/2383
317+
if (child.isInsideScrollViewWithRemoveClippedSubviews()) {
318318
for (j in 0 until child.childCount) {
319319
child.addView(View(context))
320320
}

‎android/src/main/java/com/swmansion/rnscreens/ext/ViewExt.kt‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
package com.swmansion.rnscreens.ext
2+
3+
import android.graphics.drawable.ColorDrawable
4+
import android.view.View
5+
import android.view.ViewGroup
6+
import com.facebook.react.views.scroll.ReactHorizontalScrollView
7+
import com.facebook.react.views.scroll.ReactScrollView
8+
import com.swmansion.rnscreens.ScreenStack
9+
10+
internal fun View.parentAsView() = this.parent as? View
11+
12+
internal fun View.parentAsViewGroup() = this.parent as? ViewGroup
13+
14+
internal fun View.recycle(): View {
15+
// screen fragments reuse view instances instead of creating new ones. In order to reuse a given
16+
// view it needs to be detached from the view hierarchy to allow the fragment to attach it back.
17+
this.parentAsViewGroup()?.let { parent ->
18+
parent.endViewTransition(this)
19+
parent.removeView(this)
20+
}
21+
22+
// view detached from fragment manager get their visibility changed to GONE after their state is
23+
// dumped. Since we don't restore the state but want to reuse the view we need to change
24+
// visibility back to VISIBLE in order for the fragment manager to animate in the view.
25+
this.visibility = View.VISIBLE
26+
return this
27+
}
28+
29+
internal fun View.maybeBgColor(): Int? {
30+
val bgDrawable = this.background
31+
if (bgDrawable is ColorDrawable) {
32+
return bgDrawable.color
33+
}
34+
return null
35+
}
36+
37+
internal fun View.isInsideScrollViewWithRemoveClippedSubviews(): Boolean {
38+
if (this is ReactHorizontalScrollView || this is ReactScrollView) {
39+
return false
40+
}
41+
var parentView = this.parent
42+
while (parentView is ViewGroup && parentView !is ScreenStack) {
43+
if (parentView is ReactScrollView) {
44+
return parentView.removeClippedSubviews
45+
}
46+
parentView = parentView.parent
47+
}
48+
return false
49+
}

‎apps/src/tests/Test2282.tsx‎

Lines changed: 108 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,123 @@
1+
import { NavigationContainer } from '@react-navigation/native';
12
import React from 'react';
2-
import { FlatList, Button, Text } from 'react-native';
33
import { createNativeStackNavigator } from '@react-navigation/native-stack';
4-
import { NavigationContainer } from '@react-navigation/native';
4+
import { enableScreens } from 'react-native-screens';
5+
import {
6+
StyleSheet,
7+
Text,
8+
View,
9+
FlatList,
10+
Button,
11+
ViewProps,
12+
Image,
13+
FlatListProps,
14+
} from 'react-native';
515

6-
const Stack = createNativeStackNavigator();
16+
enableScreens(true);
17+
18+
function Item({ children, ...props }: ViewProps) {
19+
return (
20+
<View style={styles.item} {...props}>
21+
<Image source={require('../assets/trees.jpg')} style={styles.image} />
22+
<Text style={styles.text}>{children}</Text>
23+
</View>
24+
);
25+
}
26+
27+
function Home({ navigation }: any) {
28+
return (
29+
<View style={styles.container}>
30+
<Button title="Go to List" onPress={() => navigation.navigate('List')} />
31+
</View>
32+
);
33+
}
34+
35+
function ListScreen() {
36+
return (
37+
<FlatList
38+
data={Array.from({ length: 30 }).fill(0)}
39+
renderItem={({ index }) => {
40+
if (index === 15) {
41+
return <NestedFlatlist key={index} />;
42+
} else if (index === 18) {
43+
return <ExtraNestedFlatlist key={index} />;
44+
} else if (index === 26) {
45+
return <NestedFlatlist key={index} horizontal />;
46+
} else if (index === 28) {
47+
return <ExtraNestedFlatlist key={index} horizontal />;
48+
} else {
49+
return <Item key={index}>List item {index + 1}</Item>;
50+
}
51+
}}
52+
/>
53+
);
54+
}
755

8-
const First = ({ navigation }: any) => (
9-
<Button onPress={() => navigation.navigate('Second')} title="Navigate" />
10-
);
56+
function NestedFlatlist(props: Partial<FlatListProps<number>>) {
57+
return (
58+
<FlatList
59+
style={[styles.nestedList, props.style]}
60+
data={Array.from({ length: 10 }).fill(0) as number[]}
61+
renderItem={({ index }) => (
62+
<Item key={'nested' + index}>Nested list item {index + 1}</Item>
63+
)}
64+
{...props}
65+
/>
66+
);
67+
}
1168

12-
const Second = ({ navigation }: any) => (
13-
<>
69+
function ExtraNestedFlatlist(props: Partial<FlatListProps<number>>) {
70+
return (
1471
<FlatList
15-
renderItem={({ item }) => <Text>{item}</Text>}
16-
data={[1,2,3,4,5,6]}
17-
keyExtractor={item => item.toString()}
72+
style={styles.nestedList}
73+
data={Array.from({ length: 10 }).fill(0) as number[]}
74+
renderItem={({ index }) =>
75+
index === 4 ? (
76+
<NestedFlatlist key={index} style={{ backgroundColor: '#d24729' }} />
77+
) : (
78+
<Item key={'nested' + index}>Nested list item {index + 1}</Item>
79+
)
80+
}
81+
{...props}
1882
/>
19-
<Button onPress={navigation.goBack} title="Go back" />
20-
</>
21-
);
83+
);
84+
}
2285

23-
export default function App() {
86+
const Stack = createNativeStackNavigator();
87+
88+
export default function App(): React.JSX.Element {
2489
return (
2590
<NavigationContainer>
2691
<Stack.Navigator>
27-
<Stack.Screen name="First" component={First} />
28-
<Stack.Screen name="Second" component={Second} />
92+
<Stack.Screen name="Home" component={Home} />
93+
<Stack.Screen name="List" component={ListScreen} />
2994
</Stack.Navigator>
3095
</NavigationContainer>
3196
);
3297
}
98+
99+
const styles = StyleSheet.create({
100+
container: {
101+
flex: 1,
102+
alignItems: 'center',
103+
justifyContent: 'center',
104+
},
105+
nestedList: {
106+
backgroundColor: '#FFA07A',
107+
},
108+
item: {
109+
flexDirection: 'row',
110+
alignItems: 'center',
111+
padding: 10,
112+
gap: 10,
113+
},
114+
text: {
115+
fontSize: 24,
116+
fontWeight: 'bold',
117+
color: 'black',
118+
},
119+
image: {
120+
width: 50,
121+
height: 50,
122+
},
123+
});

0 commit comments

Comments
 (0)