Skip to content

Commit

Permalink
fix(Android): going back on fabric with nested list (#2383)
Browse files Browse the repository at this point in the history
## Description

This PR intents to fix crash happening on Android (fabric) when
navigating back from a screen with nested list.

### Before 
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">
|

### After


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

## Changes

- added `isInsideScrollViewWithRemoveClippedSubviews` check to
`startTransitionRecursive`
- modified `Test2282.tsx` repro

<!--

## 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

- use `Test2282.tsx` repro

## Checklist

- [x] Included code example that can be used to test this change
- [x] Ensured that CI passes
  • Loading branch information
alduzy authored Oct 9, 2024
1 parent 8d25f4c commit b67af86
Show file tree
Hide file tree
Showing 3 changed files with 129 additions and 21 deletions.
8 changes: 4 additions & 4 deletions android/src/main/java/com/swmansion/rnscreens/Screen.kt
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@ 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.google.android.material.shape.CornerFamily
import com.google.android.material.shape.MaterialShapeDrawable
import com.google.android.material.shape.ShapeAppearanceModel
import com.swmansion.rnscreens.events.HeaderHeightChangeEvent
import com.swmansion.rnscreens.events.SheetDetentChangedEvent
import com.swmansion.rnscreens.ext.isInsideScrollViewWithRemoveClippedSubviews

@SuppressLint("ViewConstructor") // Only we construct this view, it is never inflated.
class Screen(
Expand Down Expand Up @@ -393,10 +393,10 @@ class Screen(
}
if (child is ViewGroup) {
// The children are miscounted when there's a FlatList with
// removeCLippedSubviews set to true (default).
// 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) {
// See https://github.com/software-mansion/react-native-screens/pull/2383
if (child.isInsideScrollViewWithRemoveClippedSubviews()) {
for (j in 0 until child.childCount) {
child.addView(View(context))
}
Expand Down
17 changes: 17 additions & 0 deletions android/src/main/java/com/swmansion/rnscreens/ext/ViewExt.kt
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ package com.swmansion.rnscreens.ext
import android.graphics.drawable.ColorDrawable
import android.view.View
import android.view.ViewGroup
import com.facebook.react.views.scroll.ReactHorizontalScrollView
import com.facebook.react.views.scroll.ReactScrollView
import com.swmansion.rnscreens.ScreenStack

internal fun View.parentAsView() = this.parent as? View

Expand Down Expand Up @@ -30,3 +33,17 @@ internal fun View.maybeBgColor(): Int? {
}
return null
}

internal fun View.isInsideScrollViewWithRemoveClippedSubviews(): Boolean {
if (this is ReactHorizontalScrollView || this is ReactScrollView) {
return false
}
var parentView = this.parent
while (parentView is ViewGroup && parentView !is ScreenStack) {
if (parentView is ReactScrollView) {
return parentView.removeClippedSubviews
}
parentView = parentView.parent
}
return false
}
125 changes: 108 additions & 17 deletions apps/src/tests/Test2282.tsx
Original file line number Diff line number Diff line change
@@ -1,32 +1,123 @@
import { NavigationContainer } from '@react-navigation/native';
import React from 'react';
import { FlatList, Button, Text } from 'react-native';
import { createNativeStackNavigator } from '@react-navigation/native-stack';
import { NavigationContainer } from '@react-navigation/native';
import { enableScreens } from 'react-native-screens';
import {
StyleSheet,
Text,
View,
FlatList,
Button,
ViewProps,
Image,
FlatListProps,
} from 'react-native';

const Stack = createNativeStackNavigator();
enableScreens(true);

function Item({ children, ...props }: ViewProps) {
return (
<View style={styles.item} {...props}>
<Image source={require('../assets/trees.jpg')} style={styles.image} />
<Text style={styles.text}>{children}</Text>
</View>
);
}

function Home({ navigation }: any) {
return (
<View style={styles.container}>
<Button title="Go to List" onPress={() => navigation.navigate('List')} />
</View>
);
}

function ListScreen() {
return (
<FlatList
data={Array.from({ length: 30 }).fill(0)}
renderItem={({ index }) => {
if (index === 15) {
return <NestedFlatlist key={index} />;
} else if (index === 18) {
return <ExtraNestedFlatlist key={index} />;
} else if (index === 26) {
return <NestedFlatlist key={index} horizontal />;
} else if (index === 28) {
return <ExtraNestedFlatlist key={index} horizontal />;
} else {
return <Item key={index}>List item {index + 1}</Item>;
}
}}
/>
);
}

const First = ({ navigation }: any) => (
<Button onPress={() => navigation.navigate('Second')} title="Navigate" />
);
function NestedFlatlist(props: Partial<FlatListProps<number>>) {
return (
<FlatList
style={[styles.nestedList, props.style]}
data={Array.from({ length: 10 }).fill(0) as number[]}
renderItem={({ index }) => (
<Item key={'nested' + index}>Nested list item {index + 1}</Item>
)}
{...props}
/>
);
}

const Second = ({ navigation }: any) => (
<>
function ExtraNestedFlatlist(props: Partial<FlatListProps<number>>) {
return (
<FlatList
renderItem={({ item }) => <Text>{item}</Text>}
data={[1,2,3,4,5,6]}
keyExtractor={item => item.toString()}
style={styles.nestedList}
data={Array.from({ length: 10 }).fill(0) as number[]}
renderItem={({ index }) =>
index === 4 ? (
<NestedFlatlist key={index} style={{ backgroundColor: '#d24729' }} />
) : (
<Item key={'nested' + index}>Nested list item {index + 1}</Item>
)
}
{...props}
/>
<Button onPress={navigation.goBack} title="Go back" />
</>
);
);
}

export default function App() {
const Stack = createNativeStackNavigator();

export default function App(): React.JSX.Element {
return (
<NavigationContainer>
<Stack.Navigator>
<Stack.Screen name="First" component={First} />
<Stack.Screen name="Second" component={Second} />
<Stack.Screen name="Home" component={Home} />
<Stack.Screen name="List" component={ListScreen} />
</Stack.Navigator>
</NavigationContainer>
);
}

const styles = StyleSheet.create({
container: {
flex: 1,
alignItems: 'center',
justifyContent: 'center',
},
nestedList: {
backgroundColor: '#FFA07A',
},
item: {
flexDirection: 'row',
alignItems: 'center',
padding: 10,
gap: 10,
},
text: {
fontSize: 24,
fontWeight: 'bold',
color: 'black',
},
image: {
width: 50,
height: 50,
},
});

0 comments on commit b67af86

Please sign in to comment.