From 8044b2d12cfd2b937bfbe846d98f9034a88aa254 Mon Sep 17 00:00:00 2001 From: Guy Carmeli Date: Tue, 27 Aug 2019 11:18:31 +0300 Subject: [PATCH] Fix mergeOptions Merging TopBar title, buttons and status bar options is broken on iOS in 3.1.1. Parent options aren't resolved properly and default options aren't regarded when they should be. --- lib/ios/RNNBasePresenter.h | 4 ++ lib/ios/RNNBasePresenter.m | 21 ++++++++ lib/ios/RNNNavigationController.m | 2 +- lib/ios/RNNNavigationControllerPresenter.m | 11 ++-- lib/ios/RNNRootViewController.m | 14 +---- lib/ios/RNNSideMenuChildVC.m | 2 +- lib/ios/RNNTabBarController.m | 2 +- lib/ios/RNNViewControllerPresenter.m | 15 +++--- .../RNNBasePresenterTest.m | 30 +++++++++++ .../RNNTabBarControllerTest.m | 10 +--- .../UIViewController+LayoutProtocolTest.m | 51 ++++++++++++++++--- lib/ios/UIViewController+LayoutProtocol.m | 10 ++-- 12 files changed, 126 insertions(+), 46 deletions(-) diff --git a/lib/ios/RNNBasePresenter.h b/lib/ios/RNNBasePresenter.h index 6c25696009f..29ecbabc178 100644 --- a/lib/ios/RNNBasePresenter.h +++ b/lib/ios/RNNBasePresenter.h @@ -31,4 +31,8 @@ typedef void (^RNNReactViewReadyCompletionBlock)(void); - (void)renderComponents:(RNNNavigationOptions *)options perform:(RNNReactViewReadyCompletionBlock)readyBlock; - (void)viewDidLayoutSubviews; + +- (UIStatusBarStyle)getStatusBarStyle:(RNNNavigationOptions *)resolvedOptions; + +- (BOOL)isStatusBarVisibility:(UINavigationController *)stack resolvedOptions:(RNNNavigationOptions *)resolvedOptions; @end diff --git a/lib/ios/RNNBasePresenter.m b/lib/ios/RNNBasePresenter.m index 413a81da0f2..c6b9239c760 100644 --- a/lib/ios/RNNBasePresenter.m +++ b/lib/ios/RNNBasePresenter.m @@ -164,4 +164,25 @@ - (void)viewDidLayoutSubviews { - (void)applyDotIndicator:(UIViewController *)child { [[self dotIndicatorPresenter] apply:child:[child resolveOptions].bottomTab.dotIndicator]; } + +- (UIStatusBarStyle)getStatusBarStyle:(RNNNavigationOptions *)resolvedOptions { + RNNNavigationOptions *withDefault = [resolvedOptions withDefault:[self defaultOptions]]; + if ([[withDefault.statusBar.style getWithDefaultValue:@"default"] isEqualToString:@"light"]) { + return UIStatusBarStyleLightContent; + } else { + return UIStatusBarStyleDefault; + } +} + +- (BOOL)isStatusBarVisibility:(UINavigationController *)stack resolvedOptions:(RNNNavigationOptions *)resolvedOptions { + RNNNavigationOptions *withDefault = [resolvedOptions withDefault:[self defaultOptions]]; + if (withDefault.statusBar.visible.hasValue) { + return ![withDefault.statusBar.visible get]; + } else if ([withDefault.statusBar.hideWithTopBar getWithDefaultValue:NO]) { + return stack.isNavigationBarHidden; + } + return NO; +} + + @end diff --git a/lib/ios/RNNNavigationController.m b/lib/ios/RNNNavigationController.m index 29f2c074acf..ff20fa73ae1 100644 --- a/lib/ios/RNNNavigationController.m +++ b/lib/ios/RNNNavigationController.m @@ -32,7 +32,7 @@ - (UINavigationController *)navigationController { } - (UIStatusBarStyle)preferredStatusBarStyle { - return self.getCurrentChild.preferredStatusBarStyle; + return [_presenter getStatusBarStyle:self.resolveOptions]; } - (UIModalPresentationStyle)modalPresentationStyle { diff --git a/lib/ios/RNNNavigationControllerPresenter.m b/lib/ios/RNNNavigationControllerPresenter.m index 8bc2fbf4155..571367a52b6 100644 --- a/lib/ios/RNNNavigationControllerPresenter.m +++ b/lib/ios/RNNNavigationControllerPresenter.m @@ -120,12 +120,15 @@ - (void)mergeOptions:(RNNNavigationOptions *)newOptions currentOptions:(RNNNavig } - RNNLargeTitleOptions *largteTitleOptions = newOptions.topBar.largeTitle; - if (largteTitleOptions.color.hasValue || largteTitleOptions.fontSize.hasValue || largteTitleOptions.fontFamily.hasValue) { + RNNLargeTitleOptions *largeTitleOptions = newOptions.topBar.largeTitle; + if (largeTitleOptions.color.hasValue || largeTitleOptions.fontSize.hasValue || largeTitleOptions.fontFamily.hasValue) { [navigationController rnn_setNavigationBarLargeTitleFontFamily:[newOptions.topBar.largeTitle.fontFamily getWithDefaultValue:nil] fontSize:[newOptions.topBar.largeTitle.fontSize getWithDefaultValue:nil] color:[newOptions.topBar.largeTitle.color getWithDefaultValue:nil]]; } - - [navigationController rnn_setNavigationBarFontFamily:[newOptions.topBar.title.fontFamily getWithDefaultValue:nil] fontSize:[newOptions.topBar.title.fontSize getWithDefaultValue:nil] color:[newOptions.topBar.title.color getWithDefaultValue:nil]]; + + RNNNavigationOptions * withDefault = (RNNNavigationOptions *) [[newOptions mergeInOptions:currentOptions] withDefault:[self defaultOptions]]; + [navigationController rnn_setNavigationBarFontFamily:[withDefault.topBar.title.fontFamily getWithDefaultValue:nil] + fontSize:[withDefault.topBar.title.fontSize getWithDefaultValue:nil] + color:[withDefault.topBar.title.color getWithDefaultValue:nil]]; if (newOptions.topBar.component.name.hasValue) { [self setCustomNavigationBarView:newOptions perform:nil]; diff --git a/lib/ios/RNNRootViewController.m b/lib/ios/RNNRootViewController.m index ca8914a4200..79d8c9659ea 100644 --- a/lib/ios/RNNRootViewController.m +++ b/lib/ios/RNNRootViewController.m @@ -116,21 +116,11 @@ - (BOOL)isExternalViewController { } - (BOOL)prefersStatusBarHidden { - if (self.resolveOptions.statusBar.visible.hasValue) { - return ![self.resolveOptions.statusBar.visible get]; - } else if ([self.resolveOptions.statusBar.hideWithTopBar getWithDefaultValue:NO]) { - return self.navigationController.isNavigationBarHidden; - } - - return NO; + return [_presenter isStatusBarVisibility:self.navigationController resolvedOptions:self.resolveOptions]; } - (UIStatusBarStyle)preferredStatusBarStyle { - if ([[self.resolveOptions.statusBar.style getWithDefaultValue:@"default"] isEqualToString:@"light"]) { - return UIStatusBarStyleLightContent; - } else { - return UIStatusBarStyleDefault; - } + return [_presenter getStatusBarStyle:[self resolveOptions]]; } - (UIInterfaceOrientationMask)supportedInterfaceOrientations { diff --git a/lib/ios/RNNSideMenuChildVC.m b/lib/ios/RNNSideMenuChildVC.m index 9c72879ba5b..1a78d0823b3 100644 --- a/lib/ios/RNNSideMenuChildVC.m +++ b/lib/ios/RNNSideMenuChildVC.m @@ -40,7 +40,7 @@ - (UIViewController *)getCurrentChild { } - (UIStatusBarStyle)preferredStatusBarStyle { - return self.child.preferredStatusBarStyle; + return [[self presenter] getStatusBarStyle:[self resolveOptions]]; } - (UIInterfaceOrientationMask)supportedInterfaceOrientations { diff --git a/lib/ios/RNNTabBarController.m b/lib/ios/RNNTabBarController.m index 5e5fbcca4f2..6de5c6e88c9 100644 --- a/lib/ios/RNNTabBarController.m +++ b/lib/ios/RNNTabBarController.m @@ -44,7 +44,7 @@ - (void)setSelectedIndex:(NSUInteger)selectedIndex { } - (UIStatusBarStyle)preferredStatusBarStyle { - return self.selectedViewController.preferredStatusBarStyle; + return [[self presenter] getStatusBarStyle:self.resolveOptions]; } #pragma mark UITabBarControllerDelegate diff --git a/lib/ios/RNNViewControllerPresenter.m b/lib/ios/RNNViewControllerPresenter.m index f44b6f5d52d..f7c96889d6c 100644 --- a/lib/ios/RNNViewControllerPresenter.m +++ b/lib/ios/RNNViewControllerPresenter.m @@ -79,7 +79,8 @@ - (void)applyOptionsOnInit:(RNNNavigationOptions *)options { - (void)mergeOptions:(RNNNavigationOptions *)newOptions currentOptions:(RNNNavigationOptions *)currentOptions { [super mergeOptions:newOptions currentOptions:currentOptions]; - + RNNNavigationOptions * withDefault = (RNNNavigationOptions *) [[currentOptions overrideOptions:newOptions] withDefault:[self defaultOptions]]; + UIViewController* viewController = self.boundViewController; if (newOptions.backgroundImage.hasValue) { @@ -135,7 +136,7 @@ - (void)mergeOptions:(RNNNavigationOptions *)newOptions currentOptions:(RNNNavig } if (newOptions.statusBar.style.hasValue) { - [viewController rnn_setStatusBarStyle:newOptions.statusBar.style.get animated:[newOptions.statusBar.animate getWithDefaultValue:YES]]; + [viewController rnn_setStatusBarStyle:newOptions.statusBar.style.get animated:[withDefault.statusBar.animate getWithDefaultValue:YES]]; } if (newOptions.topBar.backButton.visible.hasValue) { @@ -143,17 +144,17 @@ - (void)mergeOptions:(RNNNavigationOptions *)newOptions currentOptions:(RNNNavig } if (newOptions.topBar.leftButtons || newOptions.topBar.rightButtons) { - RNNNavigationOptions* buttonsResolvedOptions = (RNNNavigationOptions *)[currentOptions overrideOptions:newOptions]; - [_navigationButtons applyLeftButtons:newOptions.topBar.leftButtons rightButtons:newOptions.topBar.rightButtons defaultLeftButtonStyle:buttonsResolvedOptions.topBar.leftButtonStyle defaultRightButtonStyle:buttonsResolvedOptions.topBar.rightButtonStyle]; + [_navigationButtons applyLeftButtons:newOptions.topBar.leftButtons rightButtons:newOptions.topBar.rightButtons defaultLeftButtonStyle:withDefault.topBar.leftButtonStyle defaultRightButtonStyle:withDefault.topBar.rightButtonStyle]; } + if (newOptions.overlay.interceptTouchOutside.hasValue) { RCTRootView* rootView = (RCTRootView*)viewController.view; rootView.passThroughTouches = !newOptions.overlay.interceptTouchOutside.get; } - - [self setTitleViewWithSubtitle:(RNNNavigationOptions *)[currentOptions overrideOptions:newOptions]]; - + + [self setTitleViewWithSubtitle:withDefault]; + if (newOptions.topBar.title.component.name.hasValue) { [self setCustomNavigationTitleView:newOptions perform:nil]; } diff --git a/lib/ios/ReactNativeNavigationTests/RNNBasePresenterTest.m b/lib/ios/ReactNativeNavigationTests/RNNBasePresenterTest.m index a3415a1cd05..adf9d075ad5 100644 --- a/lib/ios/ReactNativeNavigationTests/RNNBasePresenterTest.m +++ b/lib/ios/ReactNativeNavigationTests/RNNBasePresenterTest.m @@ -60,4 +60,34 @@ - (void)testApplyOptions_setTabBarItemBadgeShouldWhenNoValue { [self.mockBoundViewController verify]; } +- (void)testGetPreferredStatusBarStyle_returnLightIfLight { + RNNNavigationOptions * lightOptions = [[RNNNavigationOptions alloc] initEmptyOptions]; + lightOptions.statusBar.style = [[Text alloc] initWithValue:@"light"]; + + XCTAssertEqual([_uut getStatusBarStyle:lightOptions], UIStatusBarStyleLightContent); +} + +- (void)testGetPreferredStatusBarStyle_returnDefaultIfDark { + RNNNavigationOptions * darkOptions = [[RNNNavigationOptions alloc] initEmptyOptions]; + darkOptions.statusBar.style = [[Text alloc] initWithValue:@"dark"]; + + XCTAssertEqual([_uut getStatusBarStyle:darkOptions], UIStatusBarStyleDefault); +} + +- (void)testGetPreferredStatusBarStyle_returnDefaultIfNil { + RNNNavigationOptions * options = [[RNNNavigationOptions alloc] initEmptyOptions]; + + XCTAssertEqual([_uut getStatusBarStyle:options], UIStatusBarStyleDefault); +} + +- (void)testGetPreferredStatusBarStyle_considersDefaultOptions { + RNNNavigationOptions * options = [[RNNNavigationOptions alloc] initEmptyOptions]; + RNNNavigationOptions * lightOptions = [[RNNNavigationOptions alloc] initEmptyOptions]; + lightOptions.statusBar.style = [[Text alloc] initWithValue:@"light"]; + [_uut setDefaultOptions:lightOptions]; + + XCTAssertEqual([_uut getStatusBarStyle:options], UIStatusBarStyleLightContent); +} + + @end diff --git a/lib/ios/ReactNativeNavigationTests/RNNTabBarControllerTest.m b/lib/ios/ReactNativeNavigationTests/RNNTabBarControllerTest.m index aa935ff8c50..d2716efb129 100644 --- a/lib/ios/ReactNativeNavigationTests/RNNTabBarControllerTest.m +++ b/lib/ios/ReactNativeNavigationTests/RNNTabBarControllerTest.m @@ -126,15 +126,9 @@ - (void)testGetCurrentChild_shouldReturnSelectedViewController { } - (void)testPreferredStatusBarStyle_shouldInvokeSelectedViewControllerPreferredStatusBarStyle { - [[self.mockChildViewController expect] preferredStatusBarStyle]; + [[self.mockTabBarPresenter expect] getStatusBarStyle:[OCMArg any]]; [self.uut preferredStatusBarStyle]; - [self.mockChildViewController verify]; -} - -- (void)testPreferredStatusBarStyle_shouldInvokeOnSelectedViewController { - [[self.mockChildViewController expect] preferredStatusBarStyle]; - [self.uut preferredStatusBarStyle]; - [self.mockChildViewController verify]; + [self.mockTabBarPresenter verify]; } - (void)testTabBarControllerDidSelectViewControllerDelegate_shouldInvokeSendBottomTabSelectedEvent { diff --git a/lib/ios/ReactNativeNavigationTests/UIViewController+LayoutProtocolTest.m b/lib/ios/ReactNativeNavigationTests/UIViewController+LayoutProtocolTest.m index fbed5689f00..e47cda9ca17 100644 --- a/lib/ios/ReactNativeNavigationTests/UIViewController+LayoutProtocolTest.m +++ b/lib/ios/ReactNativeNavigationTests/UIViewController+LayoutProtocolTest.m @@ -9,7 +9,7 @@ @interface UIViewController_LayoutProtocolTest : XCTestCase -@property (nonatomic, retain) UIViewController* uut; +@property (nonatomic, retain) UIViewController * uut; @end @@ -18,8 +18,8 @@ @implementation UIViewController_LayoutProtocolTest - (void)setUp { [super setUp]; self.uut = [OCMockObject partialMockForObject:[UIViewController new]]; - self.uut.layoutInfo = [[RNNLayoutInfo alloc] init]; - self.uut.layoutInfo.componentId = @"componentId"; + _uut.layoutInfo = [[RNNLayoutInfo alloc] init]; + _uut.layoutInfo.componentId = @"componentId"; } - (void)testInitWithLayoutApplyDefaultOptions { @@ -53,8 +53,8 @@ - (void)testSetBackButtonIcon_withColor_shouldSetColor { - (void)testSetBackButtonIcon_withColor_shouldSetTitle { UIViewController* uut = [UIViewController new]; - UINavigationController* nav = [[UINavigationController alloc] initWithRootViewController:uut]; - NSString* title = @"Title"; + UINavigationController* nav = [[UINavigationController alloc] initWithRootViewController:uut]; + NSString* title = @"Title"; [uut rnn_setBackButtonIcon:nil withColor:nil title:title]; XCTAssertEqual(title, uut.navigationItem.backBarButtonItem.title); @@ -62,8 +62,8 @@ - (void)testSetBackButtonIcon_withColor_shouldSetTitle { - (void)testSetBackButtonIcon_withColor_shouldSetIcon { UIViewController* uut = [UIViewController new]; - UINavigationController* nav = [[UINavigationController alloc] initWithRootViewController:uut]; - UIImage* icon = [UIImage new]; + UINavigationController* nav = [[UINavigationController alloc] initWithRootViewController:uut]; + UIImage* icon = [UIImage new]; [uut rnn_setBackButtonIcon:icon withColor:nil title:nil]; XCTAssertEqual(icon, uut.navigationItem.backBarButtonItem.image); @@ -99,4 +99,41 @@ - (void)testResolveOptions { XCTAssertEqual([[parent resolveOptions].bottomTab.selectedIconColor get], UIColor.redColor); } +- (void)testMergeOptions_invokedOnParentViewController { + id parent = [OCMockObject partialMockForObject:[RNNNavigationController new]]; + RNNNavigationOptions * toMerge = [[RNNNavigationOptions alloc] initEmptyOptions]; + [(UIViewController *) [parent expect] mergeOptions:toMerge]; + + RNNNavigationController* uut = [[RNNNavigationController alloc] initWithLayoutInfo:nil creator:nil options:nil defaultOptions:nil presenter:nil eventEmitter:nil childViewControllers:nil]; + [parent addChildViewController:uut]; + + [uut mergeOptions:toMerge]; + [parent verify]; +} + +- (void)testMergeOptions_presenterIsInvokedWithResolvedOptions { + id parent = [OCMockObject partialMockForObject:[RNNNavigationController new]]; + id presenter = [OCMockObject partialMockForObject:[RNNNavigationControllerPresenter new]]; + RNNNavigationOptions * toMerge = [[RNNNavigationOptions alloc] initEmptyOptions]; + toMerge.topBar.title.color = [[Color alloc] initWithValue:[UIColor redColor]]; + + [[presenter expect] mergeOptions:toMerge currentOptions:[OCMArg checkWithBlock:^(id value) { + RNNNavigationOptions * options = (RNNNavigationOptions *) value; + XCTAssertEqual([options.topBar.title.text get], @"Initial title"); + XCTAssertEqual([options.bottomTab.text get], @"Child tab text"); + return YES; + }]]; + + RNNNavigationOptions * childOptions = [[RNNNavigationOptions alloc] initEmptyOptions]; + childOptions.bottomTab.text = [[Text alloc] initWithValue:@"Child tab text"]; + UIViewController* child = [[UIViewController alloc] initWithLayoutInfo:nil creator:nil options:childOptions defaultOptions:nil presenter:presenter eventEmitter:nil childViewControllers:nil]; + RNNNavigationOptions * initialOptions = [[RNNNavigationOptions alloc] initEmptyOptions]; + initialOptions.topBar.title.text = [[Text alloc] initWithValue:@"Initial title"]; + RNNNavigationController* uut = [[RNNNavigationController alloc] initWithLayoutInfo:nil creator:nil options:initialOptions defaultOptions:nil presenter:presenter eventEmitter:nil childViewControllers:@[child]]; + [parent addChildViewController:uut]; + + [uut mergeOptions:toMerge]; + [presenter verify]; +} + @end diff --git a/lib/ios/UIViewController+LayoutProtocol.m b/lib/ios/UIViewController+LayoutProtocol.m index de9cba797a0..5ff12862932 100644 --- a/lib/ios/UIViewController+LayoutProtocol.m +++ b/lib/ios/UIViewController+LayoutProtocol.m @@ -28,15 +28,15 @@ - (instancetype)initWithLayoutInfo:(RNNLayoutInfo *)layoutInfo return self; } -- (RNNNavigationOptions *)resolveOptions { - return (RNNNavigationOptions *) [self.options mergeInOptions:self.getCurrentChild.resolveOptions.copy]; -} - - (void)mergeOptions:(RNNNavigationOptions *)options { - [self.presenter mergeOptions:options currentOptions:self.options]; + [self.presenter mergeOptions:options currentOptions:self.resolveOptions]; [self.parentViewController mergeOptions:options]; } +- (RNNNavigationOptions *)resolveOptions { + return (RNNNavigationOptions *) [self.options mergeInOptions:self.getCurrentChild.resolveOptions.copy]; +} + - (void)overrideOptions:(RNNNavigationOptions *)options { [self.options overrideOptions:options]; }