From 88d9794330b30004b8e8fc9e99844958c9f111ef Mon Sep 17 00:00:00 2001
From: Hirbod <504909+hirbod@users.noreply.github.com>
Date: Mon, 28 Oct 2024 17:39:38 +0100
Subject: [PATCH] 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 |
|--------|--------|
|
|
|
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](https://github.com/software-mansion/react-native-screens/pull/2113).
The issue originally started
[here](https://github.com/lodev09/react-native-true-sheet/issues/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
Co-authored-by: adrianryt
---
apps/src/tests/Test2175.tsx | 151 ++++++++++++++++++++++++++++++++++++
apps/src/tests/index.ts | 1 +
ios/RNSScreenStack.mm | 19 ++++-
src/components/Screen.tsx | 11 +--
4 files changed, 172 insertions(+), 10 deletions(-)
create mode 100644 apps/src/tests/Test2175.tsx
diff --git a/apps/src/tests/Test2175.tsx b/apps/src/tests/Test2175.tsx
new file mode 100644
index 0000000000..dbab010c24
--- /dev/null
+++ b/apps/src/tests/Test2175.tsx
@@ -0,0 +1,151 @@
+import React from 'react';
+import { Button, Modal, SafeAreaView, Text, View } from 'react-native';
+
+import { createNativeStackNavigator } from '@react-navigation/native-stack';
+import { NavigationContainer } from '@react-navigation/native';
+import { createBottomTabNavigator } from '@react-navigation/bottom-tabs';
+
+const Stack = createNativeStackNavigator();
+const Tabs = createBottomTabNavigator();
+
+
+function TabScreens({ navigation }): React.JSX.Element {
+ return (
+
+
+
+
+ );
+}
+
+function TabHomeScreen({ navigation }): React.JSX.Element {
+ return (
+
+ Where do you where do you go, my lovely, oh oh oh oh
+
+ )
+}
+
+
+function HomeScreen({ navigation }): React.JSX.Element {
+ const [toggle, setToggle] = React.useState(false);
+ return (
+
+ Where do you where do you go, my lovely, oh oh oh oh
+
+ )
+}
+
+function ModalScreen({ navigation }): React.JSX.Element {
+ const [toggle2, setToggle2] = React.useState(false);
+
+ return (
+
+ Where do you where do you go, my lovely, oh oh oh oh
+
+ )
+}
+
+function ForeignModal({ navigation }): React.JSX.Element | null {
+ const [toggle, setToggle] = React.useState(false);
+ return (
+ setToggle(false)}
+ presentationStyle='formSheet'
+ animationType='slide'
+ >
+
+ Hello I'm a foreign modal
+
+
+ )
+}
+
+
+const TestScreen = ({ navigation }): React.JSX.Element => {
+ return (
+
+
+ );
+};
+
+function App(): React.JSX.Element {
+ return (
+
+
+
+
+
+
+
+
+
+ );
+}
+
+export default App;
diff --git a/apps/src/tests/index.ts b/apps/src/tests/index.ts
index f4142428b7..fa45d8ee7f 100644
--- a/apps/src/tests/index.ts
+++ b/apps/src/tests/index.ts
@@ -101,6 +101,7 @@ export { default as Test2028 } from './Test2028';
export { default as Test2048 } from './Test2048';
export { default as Test2069 } from './Test2069';
export { default as Test2118 } from './Test2118';
+export { default as Test2175 } from './Test2175';
export { default as Test2184 } from './Test2184';
export { default as Test2223 } from './Test2223';
export { default as Test2227 } from './Test2227';
diff --git a/ios/RNSScreenStack.mm b/ios/RNSScreenStack.mm
index 8fa4fbb7be..cc26600522 100644
--- a/ios/RNSScreenStack.mm
+++ b/ios/RNSScreenStack.mm
@@ -1273,12 +1273,25 @@ - (void)prepareForRecycle
- (void)invalidate
{
_invalidated = YES;
- for (UIViewController *controller in _presentedModals) {
+ [self dismissAllRelatedModals];
+ [_controller willMoveToParentViewController:nil];
+ [_controller removeFromParentViewController];
+}
+
+// This method aims to dismiss all modals for which presentation process
+// has been initiated in this navigation controller, i. e. either a Screen
+// with modal presentation or foreign modal presented from inside a Screen.
+- (void)dismissAllRelatedModals
+{
+ [_controller dismissViewControllerAnimated:NO completion:nil];
+
+ // This loop seems to be excessive. Above message send to `_controller` should
+ // be enough, because system dismisses the controllers recursively,
+ // however better safe than sorry & introduce a regression, thus it is left here.
+ for (UIViewController *controller in [_presentedModals reverseObjectEnumerator]) {
[controller dismissViewControllerAnimated:NO completion:nil];
}
[_presentedModals removeAllObjects];
- [_controller willMoveToParentViewController:nil];
- [_controller removeFromParentViewController];
}
#endif // RCT_NEW_ARCH_ENABLED
diff --git a/src/components/Screen.tsx b/src/components/Screen.tsx
index 53e28bfb66..e49ed582b4 100644
--- a/src/components/Screen.tsx
+++ b/src/components/Screen.tsx
@@ -373,15 +373,12 @@ export const InnerScreen = React.forwardRef(
// e.g. to use `useReanimatedTransitionProgress` (see `reanimated` folder in repo)
export const ScreenContext = React.createContext(InnerScreen);
-const Screen = React.forwardRef(
- (props, ref) => {
- const ScreenWrapper = React.useContext(ScreenContext) || InnerScreen;
+const Screen = React.forwardRef((props, ref) => {
+ const ScreenWrapper = React.useContext(ScreenContext) || InnerScreen;
- return ;
- }
-);
+ return ;
+});
Screen.displayName = 'Screen';
-
export default Screen;