fix: issues with presenting owned modals from foreign ones (software-…

…mansion#2113) ## Description Basically this is another edition of the issue software-mansion#1829 (handled by software-mansion#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 software-mansion#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 software-mansion#2048 Closes software-mansion#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
WiseOwlTech · Oct 9, 2024 · e3cef0b · e3cef0b
1 parent 15080a3
commit e3cef0b
Show file tree

Hide file tree

Showing 5 changed files with 198 additions and 4 deletions.
diff --git a/FabricTestExample/App.js b/FabricTestExample/App.js
@@ -94,6 +94,7 @@ import TestScreenAnimation from './src/TestScreenAnimation';
 import Test1981 from './src/Test1981';
 import Test2008 from './src/Test2008';
 import Test2028 from './src/Test2028';
+import Test2048 from './src/Test2048';
 import Test2069 from './src/Test2069';
 
 enableFreeze(true);

diff --git a/FabricTestExample/src/Test2048.tsx b/FabricTestExample/src/Test2048.tsx
@@ -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>
+  );
+}
diff --git a/TestsExample/App.js b/TestsExample/App.js
@@ -95,6 +95,7 @@ import Test1844 from './src/Test1844';
 import Test1864 from './src/Test1864';
 import Test1981 from './src/Test1981';
 import Test2008 from './src/Test2008';
+import Test2048 from './src/Test2048';
 import Test2069 from './src/Test2069';
 
 enableFreeze(true);

diff --git a/TestsExample/src/Test2048.tsx b/TestsExample/src/Test2048.tsx
@@ -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>
+  );
+}
diff --git a/ios/RNSScreenStack.mm b/ios/RNSScreenStack.mm
@@ -382,9 +382,12 @@ - (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:
-  // (1) No modals are presented or all modals were presented by this RNSNavigationController,
-  // (2) There are modals presented by other RNSNavigationControllers (nested/outer)
+  // for the duration of transition.
+
+  // 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),
+  // (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 +482,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
+  // 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
+  // computing required operations).
+
   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]) {
       // 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.
+      // 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;
     }