From 9faf458cb451829e86809d9162728eed17a7f56c Mon Sep 17 00:00:00 2001 From: Guy Carmeli Date: Wed, 4 Sep 2019 13:05:28 +0300 Subject: [PATCH] Fix options resolving issues (#5450) * Enabled option was wrongly consumed after it was merged. That meant that the next time SideMenu options were applied, the wrong enabled value was applied * supportedInterfaceOrientations didn't take default orientation value into account * mergeOptions didn't save the new options in the ViewControllers current options * SideMenu always returned the centre ViewController as the current child and didn't take open SideMenu into account Fixes #5444 --- lib/ios/RNNBasePresenter.h | 2 ++ lib/ios/RNNBasePresenter.m | 4 +++ lib/ios/RNNNavigationController.m | 4 --- lib/ios/RNNRootViewController.m | 6 +--- lib/ios/RNNSideMenuChildVC.m | 4 --- lib/ios/RNNSideMenuController.m | 18 ++++++---- lib/ios/RNNSideMenuOptions.m | 1 - lib/ios/RNNSideMenuPresenter.m | 2 -- lib/ios/RNNTabBarController.m | 4 --- lib/ios/RNNViewControllerPresenter.m | 2 -- .../RNNRootViewControllerTest.m | 21 ++++-------- .../RNNSideMenuControllerTest.m | 34 +++++++++++++++++++ .../RNNTabBarControllerTest.m | 2 +- .../UIViewController+LayoutProtocolTest.m | 11 +++++- lib/ios/UIViewController+LayoutProtocol.h | 2 ++ lib/ios/UIViewController+LayoutProtocol.m | 12 ++++++- 16 files changed, 83 insertions(+), 46 deletions(-) diff --git a/lib/ios/RNNBasePresenter.h b/lib/ios/RNNBasePresenter.h index 29ecbabc178..5f5036d3786 100644 --- a/lib/ios/RNNBasePresenter.h +++ b/lib/ios/RNNBasePresenter.h @@ -34,5 +34,7 @@ typedef void (^RNNReactViewReadyCompletionBlock)(void); - (UIStatusBarStyle)getStatusBarStyle:(RNNNavigationOptions *)resolvedOptions; +- (UIInterfaceOrientationMask)getOrientation:(RNNNavigationOptions *)options; + - (BOOL)isStatusBarVisibility:(UINavigationController *)stack resolvedOptions:(RNNNavigationOptions *)resolvedOptions; @end diff --git a/lib/ios/RNNBasePresenter.m b/lib/ios/RNNBasePresenter.m index c6b9239c760..fc65743942a 100644 --- a/lib/ios/RNNBasePresenter.m +++ b/lib/ios/RNNBasePresenter.m @@ -174,6 +174,10 @@ - (UIStatusBarStyle)getStatusBarStyle:(RNNNavigationOptions *)resolvedOptions { } } +- (UIInterfaceOrientationMask)getOrientation:(RNNNavigationOptions *)options { + return [options withDefault:[self defaultOptions]].layout.supportedOrientations; +} + - (BOOL)isStatusBarVisibility:(UINavigationController *)stack resolvedOptions:(RNNNavigationOptions *)resolvedOptions { RNNNavigationOptions *withDefault = [resolvedOptions withDefault:[self defaultOptions]]; if (withDefault.statusBar.visible.hasValue) { diff --git a/lib/ios/RNNNavigationController.m b/lib/ios/RNNNavigationController.m index ff20fa73ae1..84bb3939531 100644 --- a/lib/ios/RNNNavigationController.m +++ b/lib/ios/RNNNavigationController.m @@ -23,10 +23,6 @@ - (CGFloat)getTopBarHeight { return self.navigationBar.frame.size.height; } -- (UIInterfaceOrientationMask)supportedInterfaceOrientations { - return self.getCurrentChild.supportedInterfaceOrientations; -} - - (UINavigationController *)navigationController { return self; } diff --git a/lib/ios/RNNRootViewController.m b/lib/ios/RNNRootViewController.m index 79d8c9659ea..ec7e5c637a7 100644 --- a/lib/ios/RNNRootViewController.m +++ b/lib/ios/RNNRootViewController.m @@ -33,7 +33,7 @@ - (void)setDefaultOptions:(RNNNavigationOptions *)defaultOptions { - (void)mergeOptions:(RNNNavigationOptions *)options { [_presenter mergeOptions:options currentOptions:self.options]; - [self.parentViewController mergeOptions:options]; + [self.parentViewController mergeChildOptions:options]; } - (void)overrideOptions:(RNNNavigationOptions *)options { @@ -123,10 +123,6 @@ - (UIStatusBarStyle)preferredStatusBarStyle { return [_presenter getStatusBarStyle:[self resolveOptions]]; } -- (UIInterfaceOrientationMask)supportedInterfaceOrientations { - return self.resolveOptions.layout.supportedOrientations; -} - - (void)navigationController:(UINavigationController *)navigationController didShowViewController:(UIViewController *)viewController animated:(BOOL)animated{ RNNRootViewController* vc = (RNNRootViewController*)viewController; if (![[vc.self.resolveOptions.topBar.backButton.transition getWithDefaultValue:@""] isEqualToString:@"custom"]){ diff --git a/lib/ios/RNNSideMenuChildVC.m b/lib/ios/RNNSideMenuChildVC.m index 1a78d0823b3..4bba393f885 100644 --- a/lib/ios/RNNSideMenuChildVC.m +++ b/lib/ios/RNNSideMenuChildVC.m @@ -43,8 +43,4 @@ - (UIStatusBarStyle)preferredStatusBarStyle { return [[self presenter] getStatusBarStyle:[self resolveOptions]]; } -- (UIInterfaceOrientationMask)supportedInterfaceOrientations { - return self.child.supportedInterfaceOrientations; -} - @end diff --git a/lib/ios/RNNSideMenuController.m b/lib/ios/RNNSideMenuController.m index 2d35896fffe..2143ee17e9f 100644 --- a/lib/ios/RNNSideMenuController.m +++ b/lib/ios/RNNSideMenuController.m @@ -96,7 +96,7 @@ -(void)setControllers:(NSArray*)controllers { for (id controller in controllers) { if ([controller isKindOfClass:[RNNSideMenuChildVC class]]) { RNNSideMenuChildVC *child = (RNNSideMenuChildVC*)controller; - + if (child.type == RNNSideMenuChildTypeCenter) { self.center = child; } @@ -106,10 +106,10 @@ -(void)setControllers:(NSArray*)controllers { else if(child.type == RNNSideMenuChildTypeRight) { self.right = child; } - + [self addChildViewController:child]; } - + else { @throw [NSException exceptionWithName:@"UnknownSideMenuControllerType" reason:[@"Unknown side menu type " stringByAppendingString:[controller description]] userInfo:nil]; } @@ -120,8 +120,8 @@ - (UIStatusBarStyle)preferredStatusBarStyle { return self.openedViewController.preferredStatusBarStyle; } -- (UIInterfaceOrientationMask)supportedInterfaceOrientations { - return self.openedViewController.supportedInterfaceOrientations; +- (UIViewController *)getCurrentChild { + return self.openedViewController; } - (UIViewController *)openedViewController { @@ -137,8 +137,12 @@ - (UIViewController *)openedViewController { } } -- (UIViewController *)getCurrentChild { - return self.center; +- (RNNNavigationOptions *)resolveOptions { + RNNNavigationOptions * options = super.resolveOptions; + if (self.openedViewController != self.center) { + [options.sideMenu mergeOptions:self.center.resolveOptions.sideMenu]; + } + return options; } - (CGFloat)getTopBarHeight { diff --git a/lib/ios/RNNSideMenuOptions.m b/lib/ios/RNNSideMenuOptions.m index 93e384d93a3..aa53490112f 100644 --- a/lib/ios/RNNSideMenuOptions.m +++ b/lib/ios/RNNSideMenuOptions.m @@ -1,5 +1,4 @@ #import "RNNSideMenuOptions.h" -#import "RNNSideMenuController.h" #import "SideMenuOpenGestureModeParser.h" @implementation RNNSideMenuOptions diff --git a/lib/ios/RNNSideMenuPresenter.m b/lib/ios/RNNSideMenuPresenter.m index 601b1adaf92..c2a56c74121 100644 --- a/lib/ios/RNNSideMenuPresenter.m +++ b/lib/ios/RNNSideMenuPresenter.m @@ -66,12 +66,10 @@ - (void)mergeOptions:(RNNNavigationOptions *)newOptions currentOptions:(RNNNavig if (newOptions.sideMenu.left.enabled.hasValue) { [sideMenuController side:MMDrawerSideLeft enabled:newOptions.sideMenu.left.enabled.get]; - [newOptions.sideMenu.left.enabled consume]; } if (newOptions.sideMenu.right.enabled.hasValue) { [sideMenuController side:MMDrawerSideRight enabled:newOptions.sideMenu.right.enabled.get]; - [newOptions.sideMenu.right.enabled consume]; } if (newOptions.sideMenu.left.visible.hasValue) { diff --git a/lib/ios/RNNTabBarController.m b/lib/ios/RNNTabBarController.m index 6de5c6e88c9..b85daded9f2 100644 --- a/lib/ios/RNNTabBarController.m +++ b/lib/ios/RNNTabBarController.m @@ -24,10 +24,6 @@ - (CGFloat)getTopBarHeight { return [super getTopBarHeight]; } -- (UIInterfaceOrientationMask)supportedInterfaceOrientations { - return self.selectedViewController.supportedInterfaceOrientations; -} - - (void)setSelectedIndexByComponentID:(NSString *)componentID { for (id child in self.childViewControllers) { UIViewController* vc = child; diff --git a/lib/ios/RNNViewControllerPresenter.m b/lib/ios/RNNViewControllerPresenter.m index 46dad976614..ddac97f822a 100644 --- a/lib/ios/RNNViewControllerPresenter.m +++ b/lib/ios/RNNViewControllerPresenter.m @@ -212,6 +212,4 @@ - (void)setTitleViewWithSubtitle:(RNNNavigationOptions *)options { - (void)dealloc { [_componentRegistry clearComponentsForParentId:self.boundComponentId]; } - - @end diff --git a/lib/ios/ReactNativeNavigationTests/RNNRootViewControllerTest.m b/lib/ios/ReactNativeNavigationTests/RNNRootViewControllerTest.m index 497195a7255..da3034d1373 100644 --- a/lib/ios/ReactNativeNavigationTests/RNNRootViewControllerTest.m +++ b/lib/ios/ReactNativeNavigationTests/RNNRootViewControllerTest.m @@ -373,11 +373,9 @@ -(void)testOrientation_default { -(void)testOrientationTabsController_portrait { NSArray* supportedOrientations = @[@"portrait"]; self.options.layout.orientation = supportedOrientations; - __unused RNNTabBarController* vc = [[RNNTabBarController alloc] init]; - NSMutableArray* controllers = [NSMutableArray new]; + NSMutableArray* controllers = [[NSMutableArray alloc] initWithArray:@[self.uut]]; + __unused RNNTabBarController* vc = [[RNNTabBarController alloc] initWithLayoutInfo:nil creator:nil options:[[RNNNavigationOptions alloc] initEmptyOptions] defaultOptions:nil presenter:[RNNViewControllerPresenter new] eventEmitter:nil childViewControllers:controllers]; - [controllers addObject:self.uut]; - [vc setViewControllers:controllers]; [self.uut viewWillAppear:false]; UIInterfaceOrientationMask expectedOrientation = UIInterfaceOrientationMaskPortrait; @@ -387,11 +385,9 @@ -(void)testOrientationTabsController_portrait { -(void)testOrientationTabsController_portraitAndLandscape { NSArray* supportedOrientations = @[@"portrait", @"landscape"]; self.options.layout.orientation = supportedOrientations; - __unused RNNTabBarController* vc = [[RNNTabBarController alloc] init]; - NSMutableArray* controllers = [NSMutableArray new]; + NSMutableArray* controllers = [[NSMutableArray alloc] initWithArray:@[self.uut]]; + __unused RNNTabBarController* vc = [[RNNTabBarController alloc] initWithLayoutInfo:nil creator:nil options:[[RNNNavigationOptions alloc] initEmptyOptions] defaultOptions:nil presenter:[RNNViewControllerPresenter new] eventEmitter:nil childViewControllers:controllers]; - [controllers addObject:self.uut]; - [vc setViewControllers:controllers]; [self.uut viewWillAppear:false]; UIInterfaceOrientationMask expectedOrientation = (UIInterfaceOrientationMaskPortrait | UIInterfaceOrientationMaskLandscape); @@ -401,11 +397,9 @@ -(void)testOrientationTabsController_portraitAndLandscape { -(void)testOrientationTabsController_all { NSArray* supportedOrientations = @[@"all"]; self.options.layout.orientation = supportedOrientations; - __unused RNNTabBarController* vc = [[RNNTabBarController alloc] init]; - NSMutableArray* controllers = [NSMutableArray new]; + NSMutableArray* controllers = [[NSMutableArray alloc] initWithArray:@[self.uut]]; + __unused RNNTabBarController* vc = [[RNNTabBarController alloc] initWithLayoutInfo:nil creator:nil options:[[RNNNavigationOptions alloc] initEmptyOptions] defaultOptions:nil presenter:[RNNViewControllerPresenter new] eventEmitter:nil childViewControllers:controllers]; - [controllers addObject:self.uut]; - [vc setViewControllers:controllers]; [self.uut viewWillAppear:false]; UIInterfaceOrientationMask expectedOrientation = UIInterfaceOrientationMaskAll; @@ -417,7 +411,7 @@ -(void)testRightButtonsWithTitle_withoutStyle { self.uut = [[RNNRootViewController alloc] initWithLayoutInfo:nil rootViewCreator:nil eventEmitter:nil presenter:[RNNViewControllerPresenter new] options:self.options defaultOptions:nil]; RNNNavigationController* nav = [[RNNNavigationController alloc] initWithLayoutInfo:nil creator:_creator options:nil defaultOptions:nil presenter:nil eventEmitter:nil childViewControllers:@[self.uut]]; - RNNUIBarButtonItem* button = (RNNUIBarButtonItem*)[nav.topViewController.navigationItem.rightBarButtonItems objectAtIndex:0]; + RNNUIBarButtonItem* button = (RNNUIBarButtonItem*) nav.topViewController.navigationItem.rightBarButtonItems[0]; NSString* expectedButtonId = @"testId"; NSString* expectedTitle = @"test"; XCTAssertTrue([button.buttonId isEqualToString:expectedButtonId]); @@ -442,7 +436,6 @@ -(void)testRightButtonsWithTitle_withStyle { //TODO: Determine how to tests buttonColor,buttonFontSize and buttonFontWeight? } - -(void)testLeftButtonsWithTitle_withoutStyle { self.options.topBar.leftButtons = @[@{@"id": @"testId", @"text": @"test"}]; self.uut = [[RNNRootViewController alloc] initWithLayoutInfo:nil rootViewCreator:nil eventEmitter:nil presenter:[RNNViewControllerPresenter new] options:self.options defaultOptions:nil]; diff --git a/lib/ios/ReactNativeNavigationTests/RNNSideMenuControllerTest.m b/lib/ios/ReactNativeNavigationTests/RNNSideMenuControllerTest.m index 8a33c10b6d9..39b224b7599 100644 --- a/lib/ios/ReactNativeNavigationTests/RNNSideMenuControllerTest.m +++ b/lib/ios/ReactNativeNavigationTests/RNNSideMenuControllerTest.m @@ -29,4 +29,38 @@ - (void)testSetSideMenuWidthShouldUpdateRightReactViewFrameWidth { XCTAssertEqual(self.uut.right.child.view.frame.size.width, 150.f); } +- (void)testGetCurrentChild { + XCTestExpectation *expectation = [self expectationWithDescription:@"Testing Async Method"]; + XCTAssertEqual(_uut.getCurrentChild, _centerVC); + + [_uut openDrawerSide:MMDrawerSideLeft animated:NO completion:(void (^)(BOOL)) ^{ + XCTAssertEqual(_uut.getCurrentChild, _leftVC); + }]; + + [_uut closeDrawerAnimated:NO completion:nil]; + [_uut openDrawerSide:MMDrawerSideRight animated:NO completion:(void (^)(BOOL)) ^{ + XCTAssertEqual(_uut.getCurrentChild, _rightVC); + [expectation fulfill]; + }]; + + [self waitForExpectationsWithTimeout:1 handler:nil]; +} + +- (void)testResolveOptions { + XCTestExpectation *expectation = [self expectationWithDescription:@"Testing Async Method"]; + + RNNNavigationOptions *options = [[RNNNavigationOptions alloc] initEmptyOptions]; + options.sideMenu.left.visible = [[Bool alloc] initWithBOOL:YES]; + [_centerVC overrideOptions:options]; + + XCTAssertTrue(_uut.resolveOptions.sideMenu.left.visible); + + [_uut openDrawerSide:MMDrawerSideLeft animated:NO completion:^(BOOL finished) { + XCTAssertTrue(_uut.resolveOptions.sideMenu.left.visible); + [expectation fulfill]; + }]; + + [self waitForExpectationsWithTimeout:1 handler:nil]; +} + @end diff --git a/lib/ios/ReactNativeNavigationTests/RNNTabBarControllerTest.m b/lib/ios/ReactNativeNavigationTests/RNNTabBarControllerTest.m index d2716efb129..4e1bd0a90f8 100644 --- a/lib/ios/ReactNativeNavigationTests/RNNTabBarControllerTest.m +++ b/lib/ios/ReactNativeNavigationTests/RNNTabBarControllerTest.m @@ -100,7 +100,7 @@ - (void)testMergeOptions_shouldInvokeParentMergeOptions { RNNNavigationOptions *options = [[RNNNavigationOptions alloc] initWithDict:@{}]; OCMStub([self.uut parentViewController]).andReturn(parentMock); - [((RNNRootViewController *) [parentMock expect]) mergeOptions:options]; + [((RNNRootViewController *) [parentMock expect]) mergeChildOptions:options]; [self.uut mergeOptions:options]; [parentMock verify]; } diff --git a/lib/ios/ReactNativeNavigationTests/UIViewController+LayoutProtocolTest.m b/lib/ios/ReactNativeNavigationTests/UIViewController+LayoutProtocolTest.m index e47cda9ca17..75db50905c7 100644 --- a/lib/ios/ReactNativeNavigationTests/UIViewController+LayoutProtocolTest.m +++ b/lib/ios/ReactNativeNavigationTests/UIViewController+LayoutProtocolTest.m @@ -102,7 +102,7 @@ - (void)testResolveOptions { - (void)testMergeOptions_invokedOnParentViewController { id parent = [OCMockObject partialMockForObject:[RNNNavigationController new]]; RNNNavigationOptions * toMerge = [[RNNNavigationOptions alloc] initEmptyOptions]; - [(UIViewController *) [parent expect] mergeOptions:toMerge]; + [(UIViewController *) [parent expect] mergeChildOptions:toMerge]; RNNNavigationController* uut = [[RNNNavigationController alloc] initWithLayoutInfo:nil creator:nil options:nil defaultOptions:nil presenter:nil eventEmitter:nil childViewControllers:nil]; [parent addChildViewController:uut]; @@ -136,4 +136,13 @@ - (void)testMergeOptions_presenterIsInvokedWithResolvedOptions { [presenter verify]; } +- (void)testMergeOptions_mergedIntoCurrentOptions { + UIViewController* uut = [[UIViewController alloc] initWithLayoutInfo:nil creator:nil options:[[RNNNavigationOptions alloc] initEmptyOptions] defaultOptions:nil presenter:nil eventEmitter:nil childViewControllers:nil]; + RNNNavigationOptions * toMerge = [[RNNNavigationOptions alloc] initEmptyOptions]; + toMerge.topBar.title.text = [[Text alloc] initWithValue:@"merged"]; + + [uut mergeOptions:toMerge]; + XCTAssertEqual(uut.resolveOptions.topBar.title.text.get, @"merged"); +} + @end diff --git a/lib/ios/UIViewController+LayoutProtocol.h b/lib/ios/UIViewController+LayoutProtocol.h index 1bc4cedcf1c..f22c8f6784e 100644 --- a/lib/ios/UIViewController+LayoutProtocol.h +++ b/lib/ios/UIViewController+LayoutProtocol.h @@ -12,6 +12,8 @@ typedef void (^RNNReactViewReadyCompletionBlock)(void); - (void)mergeOptions:(RNNNavigationOptions *)options; +- (void)mergeChildOptions:(RNNNavigationOptions *)options; + - (RNNNavigationOptions *)resolveOptions; - (void)setDefaultOptions:(RNNNavigationOptions *)defaultOptions; diff --git a/lib/ios/UIViewController+LayoutProtocol.m b/lib/ios/UIViewController+LayoutProtocol.m index 5ff12862932..402e21191d0 100644 --- a/lib/ios/UIViewController+LayoutProtocol.m +++ b/lib/ios/UIViewController+LayoutProtocol.m @@ -29,8 +29,14 @@ - (instancetype)initWithLayoutInfo:(RNNLayoutInfo *)layoutInfo } - (void)mergeOptions:(RNNNavigationOptions *)options { + [self.options overrideOptions:options]; [self.presenter mergeOptions:options currentOptions:self.resolveOptions]; - [self.parentViewController mergeOptions:options]; + [self.parentViewController mergeChildOptions:options]; +} + +- (void)mergeChildOptions:(RNNNavigationOptions *)options { + [self.presenter mergeOptions:options currentOptions:self.resolveOptions]; + [self.parentViewController mergeChildOptions:options]; } - (RNNNavigationOptions *)resolveOptions { @@ -41,6 +47,10 @@ - (void)overrideOptions:(RNNNavigationOptions *)options { [self.options overrideOptions:options]; } +- (UIInterfaceOrientationMask)supportedInterfaceOrientations { + return [self.presenter getOrientation:[self resolveOptions]]; +} + - (void)renderTreeAndWait:(BOOL)wait perform:(RNNReactViewReadyCompletionBlock)readyBlock { dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_HIGH, 0), ^{ dispatch_group_t group = dispatch_group_create();