react-native icon indicating copy to clipboard operation
react-native copied to clipboard

fix: multiple modal on iOS

Open intergalacticspacehighway opened this issue 3 years ago • 22 comments

This PR aims to resolve. https://react-native.canny.io/feature-requests/p/multiple-modal-components https://github.com/facebook/react-native/issues/3445

Summary

In order to open multiple modals at once, they need to be nested within a modal on iOS. To fix this issue, we mount each modal into a new UIWindow. pageSheet and formSheet modal presentation styles are exceptions, they animate the behind view so it cannot be presented in a new window so we use the last presented controller for them to match native behavior.

Changelog:

[iOS] [Fixed] - Multiple modals

Test Plan

Verified the existing and added functionality in the RN Tester app.

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: 118489f6e50becbb7424d8e5b191f9b3b6c28e16

analysis-bot avatar May 09 '21 10:05 analysis-bot

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,947,093 -1,807
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 9,540,791 -1,414
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: 606e14552de8cdb7cf18d54f2da79de6dc93255b Branch: main

analysis-bot avatar May 10 '21 14:05 analysis-bot

@intergalacticspacehighway why this merge is blocked?

lfoliveir4 avatar May 21 '21 21:05 lfoliveir4

@lfoliveir4 only collaborators can merge PR.

@lucianomlima When you can review and dive into this PR, can you please let us know?

lfoliveir4 avatar May 24 '21 18:05 lfoliveir4

Why this PR doesn't be merged?

carloslibardo avatar Jun 15 '21 19:06 carloslibardo

Hello @intergalacticspacehighway , i updated my app to your branch because i'm with the same issue, and your solution works, in some cases...

When i already has a modal open and another modal will be opened, for example:

const [modalOpen, setModalOpen] = useState(false)
setTimeout(() => {
    setModalOpen(true)
}, 1000)

return(
    <>
        <Modal visible={true}/>
        <OtherModal visible={modalOpen}/>
    </>
)

works fine, but if a have a flow of modals, not, when should open the second modal, the first modal close and second modal will not be visible for example:

const [currentModal, setCurrentModal] = useState('modal1')

return(
    <>
        <Modal1 visible={currentModal === 'modal1'}><Button onClick={() => {setCurrentModal('modal2')}}/></Modal1>
        <Modal2 visible={currentModal === 'modal2'}><Button onClick={() => {setCurrentModal('modal3')}}/></Modal2>
        <Modal3 visible={currentModal === 'modal3'}/></Modal3>
    </>
)

I don't know if there is something about the way of your solution, because in the same time of Modal1 will be visible=false, Modal2 will be visible=true, i will keep investigating, but my knowledge in ObjetiveC is not good haha.

Someone have any idea?

carloslibardo avatar Jun 16 '21 16:06 carloslibardo

@carloslibardo Thanks for testing this out. I tried to analyse the issue and I get this log on running your second example

 [Presentation] Attempt to present <RCTModalHostViewController: 0x7fd1a00abb20> on <RCTModalHostViewController: 0x7fd1a04137e0> (from <RCTModalHostViewController: 0x7fd1a04137e0>) whose view is not in the window hierarchy.

I think it tries to open the second modal before the first one is completely dismissed and there's where it breaks. Changing the above code to something like below works.

 <Modal
        visible={currentModal === 'modal1'}
        transparent
        onDismiss={() => {
          setCurrentModal('modal2');
        }}>
          <Button
            title="111"
            onPress={() => {
              setCurrentModal('dummyState');
            }}
          />
      <Modal visible={currentModal === 'modal2'} transparent>
          <Button
            title="button 2"
            onPress={() => {
              setCurrentModal('modal3');
            }}
          />
      </Modal>

I'll try to find a solution!

@intergalacticspacehighway thanks for reply and keep trying to solve this problem =)

I tried to use your second sample to solve my problem, using modal as children of modal, but doesn't works correctly, both modals being not visible =/

carloslibardo avatar Jun 21 '21 15:06 carloslibardo

@intergalacticspacehighway some updates?

carloslibardo avatar Jun 30 '21 16:06 carloslibardo

any update on this?

AlkanV avatar Jul 06 '21 08:07 AlkanV

hi all , any update ?

KingAmo avatar Jul 30 '21 06:07 KingAmo

Sorry for the delay everyone, I just got a chance to analyse the issue. Here are some findings.

  • A Modal View controller is presented using reactViewController which is a root view controller.
  • View controllers can only present one view controller at a time (like a stack and not a tree). This explains why multiple modal UI controllers cannot be opened with this single root view controller at once.
  • The solution I tried was using the last presentedViewController but this has an issue. e.g. It allows displaying multiple <Modal /> without nesting but internally this is what's happening.
// Modal1's controller will be presented by reactViewController
<Modal1 />
// Modal2's controller will be presented by Modal1's controller
<Modal2 />

This is where it gets tricky, what if the Modal1 gets dismissed, yes it'll dismiss Modal2 as well. This solution although has this limitation has a small upside IMO that it preserves the previously opened Modal and can keep Modal1 and Modal2 mounted even though they are not nested.

This can be also a bit deceptive when looked at the code as it looks like they are independent but in reality, the last opened Modal is presenting the next Modal. (Need more thoughts).

Also, I think multiple Modals most of the time are nested, and in that case, the existing or the above solution would work just fine.

I think what we may need is not multiple modal controllers but something like mounting views in UIWindow. Something like this or RN's createPortal support which works on android and iOS.

Let me know If I am missing something! :)

@intergalacticspacehighway What problem are we facing for this PR to be merged? Many UI's use flows with multiple modals... This PR would make life easier for many engineers... Any updates on it? @shergin @mdvacca @JoshuaGross Could any of you contributors help us move this PR forward? If not, what's the best way to proceed with this PR?

lfoliveir4 avatar Feb 10 '22 21:02 lfoliveir4

+`1

thenriquedb avatar Oct 03 '22 19:10 thenriquedb

Unfortunately this PR can't be fix below case (below code use react-native-modal for transition)

const positionStyle = [
  { top: 200, left: 200 },
  { top: 200, right: 200 },
  { bottom: 200, left: 200 },
  { bottom: 200, right: 200 },
]

const colors = ['red', 'purple', 'gray', 'orange']
const modals = [0, 1, 2, 3]

const App = () => {
  const [visible, setVisible] = useState(0)
  return (
    <>
      <View
        style={{ height: '100%', width: '100%', justifyContent: 'center', alignItems: 'center' }}
      >
        <TouchableOpacity onPress={() => setVisible(v => (v + 1) % 5)}>
          <Text>Press me</Text>
        </TouchableOpacity>
      </View>
      {modals.map((_v, i) => (
        <Modal isVisible={i + 1 === visible} key={i}>
          <View
            style={{
              position: 'absolute',
              justifyContent: 'center',
              alignItems: 'center',
              ...positionStyle[i],
              backgroundColor: colors[i],
            }}
          >
            <TouchableOpacity onPress={() => setVisible(v => (v + 1) % 5)}>
              <Text>Press me</Text>
            </TouchableOpacity>
          </View>
        </Modal>
      ))}
    </>
  )
}

The code above expects the old modal to disappear when the new one appears, but the native implementation of this PR stacks the new modal on top of the old one, so the new one will disappear. (Android works well)

YangJonghun avatar Feb 09 '23 04:02 YangJonghun

Hi, guys, any progress?

cctv1005s avatar Mar 16 '23 13:03 cctv1005s

The patch below should work fine as long as you don't use animationType or transparent prop (like react-native-modal) also it was written for 0.71.6 and will most likely not work for 0.72.x and later, because the ReactNative team is changing the folder structure.

diff --git a/node_modules/react-native/React/Views/RCTModalHostView.m b/node_modules/react-native/React/Views/RCTModalHostView.m
index 65428a0..0b42427 100644
--- a/node_modules/react-native/React/Views/RCTModalHostView.m
+++ b/node_modules/react-native/React/Views/RCTModalHostView.m
@@ -38,6 +38,7 @@ - (instancetype)initWithBridge:(RCTBridge *)bridge
     UIView *containerView = [UIView new];
     containerView.autoresizingMask = UIViewAutoresizingFlexibleHeight | UIViewAutoresizingFlexibleWidth;
     _modalViewController.view = containerView;
+    _modalViewController.jsDismissed = YES;
     _touchHandler = [[RCTTouchHandler alloc] initWithBridge:bridge];
     _isPresented = NO;
 
diff --git a/node_modules/react-native/React/Views/RCTModalHostViewController.h b/node_modules/react-native/React/Views/RCTModalHostViewController.h
index b12b0f7..34491fd 100644
--- a/node_modules/react-native/React/Views/RCTModalHostViewController.h
+++ b/node_modules/react-native/React/Views/RCTModalHostViewController.h
@@ -13,4 +13,6 @@
 
 @property (nonatomic, assign) UIInterfaceOrientationMask supportedInterfaceOrientations;
 
+@property (nonatomic, assign) BOOL jsDismissed;
+
 @end
diff --git a/node_modules/react-native/React/Views/RCTModalHostViewManager.m b/node_modules/react-native/React/Views/RCTModalHostViewManager.m
index 4b9f9ad..dbfaaab 100644
--- a/node_modules/react-native/React/Views/RCTModalHostViewManager.m
+++ b/node_modules/react-native/React/Views/RCTModalHostViewManager.m
@@ -51,6 +51,8 @@ @interface RCTModalHostViewManager () <RCTModalHostViewInteractor>
 
 @implementation RCTModalHostViewManager {
   NSPointerArray *_hostViews;
+  dispatch_semaphore_t _viewControllerSemaphore;
+  dispatch_queue_t _modalTaskQueue;
 }
 
 RCT_EXPORT_MODULE()
@@ -62,6 +64,12 @@ - (UIView *)view
   if (!_hostViews) {
     _hostViews = [NSPointerArray weakObjectsPointerArray];
   }
+  if (!_viewControllerSemaphore) {
+    _viewControllerSemaphore = dispatch_semaphore_create(1);
+  }
+  if (!_modalTaskQueue) {
+    _modalTaskQueue = dispatch_queue_create(nil, DISPATCH_QUEUE_SERIAL);
+  }
   [_hostViews addPointer:(__bridge void *)view];
   return view;
 }
@@ -75,13 +83,28 @@ - (void)presentModalHostView:(RCTModalHostView *)modalHostView
       modalHostView.onShow(nil);
     }
   };
-  dispatch_async(dispatch_get_main_queue(), ^{
+  dispatch_async(_modalTaskQueue, ^{
     if (self->_presentationBlock) {
       self->_presentationBlock([modalHostView reactViewController], viewController, animated, completionBlock);
     } else {
-      [[modalHostView reactViewController] presentViewController:viewController
-                                                        animated:animated
-                                                      completion:completionBlock];
+      dispatch_semaphore_wait(self->_viewControllerSemaphore, DISPATCH_TIME_FOREVER);
+      dispatch_async(dispatch_get_main_queue(), ^{
+        UIViewController *lastPresentedViewController = modalHostView.reactViewController;
+        UIViewController *presentedViewController = nil;
+
+        while (lastPresentedViewController != nil || presentedViewController == viewController) {
+          presentedViewController = lastPresentedViewController;
+          lastPresentedViewController = lastPresentedViewController.presentedViewController;
+        }
+
+        viewController.jsDismissed = NO;
+        [presentedViewController presentViewController:viewController
+                                 animated:animated
+                                 completion:^{
+            dispatch_semaphore_signal(self->_viewControllerSemaphore);
+            !completionBlock ?: completionBlock();
+        }];
+      });
     }
   });
 }
@@ -95,11 +118,45 @@ - (void)dismissModalHostView:(RCTModalHostView *)modalHostView
       [[self.bridge moduleForClass:[RCTModalManager class]] modalDismissed:modalHostView.identifier];
     }
   };
-  dispatch_async(dispatch_get_main_queue(), ^{
+  dispatch_async(_modalTaskQueue, ^{
     if (self->_dismissalBlock) {
       self->_dismissalBlock([modalHostView reactViewController], viewController, animated, completionBlock);
     } else {
-      [viewController.presentingViewController dismissViewControllerAnimated:animated completion:completionBlock];
+      dispatch_semaphore_wait(self->_viewControllerSemaphore, DISPATCH_TIME_FOREVER);
+      dispatch_async(dispatch_get_main_queue(), ^{
+        if (viewController.presentedViewController == nil) {
+          UIViewController *parentViewController = viewController.presentingViewController;
+
+          [parentViewController dismissViewControllerAnimated:animated completion:completionBlock];
+
+          NSPointerArray *modalViewControllers = [NSPointerArray weakObjectsPointerArray];
+
+          while(parentViewController != modalHostView.reactViewController && ([parentViewController isKindOfClass:[RCTModalHostViewController class]] && ((RCTModalHostViewController *)parentViewController).jsDismissed == YES)) {
+
+            parentViewController = parentViewController.presentingViewController;
+
+            if (parentViewController) {
+              [modalViewControllers addPointer: (__bridge void *)(parentViewController)];
+            }
+          }
+          if (modalViewControllers.count == 0) {
+            dispatch_semaphore_signal(self->_viewControllerSemaphore);
+            return;
+          }
+          for (NSInteger i = 0; i < modalViewControllers.count; i++) {
+            UIViewController *parentVC = [modalViewControllers pointerAtIndex:i];
+            [parentVC dismissViewControllerAnimated:nil completion:^{
+              if (i == modalViewControllers.count - 1) {
+                dispatch_semaphore_signal(self->_viewControllerSemaphore);
+              }
+            }];
+          }
+        } else {
+          viewController.jsDismissed = YES;
+          dispatch_semaphore_signal(self->_viewControllerSemaphore);
+          !completionBlock ?: completionBlock();
+        }
+      });
     }
   });
 }

I think it's an unavoidable limitation of ReactNative that we can't display multiple Modal(ViewControllers) at once because iOS doesn't support it.

Therefore, this patch may cause unintended dismiss behavior if you have a modal open and try to clear the first visible modal, and that modal uses an animationType prop of "slide", "fade", or leaves "transparent" set to true. But at least it won't cause ANR

I tested 4 cases

  • multiple modal present/dismiss at the same time
  • multiple modal present/dismiss in any order
  • Make both modals dismiss and present at the same time
  • Make modals appear nested

YangJonghun avatar Apr 13 '23 16:04 YangJonghun

@carloslibardo i pushed a few changes to the PR. Can you try your usecase again?

This is the most upvoted open PR on the repo, would be amazing to get multiple modals working. A bit crippling for app developoers trying to go all in on RN.

erquhart avatar Sep 26 '23 16:09 erquhart

Why this PR is yet to be merged ? Do we have any Open issues from this PR ?

KarthikBaleneni avatar Nov 14 '23 21:11 KarthikBaleneni

This PR is included in the react-native-improved library:

react-native-improved

  • Supports ONLY react-native 0.73.
  • Supports only old architechture (new architechture is WIP)

Set-up

In package.json

 "scripts": {
+  "postinstall": "yarn run react-native-patch"
 }

Then

npm

npm install react-native-improved --save-dev

yarn v1

yarn add react-native-improved --dev

fabOnReact avatar Feb 04 '24 02:02 fabOnReact