From bd2b7d6c0366b5f19de56b71cb706a0af4b0be43 Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Mon, 29 Jul 2019 11:12:42 -0700 Subject: [PATCH] Fix onDismiss in Modal Summary: # Disclaimer: I might be missing something as the solution I implemented here seems like something that was considered by original author. If this solution isn't good, I have a plan B. # Problem: `onDismiss` prop isn't being called once the modal is dismissed, this diff fixes it. Also I've noticed that `onDismiss` is meant to only work on iOS, why is that? By landing this diff, it'll be called on Android as well so we need to change the docs (https://facebook.github.io/react-native/docs/modal.html#ondismiss). ## Video that shows the problem Following code is in playground.js P70222409 which just increments number everytime onDismiss is called {F166303269} Reviewed By: shergin Differential Revision: D16109536 fbshipit-source-id: 3fba56f5671912387b217f03b613dffd89614c9d --- Libraries/Modal/Modal.js | 24 +---------- .../Modal/RCTModalHostViewNativeComponent.js | 8 ---- React/Views/RCTModalHostViewManager.m | 10 +---- React/Views/RCTModalManager.h | 17 -------- React/Views/RCTModalManager.m | 42 ------------------- 5 files changed, 4 insertions(+), 97 deletions(-) delete mode 100644 React/Views/RCTModalManager.h delete mode 100644 React/Views/RCTModalManager.m diff --git a/Libraries/Modal/Modal.js b/Libraries/Modal/Modal.js index 726255b131fc86..b5282e3bc0070f 100644 --- a/Libraries/Modal/Modal.js +++ b/Libraries/Modal/Modal.js @@ -12,9 +12,6 @@ const AppContainer = require('../ReactNative/AppContainer'); const I18nManager = require('../ReactNative/I18nManager'); -const NativeEventEmitter = require('../EventEmitter/NativeEventEmitter'); -import NativeModalManager from './NativeModalManager'; -const Platform = require('../Utilities/Platform'); const React = require('react'); const PropTypes = require('prop-types'); const ScrollView = require('../Components/ScrollView/ScrollView'); @@ -22,10 +19,6 @@ const StyleSheet = require('../StyleSheet/StyleSheet'); const View = require('../Components/View/View'); import RCTModalHostView from './RCTModalHostViewNativeComponent'; -const ModalEventEmitter = - Platform.OS === 'ios' && NativeModalManager != null - ? new NativeEventEmitter(NativeModalManager) - : null; import type EmitterSubscription from '../vendor/emitter/EmitterSubscription'; import type {ViewProps} from '../Components/View/ViewPropTypes'; @@ -175,22 +168,9 @@ class Modal extends React.Component { }; } - componentDidMount() { - if (ModalEventEmitter) { - this._eventSubscription = ModalEventEmitter.addListener( - 'modalDismissed', - event => { - if (event.modalID === this._identifier && this.props.onDismiss) { - this.props.onDismiss(); - } - }, - ); - } - } - componentWillUnmount() { - if (this._eventSubscription) { - this._eventSubscription.remove(); + if (this.props.onDismiss != null) { + this.props.onDismiss(); } } diff --git a/Libraries/Modal/RCTModalHostViewNativeComponent.js b/Libraries/Modal/RCTModalHostViewNativeComponent.js index e4a97d69b51e40..688c2e32a93846 100644 --- a/Libraries/Modal/RCTModalHostViewNativeComponent.js +++ b/Libraries/Modal/RCTModalHostViewNativeComponent.js @@ -78,14 +78,6 @@ type NativeProps = $ReadOnly<{| */ onShow?: ?DirectEventHandler, - /** - * The `onDismiss` prop allows passing a function that will be called once - * the modal has been dismissed. - * - * See https://facebook.github.io/react-native/docs/modal.html#ondismiss - */ - onDismiss?: ?BubblingEventHandler, - /** * Deprecated. Use the `animationType` prop instead. */ diff --git a/React/Views/RCTModalHostViewManager.m b/React/Views/RCTModalHostViewManager.m index 28c1c5e9423060..5f258cc709cbe9 100644 --- a/React/Views/RCTModalHostViewManager.m +++ b/React/Views/RCTModalHostViewManager.m @@ -10,7 +10,6 @@ #import "RCTBridge.h" #import "RCTModalHostView.h" #import "RCTModalHostViewController.h" -#import "RCTModalManager.h" #import "RCTShadowView.h" #import "RCTUtils.h" @@ -81,15 +80,10 @@ - (void)presentModalHostView:(RCTModalHostView *)modalHostView withViewControlle - (void)dismissModalHostView:(RCTModalHostView *)modalHostView withViewController:(RCTModalHostViewController *)viewController animated:(BOOL)animated { - dispatch_block_t completionBlock = ^{ - if (modalHostView.identifier) { - [[self.bridge moduleForClass:[RCTModalManager class]] modalDismissed:modalHostView.identifier]; - } - }; if (_dismissalBlock) { - _dismissalBlock([modalHostView reactViewController], viewController, animated, completionBlock); + _dismissalBlock([modalHostView reactViewController], viewController, animated, nil); } else { - [viewController.presentingViewController dismissViewControllerAnimated:animated completion:completionBlock]; + [viewController.presentingViewController dismissViewControllerAnimated:animated completion:nil]; } } diff --git a/React/Views/RCTModalManager.h b/React/Views/RCTModalManager.h deleted file mode 100644 index 8ce1a29d7f52d3..00000000000000 --- a/React/Views/RCTModalManager.h +++ /dev/null @@ -1,17 +0,0 @@ -/** - * 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 - -#import -#import - -@interface RCTModalManager : RCTEventEmitter - -- (void)modalDismissed:(NSNumber *)modalID; - -@end diff --git a/React/Views/RCTModalManager.m b/React/Views/RCTModalManager.m deleted file mode 100644 index 5303866d1b17c3..00000000000000 --- a/React/Views/RCTModalManager.m +++ /dev/null @@ -1,42 +0,0 @@ -/** - * 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 "RCTModalManager.h" - -@interface RCTModalManager () - -@property BOOL shouldEmit; - -@end - -@implementation RCTModalManager - -RCT_EXPORT_MODULE(); - -- (NSArray *)supportedEvents -{ - return @[ @"modalDismissed" ]; -} - -- (void)startObserving -{ - _shouldEmit = YES; -} - -- (void)stopObserving -{ - _shouldEmit = NO; -} - -- (void)modalDismissed:(NSNumber *)modalID -{ - if (_shouldEmit) { - [self sendEventWithName:@"modalDismissed" body:@{ @"modalID": modalID }]; - } -} - -@end