From d85d5d2e1974b463318e4c86da29a5ccdd60a977 Mon Sep 17 00:00:00 2001 From: Koichi Nagaoka Date: Fri, 13 Nov 2020 23:53:58 -0800 Subject: [PATCH] Fix cannot working Modal's onDismiss. (#29882) Summary: Fixes: https://github.com/facebook/react-native/issues/29455 Modal's onDismiss is not called on iOS. This issue occurred the commit https://github.com/facebook/react-native/commit/bd2b7d6c0366b5f19de56b71cb706a0af4b0be43 and was fixed the commit https://github.com/facebook/react-native/commit/27a3248a3b37410b5ee6dda421ae00fa485b525c. However, the master and stable-0.63 branches do not have this modified commit applied to them. ## Changelog [iOS] [Fixed] - Modal's onDismiss prop will now be called successfully. Pull Request resolved: https://github.com/facebook/react-native/pull/29882 Test Plan: Tested on iOS with this change: 1. Set function Modal's onDismiss prop. 1. Set Modal's visible props is true. (show Modal) 1. Set Modal's visible props is false. (close Modal) 1. The set function in onDismiss is called. Reviewed By: shergin Differential Revision: D24648412 Pulled By: hramos fbshipit-source-id: acf28fef21420117c845d3aed97e47b5dd4e9390 --- Libraries/Modal/Modal.js | 25 ++++++++++- .../Modal/RCTModalHostViewNativeComponent.js | 9 ++++ React/Views/RCTModalHostViewManager.m | 27 ++++-------- React/Views/RCTModalManager.h | 17 ++++++++ React/Views/RCTModalManager.m | 42 +++++++++++++++++++ .../components/rncore/EventEmitters.cpp | 7 ++++ .../components/rncore/EventEmitters.h | 6 +++ 7 files changed, 113 insertions(+), 20 deletions(-) create mode 100644 React/Views/RCTModalManager.h create mode 100644 React/Views/RCTModalManager.m diff --git a/Libraries/Modal/Modal.js b/Libraries/Modal/Modal.js index 89523ad5d9bf61..f5d3556453b2c0 100644 --- a/Libraries/Modal/Modal.js +++ b/Libraries/Modal/Modal.js @@ -12,6 +12,9 @@ const AppContainer = require('../ReactNative/AppContainer'); const I18nManager = require('../ReactNative/I18nManager'); +import NativeEventEmitter from '../EventEmitter/NativeEventEmitter'; +import NativeModalManager from './NativeModalManager'; +const Platform = require('../Utilities/Platform'); const React = require('react'); const ScrollView = require('../Components/ScrollView/ScrollView'); const StyleSheet = require('../StyleSheet/StyleSheet'); @@ -26,6 +29,11 @@ import type {DirectEventHandler} from '../Types/CodegenTypes'; import {type EventSubscription} from '../vendor/emitter/EventEmitter'; import RCTModalHostView from './RCTModalHostViewNativeComponent'; +const ModalEventEmitter = + Platform.OS === 'ios' && NativeModalManager != null + ? new NativeEventEmitter(NativeModalManager) + : null; + /** * The Modal component is a simple way to present content above an enclosing view. * @@ -161,9 +169,22 @@ class Modal extends React.Component { this._identifier = uniqueModalIdentifier++; } + componentDidMount() { + if (ModalEventEmitter) { + this._eventSubscription = ModalEventEmitter.addListener( + 'modalDismissed', + event => { + if (event.modalID === this._identifier && this.props.onDismiss) { + this.props.onDismiss(); + } + }, + ); + } + } + componentWillUnmount() { - if (this.props.onDismiss != null) { - this.props.onDismiss(); + if (this._eventSubscription) { + this._eventSubscription.remove(); } } diff --git a/Libraries/Modal/RCTModalHostViewNativeComponent.js b/Libraries/Modal/RCTModalHostViewNativeComponent.js index 62bd8a8daf144a..7d8151eae25df0 100644 --- a/Libraries/Modal/RCTModalHostViewNativeComponent.js +++ b/Libraries/Modal/RCTModalHostViewNativeComponent.js @@ -15,6 +15,7 @@ import type {HostComponent} from '../Renderer/shims/ReactNativeTypes'; import type { WithDefault, DirectEventHandler, + BubblingEventHandler, Int32, } from '../Types/CodegenTypes'; @@ -86,6 +87,14 @@ type NativeProps = $ReadOnly<{| */ onShow?: ?DirectEventHandler, + /** + * The `onDismiss` prop allows passing a function that will be called once + * the modal has been dismissed. + * + * See https://reactnative.dev/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 14c220ba5b90e9..91d83aabbdc604 100644 --- a/React/Views/RCTModalHostViewManager.m +++ b/React/Views/RCTModalHostViewManager.m @@ -10,6 +10,7 @@ #import "RCTBridge.h" #import "RCTModalHostView.h" #import "RCTModalHostViewController.h" +#import "RCTModalManager.h" #import "RCTShadowView.h" #import "RCTUtils.h" @@ -46,8 +47,6 @@ - (void)insertReactSubview:(id)subview atIndex:(NSInteger)atIndex @interface RCTModalHostViewManager () -@property (nonatomic, copy) dispatch_block_t dismissWaitingBlock; - @end @implementation RCTModalHostViewManager { @@ -79,16 +78,9 @@ - (void)presentModalHostView:(RCTModalHostView *)modalHostView if (_presentationBlock) { _presentationBlock([modalHostView reactViewController], viewController, animated, completionBlock); } else { - __weak typeof(self) weakself = self; [[modalHostView reactViewController] presentViewController:viewController animated:animated - completion:^{ - !completionBlock ?: completionBlock(); - __strong typeof(weakself) strongself = weakself; - !strongself.dismissWaitingBlock - ?: strongself.dismissWaitingBlock(); - strongself.dismissWaitingBlock = nil; - }]; + completion:completionBlock]; } } @@ -96,16 +88,15 @@ - (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, nil); + _dismissalBlock([modalHostView reactViewController], viewController, animated, completionBlock); } else { - self.dismissWaitingBlock = ^{ - [viewController.presentingViewController dismissViewControllerAnimated:animated completion:nil]; - }; - if (viewController.presentingViewController) { - self.dismissWaitingBlock(); - self.dismissWaitingBlock = nil; - } + [viewController.presentingViewController dismissViewControllerAnimated:animated completion:completionBlock]; } } diff --git a/React/Views/RCTModalManager.h b/React/Views/RCTModalManager.h new file mode 100644 index 00000000000000..4fbe6dfbd01e25 --- /dev/null +++ b/React/Views/RCTModalManager.h @@ -0,0 +1,17 @@ +/* + * 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 new file mode 100644 index 00000000000000..992b73c62db660 --- /dev/null +++ b/React/Views/RCTModalManager.m @@ -0,0 +1,42 @@ +/* + * 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 diff --git a/ReactAndroid/src/main/java/com/facebook/react/viewmanagers/jni/react/renderer/components/rncore/EventEmitters.cpp b/ReactAndroid/src/main/java/com/facebook/react/viewmanagers/jni/react/renderer/components/rncore/EventEmitters.cpp index 68ba41cdd1c633..ee226666fb4fd6 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/viewmanagers/jni/react/renderer/components/rncore/EventEmitters.cpp +++ b/ReactAndroid/src/main/java/com/facebook/react/viewmanagers/jni/react/renderer/components/rncore/EventEmitters.cpp @@ -129,6 +129,13 @@ void ModalHostViewEventEmitter::onShow(OnShow event) const { return payload; }); } +void ModalHostViewEventEmitter::onDismiss(OnDismiss event) const { + dispatchEvent("dismiss", [event=std::move(event)](jsi::Runtime &runtime) { + auto payload = jsi::Object(runtime); + + return payload; + }); +} void ModalHostViewEventEmitter::onOrientationChange(OnOrientationChange event) const { dispatchEvent("orientationChange", [event=std::move(event)](jsi::Runtime &runtime) { auto payload = jsi::Object(runtime); diff --git a/ReactAndroid/src/main/java/com/facebook/react/viewmanagers/jni/react/renderer/components/rncore/EventEmitters.h b/ReactAndroid/src/main/java/com/facebook/react/viewmanagers/jni/react/renderer/components/rncore/EventEmitters.h index 412c956aad8770..16cc009e715739 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/viewmanagers/jni/react/renderer/components/rncore/EventEmitters.h +++ b/ReactAndroid/src/main/java/com/facebook/react/viewmanagers/jni/react/renderer/components/rncore/EventEmitters.h @@ -196,6 +196,10 @@ class ModalHostViewEventEmitter : public ViewEventEmitter { }; + struct OnDismiss { + + }; + enum class OnOrientationChangeOrientation { Portrait, Landscape @@ -216,6 +220,8 @@ class ModalHostViewEventEmitter : public ViewEventEmitter { void onShow(OnShow value) const; + void onDismiss(OnDismiss value) const; + void onOrientationChange(OnOrientationChange value) const; };