From ec60931d76451d062632d40df0b22ab7404e24ac Mon Sep 17 00:00:00 2001 From: Yogev Ben David Date: Thu, 17 Jun 2021 17:51:02 +0300 Subject: [PATCH] Support react-native modals (#7156) Currently, using react-native declarative modals can cause view controllers to be detached from the view hierarchy. For example, when presenting an RNN modal from react-native modal and then dismissing the react-native modal, the RNN modal on top of it remain detached as its holder is dismissed. It can also cause the new RNN modal to be dismissed. In this PR we implemented `RCTModalHostViewManager.presentationBlock` and `RCTModalHostViewManager.dismissalBlock` in order to have the same modals logic we have in RNN. --- e2e/Modals.test.js | 7 +++ lib/ios/RNNBridgeManager.mm | 8 +-- lib/ios/RNNCommandsHandler.m | 22 +++++--- lib/ios/RNNEnterExitAnimation.m | 4 +- lib/ios/RNNEventEmitter.m | 6 ++- lib/ios/RNNModalManager.h | 3 ++ lib/ios/RNNModalManager.m | 51 ++++++++++++------- lib/ios/TransitionOptions.h | 1 + lib/ios/TransitionOptions.m | 6 +++ package.json | 4 +- .../NavigationTests/RNNCommandsHandlerTest.m | 23 ++++++++- .../ios/NavigationTests/RNNModalManagerTest.m | 22 ++------ playground/src/screens/ModalScreen.tsx | 32 +++++++++++- playground/src/testIDs.ts | 2 + 14 files changed, 137 insertions(+), 54 deletions(-) diff --git a/e2e/Modals.test.js b/e2e/Modals.test.js index 90defc17c49..1269cc8c040 100644 --- a/e2e/Modals.test.js +++ b/e2e/Modals.test.js @@ -158,4 +158,11 @@ describe('modal', () => { 'dismissModal promise resolved with: UniqueStackId' ); }); + + it('dismiss previous react-native modal', async () => { + await elementById(TestIDs.TOGGLE_REACT_NATIVE_MODAL).tap(); + await elementById(TestIDs.SHOW_MODAL_AND_DISMISS_REACT_NATIVE_MODAL).tap(); + await elementById(TestIDs.DISMISS_MODAL_BTN).tap(); + await expect(elementById(TestIDs.MODAL_SCREEN_HEADER)).toBeVisible(); + }); }); diff --git a/lib/ios/RNNBridgeManager.mm b/lib/ios/RNNBridgeManager.mm index 4b6a472e003..5188fe6b977 100644 --- a/lib/ios/RNNBridgeManager.mm +++ b/lib/ios/RNNBridgeManager.mm @@ -1,8 +1,5 @@ #import "RNNBridgeManager.h" -#import -#import - #import "RNNBridgeModule.h" #import "RNNComponentViewCreator.h" #import "RNNEventEmitter.h" @@ -10,6 +7,9 @@ #import "RNNReactComponentRegistry.h" #import "RNNReactRootViewCreator.h" #import "RNNSplashScreen.h" +#import +#import +#import @interface RNNBridgeManager () @@ -105,6 +105,8 @@ - (void)onJavaScriptWillLoad { - (void)onJavaScriptLoaded { [_commandsHandler setReadyToReceiveCommands:true]; + [_modalManager + connectModalHostViewManager:[self.bridge moduleForClass:RCTModalHostViewManager.class]]; [[_bridge moduleForClass:[RNNEventEmitter class]] sendOnAppLaunched]; } diff --git a/lib/ios/RNNCommandsHandler.m b/lib/ios/RNNCommandsHandler.m index 881e0243b54..9bae66850f1 100644 --- a/lib/ios/RNNCommandsHandler.m +++ b/lib/ios/RNNCommandsHandler.m @@ -1,6 +1,7 @@ #import "RNNCommandsHandler.h" #import "RNNAssert.h" #import "RNNComponentViewController.h" +#import "RNNConvert.h" #import "RNNDefaultOptionsHelper.h" #import "RNNErrorHandler.h" #import "React/RCTI18nUtil.h" @@ -368,6 +369,12 @@ - (void)showModal:(NSDictionary *)layout __weak UIViewController *weakNewVC = newVc; newVc.waitForRender = [newVc.resolveOptionsWithDefault.animations.showModal shouldWaitForRender]; + newVc.modalPresentationStyle = + [RNNConvert UIModalPresentationStyle:[newVc.resolveOptionsWithDefault.modalPresentationStyle + withDefault:@"default"]]; + newVc.modalTransitionStyle = + [RNNConvert UIModalTransitionStyle:[newVc.resolveOptionsWithDefault.modalTransitionStyle + withDefault:@"coverVertical"]]; [newVc setReactViewReadyCallback:^{ [self->_modalManager showModal:weakNewVC @@ -403,12 +410,15 @@ - (void)dismissModal:(NSString *)componentId RNNNavigationOptions *options = [[RNNNavigationOptions alloc] initWithDict:mergeOptions]; [modalToDismiss.presentedComponentViewController mergeOptions:options]; - [_modalManager dismissModal:modalToDismiss - completion:^{ - [self->_eventEmitter sendOnNavigationCommandCompletion:dismissModal - commandId:commandId]; - completion(modalToDismiss.topMostViewController.layoutInfo.componentId); - }]; + [_modalManager + dismissModal:modalToDismiss + animated:[modalToDismiss.resolveOptionsWithDefault.animations.dismissModal.exit.enable + withDefault:YES] + completion:^{ + [self->_eventEmitter sendOnNavigationCommandCompletion:dismissModal + commandId:commandId]; + completion(modalToDismiss.topMostViewController.layoutInfo.componentId); + }]; } - (void)dismissAllModals:(NSDictionary *)mergeOptions diff --git a/lib/ios/RNNEnterExitAnimation.m b/lib/ios/RNNEnterExitAnimation.m index 1d8d0d023e5..5da7c59e29a 100644 --- a/lib/ios/RNNEnterExitAnimation.m +++ b/lib/ios/RNNEnterExitAnimation.m @@ -12,9 +12,9 @@ - (instancetype)initWithDict:(NSDictionary *)dict { } - (void)mergeOptions:(RNNEnterExitAnimation *)options { - if (options.enter.hasAnimation) + if (options.enter.hasValue) self.enter = options.enter; - if (options.exit.hasAnimation) + if (options.exit.hasValue) self.exit = options.exit; } diff --git a/lib/ios/RNNEventEmitter.m b/lib/ios/RNNEventEmitter.m index 2fffdb4c330..1882d60d572 100644 --- a/lib/ios/RNNEventEmitter.m +++ b/lib/ios/RNNEventEmitter.m @@ -124,8 +124,10 @@ - (void)sendOnPreviewCompleted:(NSString *)componentId - (void)sendModalsDismissedEvent:(NSString *)componentId numberOfModalsDismissed:(NSNumber *)modalsDismissed { - [self send:ModalDismissed - body:@{@"componentId" : componentId, @"modalsDismissed" : modalsDismissed}]; + if (componentId) { + [self send:ModalDismissed + body:@{@"componentId" : componentId, @"modalsDismissed" : modalsDismissed}]; + } } - (void)sendModalAttemptedToDismissEvent:(NSString *)componentId { diff --git a/lib/ios/RNNModalManager.h b/lib/ios/RNNModalManager.h index 6f412cccc51..609e6bb8065 100644 --- a/lib/ios/RNNModalManager.h +++ b/lib/ios/RNNModalManager.h @@ -1,6 +1,7 @@ #import "RNNModalManagerEventHandler.h" #import #import +#import #import typedef void (^RNNTransitionCompletionBlock)(void); @@ -12,11 +13,13 @@ typedef void (^RNNTransitionRejectionBlock)(NSString *_Nonnull code, NSString *_ - (instancetype _Nonnull)initWithBridge:(RCTBridge *_Nonnull)bridge eventHandler:(RNNModalManagerEventHandler *_Nonnull)eventHandler; +- (void)connectModalHostViewManager:(RCTModalHostViewManager *_Nonnull)modalHostViewManager; - (void)showModal:(UIViewController *_Nonnull)viewController animated:(BOOL)animated completion:(RNNTransitionWithComponentIdCompletionBlock _Nullable)completion; - (void)dismissModal:(UIViewController *_Nullable)viewController + animated:(BOOL)animated completion:(RNNTransitionCompletionBlock _Nullable)completion; - (void)dismissAllModalsAnimated:(BOOL)animated completion:(void (^__nullable)(void))completion; - (void)dismissAllModalsSynchronosly; diff --git a/lib/ios/RNNModalManager.m b/lib/ios/RNNModalManager.m index a586882189d..c6f41d6e967 100644 --- a/lib/ios/RNNModalManager.m +++ b/lib/ios/RNNModalManager.m @@ -33,6 +33,30 @@ - (instancetype)initWithBridge:(RCTBridge *)bridge return self; } +- (void)connectModalHostViewManager:(RCTModalHostViewManager *)modalHostViewManager { + modalHostViewManager.presentationBlock = + ^(UIViewController *reactViewController, UIViewController *viewController, BOOL animated, + dispatch_block_t completionBlock) { + [self showModal:viewController + animated:animated + completion:^(NSString *_Nonnull componentId) { + if (completionBlock) + completionBlock(); + }]; + }; + + modalHostViewManager.dismissalBlock = + ^(UIViewController *reactViewController, UIViewController *viewController, BOOL animated, + dispatch_block_t completionBlock) { + [self dismissModal:viewController + animated:animated + completion:^{ + if (completionBlock) + completionBlock(); + }]; + }; +} + - (void)showModal:(UIViewController *)viewController animated:(BOOL)animated completion:(RNNTransitionWithComponentIdCompletionBlock)completion { @@ -44,13 +68,6 @@ - (void)showModal:(UIViewController *)viewController UIViewController *topVC = [self topPresentedVC]; - viewController.modalPresentationStyle = [RNNConvert - UIModalPresentationStyle:[viewController.resolveOptionsWithDefault.modalPresentationStyle - withDefault:@"default"]]; - viewController.modalTransitionStyle = [RNNConvert - UIModalTransitionStyle:[viewController.resolveOptionsWithDefault.modalTransitionStyle - withDefault:@"coverVertical"]]; - if (viewController.presentationController) { viewController.presentationController.delegate = self; } @@ -80,10 +97,11 @@ - (void)showModal:(UIViewController *)viewController } - (void)dismissModal:(UIViewController *)viewController + animated:(BOOL)animated completion:(RNNTransitionCompletionBlock)completion { if (viewController) { [_pendingModalIdsToDismiss addObject:viewController]; - [self removePendingNextModalIfOnTop:completion]; + [self removePendingNextModalIfOnTop:completion animated:animated]; } } @@ -128,7 +146,8 @@ - (void)dismissAllModalsSynchronosly { #pragma mark - private -- (void)removePendingNextModalIfOnTop:(RNNTransitionCompletionBlock)completion { +- (void)removePendingNextModalIfOnTop:(RNNTransitionCompletionBlock)completion + animated:(BOOL)animated { UIViewController *modalToDismiss = [_pendingModalIdsToDismiss lastObject]; RNNNavigationOptions *optionsWithDefault = modalToDismiss.resolveOptionsWithDefault; @@ -152,12 +171,11 @@ - (void)removePendingNextModalIfOnTop:(RNNTransitionCompletionBlock)completion { _dismissModalTransitionDelegate; } - if (modalToDismiss == topPresentedVC || - [[topPresentedVC childViewControllers] containsObject:modalToDismiss]) { + if ((modalToDismiss == topPresentedVC || + [[topPresentedVC childViewControllers] containsObject:modalToDismiss])) { [self dismissSearchController:modalToDismiss]; [modalToDismiss - dismissViewControllerAnimated:[optionsWithDefault.animations.dismissModal.exit.enable - withDefault:YES] + dismissViewControllerAnimated:animated completion:^{ [self->_pendingModalIdsToDismiss removeObject:modalToDismiss]; if (modalToDismiss.view) { @@ -168,18 +186,15 @@ - (void)removePendingNextModalIfOnTop:(RNNTransitionCompletionBlock)completion { completion(); } - [self removePendingNextModalIfOnTop:nil]; + [self removePendingNextModalIfOnTop:nil animated:NO]; }]; } else { [modalToDismiss.view removeFromSuperview]; modalToDismiss.view = nil; - modalToDismiss.getCurrentChild.resolveOptions.animations.dismissModal.exit.enable = - [[Bool alloc] initWithBOOL:NO]; [self dismissedModal:modalToDismiss]; - if (completion) { + if (completion) completion(); - } } } diff --git a/lib/ios/TransitionOptions.h b/lib/ios/TransitionOptions.h index a34c61178f8..06d2822564b 100644 --- a/lib/ios/TransitionOptions.h +++ b/lib/ios/TransitionOptions.h @@ -17,5 +17,6 @@ - (NSTimeInterval)maxDuration; - (BOOL)hasAnimation; +- (BOOL)hasValue; @end diff --git a/lib/ios/TransitionOptions.m b/lib/ios/TransitionOptions.m index 0cdc62b568d..18c227b6bc0 100644 --- a/lib/ios/TransitionOptions.m +++ b/lib/ios/TransitionOptions.m @@ -35,6 +35,12 @@ - (void)mergeOptions:(TransitionOptions *)options { } - (BOOL)hasAnimation { + return self.x.hasAnimation || self.y.hasAnimation || self.alpha.hasAnimation || + self.translationX.hasAnimation || self.translationY.hasAnimation || + self.rotationX.hasAnimation || self.rotationY.hasAnimation; +} + +- (BOOL)hasValue { return self.x.hasAnimation || self.y.hasAnimation || self.alpha.hasAnimation || self.translationX.hasAnimation || self.translationY.hasAnimation || self.rotationX.hasAnimation || self.rotationY.hasAnimation || diff --git a/package.json b/package.json index bec5a073e2f..be021d3a69d 100644 --- a/package.json +++ b/package.json @@ -177,13 +177,13 @@ "binaryPath": "playground/android/app/build/outputs/apk/debug/app-debug.apk", "build": "cd playground/android && ./gradlew app:assembleDebug app:assembleAndroidTest -DtestBuildType=debug", "type": "android.emulator", - "name": "Pixel_API_28" + "name": "Pixel_3A_API_29" }, "android.emu.release": { "binaryPath": "playground/android/app/build/outputs/apk/release/app-release.apk", "build": "cd playground/android && ./gradlew app:assembleRelease app:assembleAndroidTest -DtestBuildType=release", "type": "android.emulator", - "name": "Pixel_API_28" + "name": "Pixel_3A_API_29" } } } diff --git a/playground/ios/NavigationTests/RNNCommandsHandlerTest.m b/playground/ios/NavigationTests/RNNCommandsHandlerTest.m index 1dedbd03c6b..0dc5318a097 100644 --- a/playground/ios/NavigationTests/RNNCommandsHandlerTest.m +++ b/playground/ios/NavigationTests/RNNCommandsHandlerTest.m @@ -749,7 +749,7 @@ - (void)testDismissModal_shouldResolveTopMostComponentId { eventEmitter:nil childViewControllers:@[ child ]]; - OCMStub([self.modalManager dismissModal:OCMArg.any completion:OCMArg.invokeBlock]); + OCMStub([self.modalManager dismissModal:OCMArg.any animated:NO completion:OCMArg.invokeBlock]); OCMStub(child.isModal).andReturn(YES); OCMStub([self.layoutManager findComponentForId:@"child"]).andReturn(child); @@ -787,11 +787,12 @@ - (void)testDismissModal_shouldMergeOptions { dismissModal:[OCMArg checkWithBlock:^BOOL(UIViewController *modalToDismiss) { return modalToDismiss.options.animations.dismissModal.exit.enable.get == NO; }] + animated:NO completion:OCMArg.any]; [self.uut dismissModal:@"child" commandId:@"commandId" - mergeOptions:@{@"animations" : @{@"dismissModal" : @{@"enabled" : @(0)}}} + mergeOptions:@{@"animations" : @{@"dismissModal" : @{@"exit" : @{@"enabled" : @(0)}}}} completion:^(NSString *_Nonnull componentId) { XCTAssertTrue([componentId isEqualToString:@"stack"]); } @@ -802,6 +803,24 @@ - (void)testDismissModal_shouldMergeOptions { [self.modalManager verify]; } +- (void)testShowModal_withPresentationStyle { + [self.uut setReadyToReceiveCommands:true]; + OCMStub([self.controllerFactory createLayout:[OCMArg any]]).andReturn(_vc1); + _vc1.options = [RNNNavigationOptions emptyOptions]; + _vc1.options.modalPresentationStyle = [Text withValue:@"overCurrentContext"]; + [self.uut showModal:@{} commandId:@"" completion:nil]; + XCTAssertEqual(_vc1.modalPresentationStyle, UIModalPresentationOverCurrentContext); +} + +- (void)testApplyOptionsOnInit_shouldShowModalWithTransitionStyle { + [self.uut setReadyToReceiveCommands:true]; + OCMStub([self.controllerFactory createLayout:[OCMArg any]]).andReturn(_vc1); + _vc1.options = [RNNNavigationOptions emptyOptions]; + _vc1.options.modalTransitionStyle = [Text withValue:@"crossDissolve"]; + [self.uut showModal:@{} commandId:@"" completion:nil]; + XCTAssertEqual(_vc1.modalTransitionStyle, UIModalTransitionStyleCrossDissolve); +} + - (void)testPush_shouldResolvePromiseAndSendCommandCompletionWithPushedComponentId { [self.uut setReadyToReceiveCommands:true]; NSString *expectedComponentId = @"pushedComponent"; diff --git a/playground/ios/NavigationTests/RNNModalManagerTest.m b/playground/ios/NavigationTests/RNNModalManagerTest.m index c8f97c1f3b5..93b1cf322a8 100644 --- a/playground/ios/NavigationTests/RNNModalManagerTest.m +++ b/playground/ios/NavigationTests/RNNModalManagerTest.m @@ -89,7 +89,7 @@ - (void)testDismissModal_InvokeDelegateWithCorrectParameters { [_modalManager showModal:_vc3 animated:NO completion:nil]; [[_modalManagerEventHandler expect] dismissedModal:_vc3]; - [_modalManager dismissModal:_vc3 completion:nil]; + [_modalManager dismissModal:_vc3 animated:NO completion:nil]; [_modalManagerEventHandler verify]; } @@ -99,7 +99,7 @@ - (void)testDismissPreviousModal_InvokeDelegateWithCorrectParameters { [_modalManager showModal:_vc3 animated:NO completion:nil]; [[_modalManagerEventHandler expect] dismissedModal:_vc2]; - [_modalManager dismissModal:_vc2 completion:nil]; + [_modalManager dismissModal:_vc2 animated:NO completion:nil]; [_modalManagerEventHandler verify]; } @@ -113,7 +113,7 @@ - (void)testDismissAllModals_AfterDismissingPreviousModal_InvokeDelegateWithCorr [_modalManager showModal:_vc3 animated:NO completion:nil]; [[_modalManagerEventHandler expect] dismissedModal:_vc2]; - [_modalManager dismissModal:_vc2 completion:nil]; + [_modalManager dismissModal:_vc2 animated:NO completion:nil]; [_modalManagerEventHandler verify]; [[_modalManagerEventHandler expect] dismissedMultipleModals:@[ _vc1, _vc3 ]]; @@ -123,7 +123,7 @@ - (void)testDismissAllModals_AfterDismissingPreviousModal_InvokeDelegateWithCorr - (void)testDismissModal_DismissNilModalDoesntCrash { [[_modalManagerEventHandler reject] dismissedModal:OCMArg.any]; - [_modalManager dismissModal:nil completion:nil]; + [_modalManager dismissModal:nil animated:NO completion:nil]; [_modalManagerEventHandler verify]; } @@ -170,18 +170,4 @@ - (void)testApplyOptionsOnInit_shouldShowModalWithDefaultTransitionStyle { XCTAssertEqual(_vc1.modalTransitionStyle, UIModalTransitionStyleCoverVertical); } -- (void)testApplyOptionsOnInit_shouldShowModalWithPresentationStyle { - _vc1.options = [RNNNavigationOptions emptyOptions]; - _vc1.options.modalPresentationStyle = [Text withValue:@"overCurrentContext"]; - [_modalManager showModal:_vc1 animated:NO completion:nil]; - XCTAssertEqual(_vc1.modalPresentationStyle, UIModalPresentationOverCurrentContext); -} - -- (void)testApplyOptionsOnInit_shouldShowModalWithTransitionStyle { - _vc1.options = [RNNNavigationOptions emptyOptions]; - _vc1.options.modalTransitionStyle = [Text withValue:@"crossDissolve"]; - [_modalManager showModal:_vc1 animated:NO completion:nil]; - XCTAssertEqual(_vc1.modalTransitionStyle, UIModalTransitionStyleCrossDissolve); -} - @end diff --git a/playground/src/screens/ModalScreen.tsx b/playground/src/screens/ModalScreen.tsx index ffe2f00dc9b..9663fafdf4d 100644 --- a/playground/src/screens/ModalScreen.tsx +++ b/playground/src/screens/ModalScreen.tsx @@ -12,7 +12,7 @@ import { stack } from '../commons/Layouts'; import Screens from './Screens'; import flags from '../flags'; import testIDs from '../testIDs'; -import { Dimensions } from 'react-native'; +import { Dimensions, Modal } from 'react-native'; const height = Math.round(Dimensions.get('window').height); const MODAL_ANIMATION_DURATION = 350; @@ -30,6 +30,8 @@ const { DISMISS_ALL_MODALS_BTN, DISMISS_FIRST_MODAL_BTN, SET_ROOT, + TOGGLE_REACT_NATIVE_MODAL, + SHOW_MODAL_AND_DISMISS_REACT_NATIVE_MODAL, } = testIDs; interface Props { @@ -39,6 +41,7 @@ interface Props { interface State { swipeableToDismiss: boolean; + reactNativeModalVisible: boolean; } export default class ModalScreen extends NavigationComponent { @@ -57,6 +60,7 @@ export default class ModalScreen extends NavigationComponent { super(props); this.state = { swipeableToDismiss: false, + reactNativeModalVisible: false, }; } @@ -124,10 +128,36 @@ export default class ModalScreen extends NavigationComponent { label={`Toggle to swipeToDismiss: ${this.state.swipeableToDismiss}`} onPress={this.toggleSwipeToDismiss} /> +