Skip to content

Commit

Permalink
Fix options resolving issues (#5450)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
guyca committed Sep 4, 2019
1 parent 452c4e6 commit 9faf458
Show file tree
Hide file tree
Showing 16 changed files with 83 additions and 46 deletions.
2 changes: 2 additions & 0 deletions lib/ios/RNNBasePresenter.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,7 @@ typedef void (^RNNReactViewReadyCompletionBlock)(void);

- (UIStatusBarStyle)getStatusBarStyle:(RNNNavigationOptions *)resolvedOptions;

- (UIInterfaceOrientationMask)getOrientation:(RNNNavigationOptions *)options;

- (BOOL)isStatusBarVisibility:(UINavigationController *)stack resolvedOptions:(RNNNavigationOptions *)resolvedOptions;
@end
4 changes: 4 additions & 0 deletions lib/ios/RNNBasePresenter.m
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
4 changes: 0 additions & 4 deletions lib/ios/RNNNavigationController.m
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,6 @@ - (CGFloat)getTopBarHeight {
return self.navigationBar.frame.size.height;
}

- (UIInterfaceOrientationMask)supportedInterfaceOrientations {
return self.getCurrentChild.supportedInterfaceOrientations;
}

- (UINavigationController *)navigationController {
return self;
}
Expand Down
6 changes: 1 addition & 5 deletions lib/ios/RNNRootViewController.m
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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"]){
Expand Down
4 changes: 0 additions & 4 deletions lib/ios/RNNSideMenuChildVC.m
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,4 @@ - (UIStatusBarStyle)preferredStatusBarStyle {
return [[self presenter] getStatusBarStyle:[self resolveOptions]];
}

- (UIInterfaceOrientationMask)supportedInterfaceOrientations {
return self.child.supportedInterfaceOrientations;
}

@end
18 changes: 11 additions & 7 deletions lib/ios/RNNSideMenuController.m
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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];
}
Expand All @@ -120,8 +120,8 @@ - (UIStatusBarStyle)preferredStatusBarStyle {
return self.openedViewController.preferredStatusBarStyle;
}

- (UIInterfaceOrientationMask)supportedInterfaceOrientations {
return self.openedViewController.supportedInterfaceOrientations;
- (UIViewController<RNNLayoutProtocol> *)getCurrentChild {
return self.openedViewController;
}

- (UIViewController *)openedViewController {
Expand All @@ -137,8 +137,12 @@ - (UIViewController *)openedViewController {
}
}

- (UIViewController<RNNLayoutProtocol> *)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 {
Expand Down
1 change: 0 additions & 1 deletion lib/ios/RNNSideMenuOptions.m
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
#import "RNNSideMenuOptions.h"
#import "RNNSideMenuController.h"
#import "SideMenuOpenGestureModeParser.h"

@implementation RNNSideMenuOptions
Expand Down
2 changes: 0 additions & 2 deletions lib/ios/RNNSideMenuPresenter.m
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
4 changes: 0 additions & 4 deletions lib/ios/RNNTabBarController.m
Original file line number Diff line number Diff line change
Expand Up @@ -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<RNNLayoutProtocol>* vc = child;
Expand Down
2 changes: 0 additions & 2 deletions lib/ios/RNNViewControllerPresenter.m
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,4 @@ - (void)setTitleViewWithSubtitle:(RNNNavigationOptions *)options {
- (void)dealloc {
[_componentRegistry clearComponentsForParentId:self.boundComponentId];
}


@end
21 changes: 7 additions & 14 deletions lib/ios/ReactNativeNavigationTests/RNNRootViewControllerTest.m
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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;
Expand All @@ -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]);
Expand All @@ -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];
Expand Down
34 changes: 34 additions & 0 deletions lib/ios/ReactNativeNavigationTests/RNNSideMenuControllerTest.m
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down Expand Up @@ -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
2 changes: 2 additions & 0 deletions lib/ios/UIViewController+LayoutProtocol.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ typedef void (^RNNReactViewReadyCompletionBlock)(void);

- (void)mergeOptions:(RNNNavigationOptions *)options;

- (void)mergeChildOptions:(RNNNavigationOptions *)options;

- (RNNNavigationOptions *)resolveOptions;

- (void)setDefaultOptions:(RNNNavigationOptions *)defaultOptions;
Expand Down
12 changes: 11 additions & 1 deletion lib/ios/UIViewController+LayoutProtocol.m
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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();
Expand Down

0 comments on commit 9faf458

Please sign in to comment.