Skip to content

Commit c21de50

Browse files
kkafarkkafar
authored
fix: issues with presenting owned modals from foreign ones (#2113)
## Description Basically this is another edition of the issue #1829 (handled by #1912). The issue comes down to the fact, that our `ScreenStack` is not aware of all modal view controllers being in presentation, but this time it is not aware of third-party modal view controllers (I've named them "foreign" modals in opposite to "owned" modals). This PR is not a comprehensive solution but rather just a patch aiming at fixing one particular interaction reported in #2048. I've left verbose code comments explaining the issue and suggesting solution in the source code, including: ``` // TODO: Find general way to manage owned and foreign modal view controllers and refactor this code. Consider building // model first (data structue, attempting to be aware of all modals in presentation and some text-like algorithm for // computing required operations). ``` Closes #2048 Closes #2085 ## Changes Trigger dissmisal of foreign modal if it is presented above `changeRoot` modal (last modal that is to stay on stack after the updates). ## Test code and steps to reproduce `Test2048` in `TestsExample` & `FabricTestExample`. ## Checklist - [x] Included code example that can be used to test this change - [x] Ensured that CI passes
1 parent f8a45e1 commit c21de50

File tree

5 files changed

+198
-4
lines changed

5 files changed

+198
-4
lines changed

‎FabricTestExample/App.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ import TestScreenAnimation from './src/TestScreenAnimation';
9494
import Test1981 from './src/Test1981';
9595
import Test2008 from './src/Test2008';
9696
import Test2028 from './src/Test2028';
97+
import Test2048 from './src/Test2048';
9798
import Test2069 from './src/Test2069';
9899

99100
enableFreeze(true);

‎FabricTestExample/src/Test2048.tsx‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
import React from 'react';
2+
import { View, Modal, Button, TouchableWithoutFeedback } from 'react-native';
3+
import { useState } from 'react';
4+
5+
import { NavigationContainer, useNavigation } from '@react-navigation/native';
6+
import { createNativeStackNavigator } from '@react-navigation/native-stack';
7+
8+
type AppStackPages = {
9+
Home: undefined;
10+
Modal: undefined;
11+
};
12+
13+
function HomeScreen() {
14+
const navigation = useNavigation();
15+
const [visible, setVisible] = useState(false);
16+
17+
return (
18+
<View style={{ flex: 1, justifyContent: 'center', alignItems: 'center' }}>
19+
<Button
20+
title="Toggle bottom modal"
21+
onPress={() => setVisible(prev => !prev)}
22+
/>
23+
<Modal animationType="slide" visible={visible} transparent>
24+
<TouchableWithoutFeedback onPress={() => setVisible(false)}>
25+
<View style={{ flex: 1 }} />
26+
</TouchableWithoutFeedback>
27+
<View
28+
style={{
29+
borderTopLeftRadius: 10,
30+
borderTopRightRadius: 10,
31+
borderWidth: 2,
32+
borderColor: 'red',
33+
padding: 10,
34+
minHeight: '40%',
35+
alignItems: 'center',
36+
justifyContent: 'center',
37+
}}>
38+
<Button
39+
title="Open navigation modal"
40+
onPress={() => {
41+
// Issue: autohiding the Modal that serves as a bottom sheet unmounts
42+
// the anchor component for the screen that is in { presentation: "modal" } mode
43+
// Previously the anchoring component for a { presentation: "modal" }-based screen was different and it worked
44+
// The culprit is: https://github.com/software-mansion/react-native-screens/pull/1912 released in https://github.com/software-mansion/react-native-screens/releases/tag/3.29.0
45+
// adding setTimeout does not bring any good, because
46+
// - we either don't see navigation action
47+
// - we unmount both the bottom sheet modal and the screen itself
48+
49+
setVisible(false);
50+
51+
navigation.navigate('Modal');
52+
}}
53+
/>
54+
</View>
55+
</Modal>
56+
</View>
57+
);
58+
}
59+
60+
function ModalScreen() {
61+
return <View style={{ flex: 1, backgroundColor: 'rgb(50,150,50)' }} />;
62+
}
63+
64+
const AppStack = createNativeStackNavigator<AppStackPages>();
65+
66+
function Navigation() {
67+
return (
68+
<AppStack.Navigator>
69+
<AppStack.Screen name="Home" component={HomeScreen} />
70+
<AppStack.Screen
71+
name="Modal"
72+
component={ModalScreen}
73+
options={{ presentation: 'modal' }}
74+
/>
75+
</AppStack.Navigator>
76+
);
77+
}
78+
79+
export default function App() {
80+
return (
81+
<NavigationContainer>
82+
<Navigation />
83+
</NavigationContainer>
84+
);
85+
}

‎TestsExample/App.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ import Test1844 from './src/Test1844';
9595
import Test1864 from './src/Test1864';
9696
import Test1981 from './src/Test1981';
9797
import Test2008 from './src/Test2008';
98+
import Test2048 from './src/Test2048';
9899
import Test2069 from './src/Test2069';
99100

100101
enableFreeze(true);

‎TestsExample/src/Test2048.tsx‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
import React from 'react';
2+
import { View, Modal, Button, TouchableWithoutFeedback } from 'react-native';
3+
import { useState } from 'react';
4+
5+
import { NavigationContainer, useNavigation } from '@react-navigation/native';
6+
import { createNativeStackNavigator } from '@react-navigation/native-stack';
7+
8+
type AppStackPages = {
9+
Home: undefined;
10+
Modal: undefined;
11+
};
12+
13+
function HomeScreen() {
14+
const navigation = useNavigation();
15+
const [visible, setVisible] = useState(false);
16+
17+
return (
18+
<View style={{ flex: 1, justifyContent: 'center', alignItems: 'center' }}>
19+
<Button
20+
title="Toggle bottom modal"
21+
onPress={() => setVisible(prev => !prev)}
22+
/>
23+
<Modal animationType="slide" visible={visible} transparent>
24+
<TouchableWithoutFeedback onPress={() => setVisible(false)}>
25+
<View style={{ flex: 1 }} />
26+
</TouchableWithoutFeedback>
27+
<View
28+
style={{
29+
borderTopLeftRadius: 10,
30+
borderTopRightRadius: 10,
31+
borderWidth: 2,
32+
borderColor: 'red',
33+
padding: 10,
34+
minHeight: '40%',
35+
alignItems: 'center',
36+
justifyContent: 'center',
37+
}}>
38+
<Button
39+
title="Open navigation modal"
40+
onPress={() => {
41+
// Issue: autohiding the Modal that serves as a bottom sheet unmounts
42+
// the anchor component for the screen that is in { presentation: "modal" } mode
43+
// Previously the anchoring component for a { presentation: "modal" }-based screen was different and it worked
44+
// The culprit is: https://github.com/software-mansion/react-native-screens/pull/1912 released in https://github.com/software-mansion/react-native-screens/releases/tag/3.29.0
45+
// adding setTimeout does not bring any good, because
46+
// - we either don't see navigation action
47+
// - we unmount both the bottom sheet modal and the screen itself
48+
49+
setVisible(false);
50+
51+
navigation.navigate('Modal');
52+
}}
53+
/>
54+
</View>
55+
</Modal>
56+
</View>
57+
);
58+
}
59+
60+
function ModalScreen() {
61+
return <View style={{ flex: 1, backgroundColor: 'rgb(50,150,50)' }} />;
62+
}
63+
64+
const AppStack = createNativeStackNavigator<AppStackPages>();
65+
66+
function Navigation() {
67+
return (
68+
<AppStack.Navigator>
69+
<AppStack.Screen name="Home" component={HomeScreen} />
70+
<AppStack.Screen
71+
name="Modal"
72+
component={ModalScreen}
73+
options={{ presentation: 'modal' }}
74+
/>
75+
</AppStack.Navigator>
76+
);
77+
}
78+
79+
export default function App() {
80+
return (
81+
<NavigationContainer>
82+
<Navigation />
83+
</NavigationContainer>
84+
);
85+
}

‎ios/RNSScreenStack.mm‎

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -382,9 +382,12 @@ - (void)setModalViewControllers:(NSArray<UIViewController *> *)controllers
382382
[newControllers removeObjectsInArray:_presentedModals];
383383

384384
// We need to find bottom-most view controller that should stay on the stack
385-
// for the duration of transition. There are couple of scenarios:
386-
// (1) No modals are presented or all modals were presented by this RNSNavigationController,
387-
// (2) There are modals presented by other RNSNavigationControllers (nested/outer)
385+
// for the duration of transition.
386+
387+
// There are couple of scenarios:
388+
// (1) no modals are presented or all modals were presented by this RNSNavigationController,
389+
// (2) there are modals presented by other RNSNavigationControllers (nested/outer),
390+
// (3) there are modals presented by other controllers (e.g. React Native's Modal view).
388391

389392
// Last controller that is common for both _presentedModals & controllers
390393
__block UIViewController *changeRootController = _controller;
@@ -479,16 +482,35 @@ - (void)setModalViewControllers:(NSArray<UIViewController *> *)controllers
479482
}
480483
};
481484

485+
// changeRootController is the last controller that *is owned by this stack*, and should stay unchanged after this
486+
// batch of transitions. Therefore changeRootController.presentedViewController is the first constroller to be
487+
// dismissed (implying also all controllers above). Notice here, that firstModalToBeDismissed could have been
488+
// RNSScreen modal presented from *this* stack, another stack, or any other view controller with modal presentation
489+
// provided by third-party libraries (e.g. React Native's Modal view). In case of presence of other (not managed by
490+
// us) modal controllers, weird interactions might arise. The code below, besides handling our presentation /
491+
// dismissal logic also attempts to handle possible wide gamut of cases of interactions with third-party modal
492+
// controllers, however it's not perfect.
493+
// TODO: Find general way to manage owned and foreign modal view controllers and refactor this code. Consider building
494+
// model first (data structue, attempting to be aware of all modals in presentation and some text-like algorithm for
495+
// computing required operations).
496+
482497
UIViewController *firstModalToBeDismissed = changeRootController.presentedViewController;
498+
483499
if (firstModalToBeDismissed != nil) {
484500
BOOL shouldAnimate = changeRootIndex == controllers.count &&
485501
[firstModalToBeDismissed isKindOfClass:[RNSScreen class]] &&
486502
((RNSScreen *)firstModalToBeDismissed).screenView.stackAnimation != RNSScreenStackAnimationNone;
487503

488-
if ([_presentedModals containsObject:firstModalToBeDismissed]) {
504+
if ([_presentedModals containsObject:firstModalToBeDismissed] ||
505+
![firstModalToBeDismissed isKindOfClass:RNSScreen.class]) {
489506
// We dismiss every VC that was presented by changeRootController VC or its descendant.
490507
// After the series of dismissals is completed we run completion block in which
491508
// we present modals on top of changeRootController (which may be the this stack VC)
509+
//
510+
// There also might the second case, where the firstModalToBeDismissed is foreign.
511+
// See: https://github.com/software-mansion/react-native-screens/issues/2048
512+
// For now, to mitigate the issue, we also decide to trigger its dismissal before
513+
// starting the presentation chain down below in finish() callback.
492514
[changeRootController dismissViewControllerAnimated:shouldAnimate completion:finish];
493515
return;
494516
}

0 commit comments

Comments
 (0)