Skip to content

Commit 88d9794

Browse files
hirbodkkafaradrianryt
authored
fix(Paper,iOS): dismiss all attached view controllers correctly on reload (#2175)
## Description This PR addresses an issue with react-native-screens where modals were not being dismissed correctly during app reloads when combined with foreign view controllers. The fix involves enhancing the invalidate method to recursively dismiss all presented view controllers, ensuring a clean state on reload. This is not a development-only problem; this fix also addresses reloads from OTA updates. ## Changes - Enhanced invalidate method in RNSScreenStack.mm to recursively dismiss all presented view controllers. - Ensured _presentedModals is cleared and _controller is detached from its parent during invalidation. - Added a helper method dismissAllPresentedViewControllersFrom to handle recursive dismissal logic. ## Screenshots / GIFs | Before | After | |--------|--------| | <video width="320" height="240" controls src="https://github.com/software-mansion/react-native-screens/assets/504909/1476ab3a-4bd9-4ffa-9256-760467a108bc"></video> | <video width="320" height="240" controls src="https://github.com/software-mansion/react-native-screens/assets/504909/e6c5eef0-16c6-49a2-9124-86467feec7f2"></video> | The red background is from a `transparentModal` by RNS, the sheet is a foreign view controller. Before the change, `react-native-screens` would break on reload if a foreign view controller was mounted on top. I came to the solution after finding [this PR](#2113). The issue originally started [here](lodev09/react-native-true-sheet#34). After my changes, RNS works correctly on reload with third-party controllers. I have no experience with Fabric, so I can't help with that. Feel free to update the solution for Fabric if needed. ## Test code and steps to reproduce Test2175 ## Checklist - [x] Included code example that can be used to test this change - [x] Ensured that CI passes --------- Co-authored-by: Kacper Kafara <kacperkafara@gmail.com> Co-authored-by: adrianryt <adrianryt3@gmail.com>
1 parent 437f6ea commit 88d9794

File tree

4 files changed

+172
-10
lines changed

4 files changed

+172
-10
lines changed

‎apps/src/tests/Test2175.tsx‎

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
import React from 'react';
2+
import { Button, Modal, SafeAreaView, Text, View } from 'react-native';
3+
4+
import { createNativeStackNavigator } from '@react-navigation/native-stack';
5+
import { NavigationContainer } from '@react-navigation/native';
6+
import { createBottomTabNavigator } from '@react-navigation/bottom-tabs';
7+
8+
const Stack = createNativeStackNavigator();
9+
const Tabs = createBottomTabNavigator();
10+
11+
12+
function TabScreens({ navigation }): React.JSX.Element {
13+
return (
14+
<Tabs.Navigator>
15+
<Tabs.Screen name='A' component={HomeScreen} />
16+
<Tabs.Screen name='B' component={TabHomeScreen} />
17+
</Tabs.Navigator>
18+
);
19+
}
20+
21+
function TabHomeScreen({ navigation }): React.JSX.Element {
22+
return (
23+
<View style={{ flex: 1, backgroundColor: 'lightcoral', justifyContent: 'center', }}>
24+
<Text>Where do you where do you go, my lovely, oh oh oh oh</Text>
25+
<Button title='Show owned transparentModal (outer navigator)' onPress={() => { navigation.navigate('TransparentModal') }} />
26+
<Button title='Show owned modal (outer navigator)' onPress={() => { navigation.navigate('Modal') }} />
27+
</View>
28+
)
29+
}
30+
31+
32+
function HomeScreen({ navigation }): React.JSX.Element {
33+
const [toggle, setToggle] = React.useState(false);
34+
return (
35+
<View style={{ flex: 1, backgroundColor: 'lightseagreen', justifyContent: 'center', }}>
36+
<Text>Where do you where do you go, my lovely, oh oh oh oh</Text>
37+
<Button title='Show owned transparentModal' onPress={() => { navigation.navigate('TransparentModal') }} />
38+
<Button title='Show owned modal' onPress={() => { navigation.navigate('Modal') }} />
39+
<Button title='Show tabs' onPress={() => { navigation.navigate('Tabs') }} />
40+
<Button title='Show foreign modal' onPress={() => { setToggle(old => !old) }} />
41+
<Modal
42+
visible={toggle}
43+
onRequestClose={() => setToggle(false)}
44+
presentationStyle='formSheet'
45+
animationType='slide'
46+
>
47+
<View style={{ flex: 1, backgroundColor: 'lightblue' }}>
48+
<Text>Hello I'm a foreign modal</Text>
49+
<Button title='Close foreign modal' onPress={() => { setToggle(false) }} />
50+
</View>
51+
</Modal>
52+
</View>
53+
)
54+
}
55+
56+
function ModalScreen({ navigation }): React.JSX.Element {
57+
const [toggle2, setToggle2] = React.useState(false);
58+
59+
return (
60+
<View style={{ flex: 1, backgroundColor: 'lightcoral', opacity: 0.4 }}>
61+
<Text>Where do you where do you go, my lovely, oh oh oh oh</Text>
62+
<Button title='Go back' onPress={() => { navigation.goBack() }} />
63+
<View style={{ width: '100%', height: 50, backgroundColor: 'red' }} />
64+
<Button title='Push another Modal' onPress={() => { navigation.push('Modal') }} />
65+
<Button title='Push foreign modal(inside Screen Component)' onPress={() => { navigation.push('ForeignModal')}} />
66+
<Button title='Push foreign modal' onPress={() => { setToggle2(old => !old )}} />
67+
<Modal
68+
visible={toggle2}
69+
onRequestClose={() => setToggle2(false)}
70+
presentationStyle='formSheet'
71+
animationType='slide'
72+
>
73+
<View style={{ flex: 1, backgroundColor: 'lightblue' }}>
74+
<Text>Hello I'm a foreign modal</Text>
75+
<Button title='Close foreign modal' onPress={() => { setToggle2(false) }} />
76+
</View>
77+
</Modal>
78+
</View>
79+
)
80+
}
81+
82+
function ForeignModal({ navigation }): React.JSX.Element | null {
83+
const [toggle, setToggle] = React.useState(false);
84+
return (
85+
<Modal
86+
visible={toggle}
87+
onRequestClose={() => setToggle(false)}
88+
presentationStyle='formSheet'
89+
animationType='slide'
90+
>
91+
<View style={{ flex: 1, backgroundColor: 'lightblue' }}>
92+
<Text>Hello I'm a foreign modal</Text>
93+
<Button title='Close foreign modal' onPress={() => { setToggle(false) }} />
94+
</View>
95+
</Modal>
96+
)
97+
}
98+
99+
100+
const TestScreen = ({ navigation }): React.JSX.Element => {
101+
return (
102+
<SafeAreaView>
103+
<Button
104+
title={
105+
'Click me and drag around a bit and I should log something still'
106+
}
107+
onPress={() => {
108+
console.log(Date.now());
109+
}}
110+
/>
111+
<Button
112+
title={'Navigate to modal'}
113+
onPress={() => {
114+
navigation.navigate('Test2');
115+
}}
116+
/>
117+
</SafeAreaView>
118+
);
119+
};
120+
121+
function App(): React.JSX.Element {
122+
return (
123+
<NavigationContainer>
124+
<Stack.Navigator initialRouteName='Tabs'>
125+
<Stack.Screen name='Tabs' component={TabScreens} options={{ headerShown: false }} />
126+
<Stack.Screen
127+
name="Home"
128+
component={HomeScreen}
129+
/>
130+
<Stack.Screen
131+
name="TransparentModal"
132+
component={ModalScreen}
133+
options={{
134+
presentation: 'transparentModal',
135+
}}
136+
/>
137+
<Stack.Screen
138+
name="Modal"
139+
component={ModalScreen}
140+
options={{
141+
presentation: 'modal',
142+
headerShown: true,
143+
}}
144+
/>
145+
<Stack.Screen name={"ForeignModal"} component={ForeignModal} />
146+
</Stack.Navigator>
147+
</NavigationContainer>
148+
);
149+
}
150+
151+
export default App;

‎apps/src/tests/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ export { default as Test2028 } from './Test2028';
101101
export { default as Test2048 } from './Test2048';
102102
export { default as Test2069 } from './Test2069';
103103
export { default as Test2118 } from './Test2118';
104+
export { default as Test2175 } from './Test2175';
104105
export { default as Test2184 } from './Test2184';
105106
export { default as Test2223 } from './Test2223';
106107
export { default as Test2227 } from './Test2227';

‎ios/RNSScreenStack.mm‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1273,12 +1273,25 @@ - (void)prepareForRecycle
12731273
- (void)invalidate
12741274
{
12751275
_invalidated = YES;
1276-
for (UIViewController *controller in _presentedModals) {
1276+
[self dismissAllRelatedModals];
1277+
[_controller willMoveToParentViewController:nil];
1278+
[_controller removeFromParentViewController];
1279+
}
1280+
1281+
// This method aims to dismiss all modals for which presentation process
1282+
// has been initiated in this navigation controller, i. e. either a Screen
1283+
// with modal presentation or foreign modal presented from inside a Screen.
1284+
- (void)dismissAllRelatedModals
1285+
{
1286+
[_controller dismissViewControllerAnimated:NO completion:nil];
1287+
1288+
// This loop seems to be excessive. Above message send to `_controller` should
1289+
// be enough, because system dismisses the controllers recursively,
1290+
// however better safe than sorry & introduce a regression, thus it is left here.
1291+
for (UIViewController *controller in [_presentedModals reverseObjectEnumerator]) {
12771292
[controller dismissViewControllerAnimated:NO completion:nil];
12781293
}
12791294
[_presentedModals removeAllObjects];
1280-
[_controller willMoveToParentViewController:nil];
1281-
[_controller removeFromParentViewController];
12821295
}
12831296

12841297
#endif // RCT_NEW_ARCH_ENABLED

‎src/components/Screen.tsx‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -373,15 +373,12 @@ export const InnerScreen = React.forwardRef<View, ScreenProps>(
373373
// e.g. to use `useReanimatedTransitionProgress` (see `reanimated` folder in repo)
374374
export const ScreenContext = React.createContext(InnerScreen);
375375

376-
const Screen = React.forwardRef<View, ScreenProps>(
377-
(props, ref) => {
378-
const ScreenWrapper = React.useContext(ScreenContext) || InnerScreen;
376+
const Screen = React.forwardRef<View, ScreenProps>((props, ref) => {
377+
const ScreenWrapper = React.useContext(ScreenContext) || InnerScreen;
379378

380-
return <ScreenWrapper {...props} ref={ref} />;
381-
}
382-
);
379+
return <ScreenWrapper {...props} ref={ref} />;
380+
});
383381

384382
Screen.displayName = 'Screen';
385383

386-
387384
export default Screen;

0 commit comments

Comments
 (0)