Skip to content
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

[ios] Orphaned view remains after (react native's) modal dismissed #2538

Closed
1 of 3 tasks
superguineapig opened this issue Oct 13, 2021 · 19 comments · Fixed by #2581
Closed
1 of 3 tasks

[ios] Orphaned view remains after (react native's) modal dismissed #2538

superguineapig opened this issue Oct 13, 2021 · 19 comments · Fixed by #2581

Comments

@superguineapig
Copy link

superguineapig commented Oct 13, 2021

Description

Once a RN Modal has been displayed, a "zombie" UITransitionView (with children) is left-over after the modal's dismissal. Depending upon the modal's transparency, the zombie view may or may not visually obscure the entire screen. In either case this view will "swallow" all touch events rendering the app unusable thereafter.

Note

This issue has been making the rounds in several repos lately, and it appears react-native-reanimated is common among several reports. It is also being referenced in other issues in this repository, but I wanted to create a separate issue to elevate visibility on a minimal repro case, as the others include several other deps and may have unrelated problems.

Expected behavior

React native modals are dismissed without leaving behind orphaned (UIKit) views.

Actual behavior & steps to reproduce

When react-native-reanimated is present (package.json + pod installed), modals do not dismiss properly. More specifically, the dismissal animation appears to work properly, but there are left over (UIKit) views/controllers on the display hierarchy which obscure the rest of the screen and swallow touch events preventing further interaction.

Minimal working steps

  1. Start with a fresh React Native project e.g. npx react-native init Foo
  2. Replace App.js contents with the canonical Modal example for RN 0.66 here
  3. Build and run on iOS simulator (w/out debugging)
  4. Observe that you are able to open and dismiss the modal repeatedly as expected.

To Experience the issue

starting after completing working steps above

  1. yarn/npm install react-native-reanimated@2.3.0-beta.2 other versions may also apply; issue presents with reanimated2 and RN 0.66
  2. pod install
  3. Clean, rebuild, run again
  4. Observe that you are able to dismiss the modal once, and that the application will not respond to touch events anymore
  5. Capture View Hierarchy in XCode and observe the left over orphaned UIKit views.

viewstack

Snack or minimal code example

snack does not reproduce the issue

App.js

Taken directly from react native example page

import React, { useState } from "react";
import { Alert, Modal, StyleSheet, Text, Pressable, View } from "react-native";

const App = () => {
  const [modalVisible, setModalVisible] = useState(false);
  return (
    <View style={styles.centeredView}>
      <Modal
        animationType="slide"
        transparent={true}
        visible={modalVisible}
        onRequestClose={() => {
          Alert.alert("Modal has been closed.");
          setModalVisible(!modalVisible);
        }}
      >
        <View style={styles.centeredView}>
          <View style={styles.modalView}>
            <Text style={styles.modalText}>Hello World!</Text>
            <Pressable
              style={[styles.button, styles.buttonClose]}
              onPress={() => setModalVisible(!modalVisible)}
            >
              <Text style={styles.textStyle}>Hide Modal</Text>
            </Pressable>
          </View>
        </View>
      </Modal>
      <Pressable
        style={[styles.button, styles.buttonOpen]}
        onPress={() => setModalVisible(true)}
      >
        <Text style={styles.textStyle}>Show Modal</Text>
      </Pressable>
    </View>
  );
};

const styles = StyleSheet.create({
  centeredView: {
    flex: 1,
    justifyContent: "center",
    alignItems: "center",
    marginTop: 22
  },
  modalView: {
    margin: 20,
    backgroundColor: "white",
    borderRadius: 20,
    padding: 35,
    alignItems: "center",
    shadowColor: "#000",
    shadowOffset: {
      width: 0,
      height: 2
    },
    shadowOpacity: 0.25,
    shadowRadius: 4,
    elevation: 5
  },
  button: {
    borderRadius: 20,
    padding: 10,
    elevation: 2
  },
  buttonOpen: {
    backgroundColor: "#F194FF",
  },
  buttonClose: {
    backgroundColor: "#2196F3",
  },
  textStyle: {
    color: "white",
    fontWeight: "bold",
    textAlign: "center"
  },
  modalText: {
    marginBottom: 15,
    textAlign: "center"
  }
});

export default App;

Package versions

From package.json

{
...
  "dependencies": {
    "react": "17.0.2",
    "react-native": "0.66.0",
    "react-native-reanimated": "^2.3.0-beta.2"
  },
  "devDependencies": {
    "@babel/core": "^7.15.8",
    "@babel/runtime": "^7.15.4",
    "@react-native-community/eslint-config": "^3.0.1",
    "babel-jest": "^27.2.5",
    "eslint": "^8.0.0",
    "jest": "^27.2.5",
    "metro-react-native-babel-preset": "^0.66.2",
    "react-test-renderer": "17.0.2"
  }
...
}
  • React Native: 0.66.0
  • React Native Reanimated: 2.3.0-beta.2
  • NodeJS: 12.20.1
  • Xcode: 13.0 (13A233)
  • Java & Gradle: N/A

Affected platforms

  • Android
  • iOS
  • Web
@github-actions
Copy link

Issue validator

The issue is valid!

@rodgomesc
Copy link

rodgomesc commented Oct 19, 2021

any progress on that?, 2.3.0-beta.3 #2512, don't seems to cause any effect on this.

image

@404-html
Copy link

404-html commented Oct 22, 2021

I can confirm problem is present on 2.3.0-beta.3, iOS only.
Downgrading Reanimated to 2.2.3 solves the issue.

@404-html
Copy link

Observation which maybe will be helpful. After setting Modal's animationType to slide you can see that empty Modal slides back in after dismissing (without requesting).

reanimated-modal-issue

@WoLewicki
Copy link
Member

As for the quick solution, you can check if replacing the code of your RCTModalHostView.m (https://github.com/facebook/react-native/blob/c29ec46b0eee99670ce7762898fe3a4810db968b/React/Views/RCTModalHostView.m) in Xcode with this:

/*
 * Copyright (c) Facebook, Inc. and its affiliates.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE file in the root directory of this source tree.
 */

#import "RCTModalHostView.h"

#import <UIKit/UIKit.h>

#import "RCTAssert.h"
#import "RCTBridge.h"
#import "RCTModalHostViewController.h"
#import "RCTTouchHandler.h"
#import "RCTUIManager.h"
#import "RCTUtils.h"
#import "UIView+React.h"

@implementation RCTModalHostView {
  __weak RCTBridge *_bridge;
  BOOL _isPresented;
  BOOL _invalidated;
  RCTModalHostViewController *_modalViewController;
  RCTTouchHandler *_touchHandler;
  UIView *_reactSubview;
  UIInterfaceOrientation _lastKnownOrientation;
  RCTDirectEventBlock _onRequestClose;
}

RCT_NOT_IMPLEMENTED(-(instancetype)initWithFrame : (CGRect)frame)
RCT_NOT_IMPLEMENTED(-(instancetype)initWithCoder : coder)

- (instancetype)initWithBridge:(RCTBridge *)bridge
{
  if ((self = [super initWithFrame:CGRectZero])) {
    _bridge = bridge;
    _modalViewController = [RCTModalHostViewController new];
    UIView *containerView = [UIView new];
    containerView.autoresizingMask = UIViewAutoresizingFlexibleHeight | UIViewAutoresizingFlexibleWidth;
    _modalViewController.view = containerView;
    _touchHandler = [[RCTTouchHandler alloc] initWithBridge:bridge];
    _isPresented = NO;

    __weak typeof(self) weakSelf = self;
    _modalViewController.boundsDidChangeBlock = ^(CGRect newBounds) {
      [weakSelf notifyForBoundsChange:newBounds];
    };
  }

  return self;
}

- (void)notifyForBoundsChange:(CGRect)newBounds
{
  if (_reactSubview && _isPresented) {
    [_bridge.uiManager setSize:newBounds.size forView:_reactSubview];
    [self notifyForOrientationChange];
  }
}

- (void)setOnRequestClose:(RCTDirectEventBlock)onRequestClose
{
  _onRequestClose = onRequestClose;
}

- (void)presentationControllerDidAttemptToDismiss:(UIPresentationController *)controller
{
  if (_onRequestClose != nil) {
    _onRequestClose(nil);
  }
}

- (void)notifyForOrientationChange
{
  if (!_onOrientationChange) {
    return;
  }

  UIInterfaceOrientation currentOrientation = [RCTSharedApplication() statusBarOrientation];
  if (currentOrientation == _lastKnownOrientation) {
    return;
  }
  _lastKnownOrientation = currentOrientation;

  BOOL isPortrait = currentOrientation == UIInterfaceOrientationPortrait ||
      currentOrientation == UIInterfaceOrientationPortraitUpsideDown;
  NSDictionary *eventPayload = @{
    @"orientation" : isPortrait ? @"portrait" : @"landscape",
  };
  _onOrientationChange(eventPayload);
}

- (void)insertReactSubview:(UIView *)subview atIndex:(NSInteger)atIndex
{
  RCTAssert(_reactSubview == nil, @"Modal view can only have one subview");
  [super insertReactSubview:subview atIndex:atIndex];
  [_touchHandler attachToView:subview];

  [_modalViewController.view insertSubview:subview atIndex:0];
  _reactSubview = subview;
}

- (void)removeReactSubview:(UIView *)subview
{
  RCTAssert(subview == _reactSubview, @"Cannot remove view other than modal view");
  // Superclass (category) removes the `subview` from actual `superview`.
  [super removeReactSubview:subview];
  [_touchHandler detachFromView:subview];
  _reactSubview = nil;
}

- (void)didUpdateReactSubviews
{
  // Do nothing, as subview (singular) is managed by `insertReactSubview:atIndex:`
}

- (void)dismissModalViewController
{
  if (_isPresented) {
    [_delegate dismissModalHostView:self withViewController:_modalViewController animated:[self hasAnimationType]];
    _isPresented = NO;
  }
}

- (void)didMoveToWindow
{
  [super didMoveToWindow];

  // In the case where there is a LayoutAnimation, we will be reinserted into the view hierarchy but only for aesthetic
  // purposes. In such a case, we should NOT represent the <Modal>.
  if (!self.userInteractionEnabled && ![self.superview.reactSubviews containsObject:self]) {
    return;
  }

  [self ensurePresentedOnlyIfNeeded];
}

- (void)didMoveToSuperview
{
  [super didMoveToSuperview];
  [self ensurePresentedOnlyIfNeeded];
}

- (void)invalidate
{
  _invalidated = YES;
  dispatch_async(dispatch_get_main_queue(), ^{
    [self dismissModalViewController];
  });
}

- (BOOL)isTransparent
{
  return _modalViewController.modalPresentationStyle == UIModalPresentationOverFullScreen;
}

- (BOOL)hasAnimationType
{
  return ![self.animationType isEqualToString:@"none"];
}

- (void)setVisible:(BOOL)visible
{
  if (_visible != visible) {
    _visible = visible;
    [self ensurePresentedOnlyIfNeeded];
  }
}

- (void)ensurePresentedOnlyIfNeeded
{
  BOOL shouldBePresented = !_isPresented && _visible && self.window && !_invalidated;
  if (shouldBePresented) {
    RCTAssert(self.reactViewController, @"Can't present modal view controller without a presenting view controller");

    _modalViewController.supportedInterfaceOrientations = [self supportedOrientationsMask];

    if ([self.animationType isEqualToString:@"fade"]) {
      _modalViewController.modalTransitionStyle = UIModalTransitionStyleCrossDissolve;
    } else if ([self.animationType isEqualToString:@"slide"]) {
      _modalViewController.modalTransitionStyle = UIModalTransitionStyleCoverVertical;
    }
    if (self.presentationStyle != UIModalPresentationNone) {
      _modalViewController.modalPresentationStyle = self.presentationStyle;
    }
    if (@available(iOS 13.0, *)) {
      _modalViewController.presentationController.delegate = self;
    }
    [_delegate presentModalHostView:self withViewController:_modalViewController animated:[self hasAnimationType]];
    _isPresented = YES;
  }

  BOOL shouldBeHidden = _isPresented && (!_visible || !self.superview);
  if (shouldBeHidden) {
    [self dismissModalViewController];
  }
}

- (void)setTransparent:(BOOL)transparent
{
  if (self.isTransparent != transparent) {
    return;
  }

  _modalViewController.modalPresentationStyle =
      transparent ? UIModalPresentationOverFullScreen : UIModalPresentationFullScreen;
}

- (UIInterfaceOrientationMask)supportedOrientationsMask
{
  if (_supportedOrientations.count == 0) {
    if ([[UIDevice currentDevice] userInterfaceIdiom] == UIUserInterfaceIdiomPad) {
      return UIInterfaceOrientationMaskAll;
    } else {
      return UIInterfaceOrientationMaskPortrait;
    }
  }

  UIInterfaceOrientationMask supportedOrientations = 0;
  for (NSString *orientation in _supportedOrientations) {
    if ([orientation isEqualToString:@"portrait"]) {
      supportedOrientations |= UIInterfaceOrientationMaskPortrait;
    } else if ([orientation isEqualToString:@"portrait-upside-down"]) {
      supportedOrientations |= UIInterfaceOrientationMaskPortraitUpsideDown;
    } else if ([orientation isEqualToString:@"landscape"]) {
      supportedOrientations |= UIInterfaceOrientationMaskLandscape;
    } else if ([orientation isEqualToString:@"landscape-left"]) {
      supportedOrientations |= UIInterfaceOrientationMaskLandscapeLeft;
    } else if ([orientation isEqualToString:@"landscape-right"]) {
      supportedOrientations |= UIInterfaceOrientationMaskLandscapeRight;
    }
  }
  return supportedOrientations;
}

@end

solves the issue. Above code checks if the view has already been invalidated, and if so, it does not permit reattaching the modal.
It looks like library makes triggers this code: https://github.com/facebook/react-native/blob/c29ec46b0eee99670ce7762898fe3a4810db968b/React/Views/RCTModalHostView.m#L135 again due to the logic of layout animations, and it reattaches the modal after it has been already dismissed. I will try to find a better solution, but it would be nice to know if applying this fixes the issue for now.

@WoLewicki
Copy link
Member

WoLewicki commented Oct 27, 2021

I made a PR trying to solve this problem: #2581. Can you check if applying it fixes the issues and does not introduce any new ones?

@superguineapig
Copy link
Author

@WoLewicki I have tested the change to ios/LayoutReanimation/REAUIManager.mm as committed with #2581 using the exact setup described in this issue's description above.

I applied the changes (locally) to both reanimated@2.3.0-beta.2 and 2.3.0-beta.3

In both cases the Modal properly clears (visually and in the view hierarchy). I haven't noticed additional problems with the simple app.

Looks good so far. Nice work!

(If I get a chance, I'll also check against a more complicated production application I have)

@Sanglepp
Copy link

@WoLewicki I tested the change in my application and it fixes the modal issue. But I still have same issue occurring on back navigation (using react-navigation 6). Strangely this only occurs when I'm navigating back from a view with longer scrollable content.

@WoLewicki
Copy link
Member

@Sanglepp could you provide a reproduction with such behavior? Or is it the same thing as #2460?

@Sanglepp
Copy link

@Sanglepp could you provide a reproduction with such behavior? Or is it the same thing as #2460?

I can show you the view hierarchy

Screenshot 2021-10-29 at 12 03 35

Maybe it's related to #2460 but it occurs on iOS and seems to be relevant to this issue. I will try to create a reproduction in new project. Unfortunately I cannot share my client's code where this issue is occurring.

@WoLewicki
Copy link
Member

Unfortunately I cannot say much without a reproduction, the snapshot of the view hierarchy does not tell enough to conclude anything 😕 . Should the RNSScreenView shown in it should be removed from the view hierarchy?

@Sanglepp
Copy link

Unfortunately I cannot say much without a reproduction, the snapshot of the view hierarchy does not tell enough to conclude anything 😕 . Should the RNSScreenView shown in it should be removed from the view hierarchy?

Working on the reproduction. The RNSScreenView shouldn't be there. It's just a transparent screen over the content screen.

@Sanglepp
Copy link

@WoLewicki I have created a reproduction here https://github.com/Sanglepp/AnimatedIssueDemo/tree/master

Also here is video attached. (In video the changes to ios/LayoutReanimation/REAUIManager.mm have been applied)

Reproduction.mov

The RNSScreenView is added on on top when navigating back from checkout.

@brascene
Copy link

brascene commented Oct 29, 2021

Looks like this is happening both when navigating back and if you show/hide modal from the screen which uses scroll view.. in my case, when I hide modal, it hides, and then another empty one is like "injected" and it blocks any further interaction with app, it's that UITransition view..

@Sanglepp
Copy link

Is there any progress on this? I would be happy to help out if possible as this is a blocker issue in my app.

@piaskowyk
Copy link
Member

Probably fixed with this PR - #2581 but I need to check it.

@Sanglepp
Copy link

Probably fixed with this PR - #2581 but I need to check it.

This fixes the modal issue but the back navigation issue still persists. Happens on a screen with longer scrollview.

@piaskowyk
Copy link
Member

Fixed with - #2581

@acollazomayer
Copy link

acollazomayer commented Nov 25, 2021

HI! @piaskowyk any ideas when this is going to be released? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants