-
-
Notifications
You must be signed in to change notification settings - Fork 575
fix: issues with presenting owned modals from foreign ones #2113
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,85 @@ | ||
import React from 'react'; | ||
import { View, Modal, Button, TouchableWithoutFeedback } from 'react-native'; | ||
import { useState } from 'react'; | ||
|
||
import { NavigationContainer, useNavigation } from '@react-navigation/native'; | ||
import { createNativeStackNavigator } from '@react-navigation/native-stack'; | ||
|
||
type AppStackPages = { | ||
Home: undefined; | ||
Modal: undefined; | ||
}; | ||
|
||
function HomeScreen() { | ||
const navigation = useNavigation(); | ||
const [visible, setVisible] = useState(false); | ||
|
||
return ( | ||
<View style={{ flex: 1, justifyContent: 'center', alignItems: 'center' }}> | ||
<Button | ||
title="Toggle bottom modal" | ||
onPress={() => setVisible(prev => !prev)} | ||
/> | ||
<Modal animationType="slide" visible={visible} transparent> | ||
<TouchableWithoutFeedback onPress={() => setVisible(false)}> | ||
<View style={{ flex: 1 }} /> | ||
</TouchableWithoutFeedback> | ||
<View | ||
style={{ | ||
borderTopLeftRadius: 10, | ||
borderTopRightRadius: 10, | ||
borderWidth: 2, | ||
borderColor: 'red', | ||
padding: 10, | ||
minHeight: '40%', | ||
alignItems: 'center', | ||
justifyContent: 'center', | ||
}}> | ||
<Button | ||
title="Open navigation modal" | ||
onPress={() => { | ||
// Issue: autohiding the Modal that serves as a bottom sheet unmounts | ||
// the anchor component for the screen that is in { presentation: "modal" } mode | ||
// Previously the anchoring component for a { presentation: "modal" }-based screen was different and it worked | ||
// 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 | ||
// adding setTimeout does not bring any good, because | ||
// - we either don't see navigation action | ||
// - we unmount both the bottom sheet modal and the screen itself | ||
|
||
setVisible(false); | ||
|
||
navigation.navigate('Modal'); | ||
}} | ||
/> | ||
</View> | ||
</Modal> | ||
</View> | ||
); | ||
} | ||
|
||
function ModalScreen() { | ||
return <View style={{ flex: 1, backgroundColor: 'rgb(50,150,50)' }} />; | ||
} | ||
|
||
const AppStack = createNativeStackNavigator<AppStackPages>(); | ||
|
||
function Navigation() { | ||
return ( | ||
<AppStack.Navigator> | ||
<AppStack.Screen name="Home" component={HomeScreen} /> | ||
<AppStack.Screen | ||
name="Modal" | ||
component={ModalScreen} | ||
options={{ presentation: 'modal' }} | ||
/> | ||
</AppStack.Navigator> | ||
); | ||
} | ||
|
||
export default function App() { | ||
return ( | ||
<NavigationContainer> | ||
<Navigation /> | ||
</NavigationContainer> | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,85 @@ | ||
import React from 'react'; | ||
import { View, Modal, Button, TouchableWithoutFeedback } from 'react-native'; | ||
import { useState } from 'react'; | ||
|
||
import { NavigationContainer, useNavigation } from '@react-navigation/native'; | ||
import { createNativeStackNavigator } from '@react-navigation/native-stack'; | ||
|
||
type AppStackPages = { | ||
Home: undefined; | ||
Modal: undefined; | ||
}; | ||
|
||
function HomeScreen() { | ||
const navigation = useNavigation(); | ||
const [visible, setVisible] = useState(false); | ||
|
||
return ( | ||
<View style={{ flex: 1, justifyContent: 'center', alignItems: 'center' }}> | ||
<Button | ||
title="Toggle bottom modal" | ||
onPress={() => setVisible(prev => !prev)} | ||
/> | ||
<Modal animationType="slide" visible={visible} transparent> | ||
<TouchableWithoutFeedback onPress={() => setVisible(false)}> | ||
<View style={{ flex: 1 }} /> | ||
</TouchableWithoutFeedback> | ||
<View | ||
style={{ | ||
borderTopLeftRadius: 10, | ||
borderTopRightRadius: 10, | ||
borderWidth: 2, | ||
borderColor: 'red', | ||
padding: 10, | ||
minHeight: '40%', | ||
alignItems: 'center', | ||
justifyContent: 'center', | ||
}}> | ||
<Button | ||
title="Open navigation modal" | ||
onPress={() => { | ||
// Issue: autohiding the Modal that serves as a bottom sheet unmounts | ||
// the anchor component for the screen that is in { presentation: "modal" } mode | ||
// Previously the anchoring component for a { presentation: "modal" }-based screen was different and it worked | ||
// 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 | ||
// adding setTimeout does not bring any good, because | ||
// - we either don't see navigation action | ||
// - we unmount both the bottom sheet modal and the screen itself | ||
|
||
setVisible(false); | ||
|
||
navigation.navigate('Modal'); | ||
}} | ||
/> | ||
</View> | ||
</Modal> | ||
</View> | ||
); | ||
} | ||
|
||
function ModalScreen() { | ||
return <View style={{ flex: 1, backgroundColor: 'rgb(50,150,50)' }} />; | ||
} | ||
|
||
const AppStack = createNativeStackNavigator<AppStackPages>(); | ||
|
||
function Navigation() { | ||
return ( | ||
<AppStack.Navigator> | ||
<AppStack.Screen name="Home" component={HomeScreen} /> | ||
<AppStack.Screen | ||
name="Modal" | ||
component={ModalScreen} | ||
options={{ presentation: 'modal' }} | ||
/> | ||
</AppStack.Navigator> | ||
); | ||
} | ||
|
||
export default function App() { | ||
return ( | ||
<NavigationContainer> | ||
<Navigation /> | ||
</NavigationContainer> | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -382,9 +382,13 @@ - (void)setModalViewControllers:(NSArray<UIViewController *> *)controllers | |
[newControllers removeObjectsInArray:_presentedModals]; | ||
|
||
// We need to find bottom-most view controller that should stay on the stack | ||
// for the duration of transition. There are couple of scenarios: | ||
// for the duration of transition. | ||
// We will want to dismiss all | ||
|
||
// There are couple of scenarios: | ||
// (1) No modals are presented or all modals were presented by this RNSNavigationController, | ||
// (2) There are modals presented by other RNSNavigationControllers (nested/outer) | ||
kkafar marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// (3) There are modals presented by other controllers (e.g. React Native's Modal view) | ||
|
||
// Last controller that is common for both _presentedModals & controllers | ||
__block UIViewController *changeRootController = _controller; | ||
|
@@ -479,16 +483,35 @@ - (void)setModalViewControllers:(NSArray<UIViewController *> *)controllers | |
} | ||
}; | ||
|
||
// changeRootController is the last controller that *is owned by this stack*, and should stay unchanged after this | ||
// batch of transitions. Therefore changeRootController.presentedViewController is the first constroller to be | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
// dismissed (implying also all controllers above). Notice here, that firstModalToBeDismissed could have been | ||
// RNSScreen modal presented from *this* stack, another stack, or any other view controller with modal presentation | ||
// provided by third-party libraries (e.g. React Native's Modal view). In case of presence of other (not managed by | ||
// us) modal controllers, weird interactions might arise. The code below, besides handling our presentation / | ||
// dismissal logic also attempts to handle possible wide gamut of cases of interactions with third-party modal | ||
// controllers, however it's not perfect. | ||
// 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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
// computing required operations). | ||
kkafar marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
UIViewController *firstModalToBeDismissed = changeRootController.presentedViewController; | ||
|
||
if (firstModalToBeDismissed != nil) { | ||
BOOL shouldAnimate = changeRootIndex == controllers.count && | ||
[firstModalToBeDismissed isKindOfClass:[RNSScreen class]] && | ||
((RNSScreen *)firstModalToBeDismissed).screenView.stackAnimation != RNSScreenStackAnimationNone; | ||
|
||
if ([_presentedModals containsObject:firstModalToBeDismissed]) { | ||
if ([_presentedModals containsObject:firstModalToBeDismissed] || | ||
![firstModalToBeDismissed isKindOfClass:RNSScreen.class]) { | ||
kkafar marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// We dismiss every VC that was presented by changeRootController VC or its descendant. | ||
// After the series of dismissals is completed we run completion block in which | ||
// we present modals on top of changeRootController (which may be the this stack VC) | ||
// | ||
// There also might the second case, where the firstModalToBeDismissed is foreign. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
// See: https://github.com/software-mansion/react-native-screens/issues/2048 | ||
// For now, to mitigate the issue, we also decide to trigger its dismissal before | ||
// starting the presentation chain down below in finish() callback. | ||
[changeRootController dismissViewControllerAnimated:shouldAnimate completion:finish]; | ||
return; | ||
} | ||
|
Uh oh!
There was an error while loading. Please reload this page.