Skip to content

Commit

Permalink
Remove component if title is defined in mergeOptions (#5634)
Browse files Browse the repository at this point in the history
When mergeOptions was called without a title component, existing component was undesirably removed.
This commit changes how component is removed when mergeOptions is called. It will now be removed if a component is not defined and title is defined in mergeOptions.
Fixes #5628
  • Loading branch information
guyca authored Nov 7, 2019
1 parent 751670c commit 6d446a8
Show file tree
Hide file tree
Showing 14 changed files with 90 additions and 40 deletions.
2 changes: 1 addition & 1 deletion lib/ios/RNNBasePresenter.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ typedef void (^RNNReactViewReadyCompletionBlock)(void);

- (instancetype)initWithDefaultOptions:(RNNNavigationOptions *)defaultOptions;

- (void)bindViewController:(UIViewController *)boundViewController;
- (void)boundViewController:(UIViewController *)boundViewController;

- (void)setDefaultOptions:(RNNNavigationOptions *)defaultOptions;

Expand Down
2 changes: 1 addition & 1 deletion lib/ios/RNNBasePresenter.m
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ -(instancetype)initWithDefaultOptions:(RNNNavigationOptions *)defaultOptions {
return self;
}

- (void)bindViewController:(UIViewController <RNNLayoutProtocol> *)boundViewController {
- (void)boundViewController:(UIViewController *)boundViewController {
self.boundComponentId = boundViewController.layoutInfo.componentId;
_boundViewController = boundViewController;
}
Expand Down
1 change: 0 additions & 1 deletion lib/ios/RNNCommandsHandler.m
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ - (void)mergeOptions:(NSString*)componentId options:(NSDictionary*)mergeOptions
[CATransaction begin];
[CATransaction setCompletionBlock:completion];

[vc overrideOptions:newOptions];
[vc mergeOptions:newOptions];

[CATransaction commit];
Expand Down
2 changes: 2 additions & 0 deletions lib/ios/RNNComponentOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,6 @@
@property (nonatomic, strong) Text* alignment;
@property (nonatomic, strong) Bool* waitForRender;

- (BOOL)hasValue;

@end
4 changes: 4 additions & 0 deletions lib/ios/RNNComponentOptions.m
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,8 @@ - (instancetype)initWithDict:(NSDictionary *)dict {
return self;
}

- (BOOL)hasValue {
return _name.hasValue;
}

@end
18 changes: 11 additions & 7 deletions lib/ios/RNNComponentPresenter.m
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ - (instancetype)initWithComponentRegistry:(RNNReactComponentRegistry *)component
return self;
}

- (void)bindViewController:(UIViewController<RNNLayoutProtocol> *)bindedViewController {
[super bindViewController:bindedViewController];
- (void)boundViewController:(UIViewController *)boundViewController {
[super boundViewController:boundViewController];
_navigationButtons = [[RNNNavigationButtons alloc] initWithViewController:self.boundViewController componentRegistry:_componentRegistry];
}

Expand Down Expand Up @@ -80,9 +80,9 @@ - (void)applyOptionsOnInit:(RNNNavigationOptions *)options {
- (void)mergeOptions:(RNNNavigationOptions *)options resolvedOptions:(RNNNavigationOptions *)currentOptions {
[super mergeOptions:options resolvedOptions:currentOptions];
RNNNavigationOptions * withDefault = (RNNNavigationOptions *) [[currentOptions overrideOptions:options] withDefault:[self defaultOptions]];

UIViewController* viewController = self.boundViewController;

[self removeTitleComponentIfNeeded:options];

if (options.backgroundImage.hasValue) {
[viewController setBackgroundImage:options.backgroundImage.get];
}
Expand Down Expand Up @@ -155,9 +155,6 @@ - (void)mergeOptions:(RNNNavigationOptions *)options resolvedOptions:(RNNNavigat

if (options.topBar.title.component.name.hasValue) {
[self setCustomNavigationTitleView:options perform:nil];
} else {
[_customTitleView removeFromSuperview];
_customTitleView = nil;
}

[self setTitleViewWithSubtitle:withDefault];
Expand All @@ -169,6 +166,13 @@ - (void)mergeOptions:(RNNNavigationOptions *)options resolvedOptions:(RNNNavigat
}
}

- (void)removeTitleComponentIfNeeded:(RNNNavigationOptions *)options {
if (options.topBar.title.text.hasValue && !options.topBar.component.hasValue) {
[_customTitleView removeFromSuperview];
_customTitleView = nil;
}
}

- (void)renderComponents:(RNNNavigationOptions *)options perform:(RNNReactViewReadyCompletionBlock)readyBlock {
[self setCustomNavigationTitleView:options perform:readyBlock];
}
Expand Down
2 changes: 1 addition & 1 deletion lib/ios/RNNSideMenuController.m
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ - (instancetype)initWithLayoutInfo:(RNNLayoutInfo *)layoutInfo creator:(id<RNNCo
self = [super initWithCenterViewController:self.center leftDrawerViewController:self.left rightDrawerViewController:self.right];

self.presenter = presenter;
[self.presenter bindViewController:self];
[self.presenter boundViewController:self];

self.defaultOptions = defaultOptions;
self.options = options;
Expand Down
8 changes: 4 additions & 4 deletions lib/ios/ReactNativeNavigation.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@
506A2B1420973DFD00F43A95 /* RNNErrorHandler.h in Headers */ = {isa = PBXBuildFile; fileRef = 506A2B1220973DFD00F43A95 /* RNNErrorHandler.h */; };
506A2B1520973DFD00F43A95 /* RNNErrorHandler.m in Sources */ = {isa = PBXBuildFile; fileRef = 506A2B1320973DFD00F43A95 /* RNNErrorHandler.m */; };
506F630D216A599300AD0D0A /* RNNTabBarControllerTest.m in Sources */ = {isa = PBXBuildFile; fileRef = 506F630C216A599300AD0D0A /* RNNTabBarControllerTest.m */; };
506F630F216A5AD700AD0D0A /* RNNViewControllerPresenterTest.m in Sources */ = {isa = PBXBuildFile; fileRef = 506F630E216A5AD700AD0D0A /* RNNViewControllerPresenterTest.m */; };
506F630F216A5AD700AD0D0A /* RNNComponentPresenterTest.m in Sources */ = {isa = PBXBuildFile; fileRef = 506F630E216A5AD700AD0D0A /* RNNComponentPresenterTest.m */; };
50706E6D20CE7CA5003345C3 /* UIImage+tint.h in Headers */ = {isa = PBXBuildFile; fileRef = 50706E6B20CE7CA5003345C3 /* UIImage+tint.h */; };
50706E6E20CE7CA5003345C3 /* UIImage+tint.m in Sources */ = {isa = PBXBuildFile; fileRef = 50706E6C20CE7CA5003345C3 /* UIImage+tint.m */; };
507E7D57201DDD3000444E6C /* RNNSharedElementAnimationOptions.h in Headers */ = {isa = PBXBuildFile; fileRef = 507E7D55201DDD3000444E6C /* RNNSharedElementAnimationOptions.h */; };
Expand Down Expand Up @@ -531,7 +531,7 @@
506A2B1220973DFD00F43A95 /* RNNErrorHandler.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = RNNErrorHandler.h; sourceTree = "<group>"; };
506A2B1320973DFD00F43A95 /* RNNErrorHandler.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = RNNErrorHandler.m; sourceTree = "<group>"; };
506F630C216A599300AD0D0A /* RNNTabBarControllerTest.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = RNNTabBarControllerTest.m; sourceTree = "<group>"; };
506F630E216A5AD700AD0D0A /* RNNViewControllerPresenterTest.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = RNNViewControllerPresenterTest.m; sourceTree = "<group>"; };
506F630E216A5AD700AD0D0A /* RNNComponentPresenterTest.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = RNNComponentPresenterTest.m; sourceTree = "<group>"; };
50706E6B20CE7CA5003345C3 /* UIImage+tint.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = "UIImage+tint.h"; sourceTree = "<group>"; };
50706E6C20CE7CA5003345C3 /* UIImage+tint.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = "UIImage+tint.m"; sourceTree = "<group>"; };
507E7D55201DDD3000444E6C /* RNNSharedElementAnimationOptions.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = RNNSharedElementAnimationOptions.h; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1054,7 +1054,7 @@
504753772109C13C00FFFBE6 /* RNNOverlayManagerTest.m */,
505EDD31214E4BE80071C7DE /* RNNNavigationControllerTest.m */,
506F630C216A599300AD0D0A /* RNNTabBarControllerTest.m */,
506F630E216A5AD700AD0D0A /* RNNViewControllerPresenterTest.m */,
506F630E216A5AD700AD0D0A /* RNNComponentPresenterTest.m */,
509B258E2178BE7A00C83C23 /* RNNStackPresenterTest.m */,
502F0E172179C39900367CC3 /* RNNTabBarPresenterTest.m */,
50CE8502217C6C9B00084EBF /* RNNSideMenuPresenterTest.m */,
Expand Down Expand Up @@ -1474,7 +1474,7 @@
502F0E162178D09600367CC3 /* RNNBasePresenterTest.m in Sources */,
504753782109C13C00FFFBE6 /* RNNOverlayManagerTest.m in Sources */,
502F0E182179C39900367CC3 /* RNNTabBarPresenterTest.m in Sources */,
506F630F216A5AD700AD0D0A /* RNNViewControllerPresenterTest.m in Sources */,
506F630F216A5AD700AD0D0A /* RNNComponentPresenterTest.m in Sources */,
505963F722676A0000EBB63C /* RNNLayoutManagerTest.m in Sources */,
7B49FECE1E95098500DEB3EA /* RNNCommandsHandlerTest.m in Sources */,
E5F6C3AB22DB4D0F0093C2CE /* UIColor+RNNUtils.m in Sources */,
Expand Down
6 changes: 3 additions & 3 deletions lib/ios/ReactNativeNavigationTests/RNNBasePresenterTest.m
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ - (void)setUp {
self.uut = [[RNNBasePresenter alloc] init];
self.boundViewController = [RNNComponentViewController new];
self.mockBoundViewController = [OCMockObject partialMockForObject:self.boundViewController];
[self.uut bindViewController:self.mockBoundViewController];
[self.uut boundViewController:self.mockBoundViewController];
self.options = [[RNNNavigationOptions alloc] initEmptyOptions];
}

Expand All @@ -45,15 +45,15 @@ - (void)testApplyOptions_shouldSetTabBarItemBadgeWithValue {
}

- (void)testApplyOptions_setTabBarItemBadgeShouldNotCalledOnUITabBarController {
[self.uut bindViewController:self.mockBoundViewController];
[self.uut boundViewController:self.mockBoundViewController];
self.options.bottomTab.badge = [[Text alloc] initWithValue:@"badge"];
[[self.mockBoundViewController reject] setTabBarItemBadge:[[RNNBottomTabOptions alloc] initWithDict:@{@"badge": @"badge"}]];
[self.uut applyOptions:self.options];
[self.mockBoundViewController verify];
}

- (void)testApplyOptions_setTabBarItemBadgeShouldWhenNoValue {
[self.uut bindViewController:self.mockBoundViewController];
[self.uut boundViewController:self.mockBoundViewController];
self.options.bottomTab.badge = nil;
[[self.mockBoundViewController reject] setTabBarItemBadge:[OCMArg any]];
[self.uut applyOptions:self.options];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@
#import "RNNComponentPresenter.h"
#import "UIViewController+RNNOptions.h"
#import "RNNComponentViewController.h"
#import "UIViewController+LayoutProtocol.h"
#import "RNNTitleViewHelper.h"

@interface RNNViewControllerPresenterTest : XCTestCase
@interface RNNComponentPresenterTest : XCTestCase

@property (nonatomic, strong) RNNComponentPresenter *uut;
@property (nonatomic, strong) RNNNavigationOptions *options;
Expand All @@ -13,14 +15,14 @@ @interface RNNViewControllerPresenterTest : XCTestCase

@end

@implementation RNNViewControllerPresenterTest
@implementation RNNComponentPresenterTest

- (void)setUp {
[super setUp];
self.componentRegistry = [OCMockObject partialMockForObject:[RNNReactComponentRegistry new]];
self.uut = [[RNNComponentPresenter alloc] initWithComponentRegistry:self.componentRegistry:[[RNNNavigationOptions alloc] initEmptyOptions]];
self.boundViewController = [OCMockObject partialMockForObject:[RNNComponentViewController new]];
[self.uut bindViewController:self.boundViewController];
[self.uut boundViewController:self.boundViewController];
self.options = [[RNNNavigationOptions alloc] initEmptyOptions];
}

Expand Down Expand Up @@ -71,7 +73,7 @@ - (void)testApplyOptions_setOverlayTouchOutsideIfHasValue {

- (void)testBindViewControllerShouldCreateNavigationButtonsCreator {
RNNComponentPresenter* presenter = [[RNNComponentPresenter alloc] init];
[presenter bindViewController:self.boundViewController];
[presenter boundViewController:self.boundViewController];
XCTAssertNotNil(presenter.navigationButtons);
}

Expand Down Expand Up @@ -136,7 +138,7 @@ -(void)testApplyOptionsOnInit_BottomTabsDrawUnder_false {
- (void)testReactViewShouldBeReleasedOnDealloc {
RNNComponentViewController* bindViewController = [RNNComponentViewController new];
bindViewController.layoutInfo = [self createLayoutInfoWithComponentId:@"componentId"];
[self.uut bindViewController:bindViewController];
[self.uut boundViewController:bindViewController];

self.options.topBar.title.component = [[RNNComponentOptions alloc] initWithDict:@{@"name": @"componentName"}];

Expand All @@ -145,22 +147,21 @@ - (void)testReactViewShouldBeReleasedOnDealloc {
[(id)self.componentRegistry verify];
}

- (void)testBindViewControllerShouldSetBindedComponentId {
- (void)testBindViewControllerShouldSetBoundComponentId {
RNNComponentViewController* bindViewController = [RNNComponentViewController new];
RNNLayoutInfo* layoutInfo = [[RNNLayoutInfo alloc] init];
layoutInfo.componentId = @"componentId";
bindViewController.layoutInfo = layoutInfo;
[self.uut bindViewController:bindViewController];

[self.uut boundViewController:bindViewController];
XCTAssertEqual(self.uut.boundComponentId, @"componentId");
}

- (void)testRenderComponentsCreateReactViewWithBindedComponentId {
RNNComponentViewController* bindedViewController = [RNNComponentViewController new];
- (void)testRenderComponentsCreateReactViewWithBoundComponentId {
RNNComponentViewController* boundViewController = [RNNComponentViewController new];
RNNLayoutInfo* layoutInfo = [self createLayoutInfoWithComponentId:@"componentId"];
bindedViewController.layoutInfo = layoutInfo;

[self.uut bindViewController:bindedViewController];
boundViewController.layoutInfo = layoutInfo;
[self.uut boundViewController:boundViewController];

self.options.topBar.title.component = [[RNNComponentOptions alloc] initWithDict:@{@"name": @"titleComponent"}];

Expand All @@ -172,15 +173,15 @@ - (void)testRenderComponentsCreateReactViewWithBindedComponentId {
XCTAssertEqual(self.uut.boundComponentId, @"componentId");
}

- (void)testApplyOptionsOnWillMoveToParent_shouldSetBackButtonOnBindedViewController_withTitle {
- (void)testApplyOptionsOnWillMoveToParent_shouldSetBackButtonOnBoundViewController_withTitle {
Text* title = [[Text alloc] initWithValue:@"Title"];
self.options.topBar.backButton.title = title;
[[(id) self.boundViewController expect] setBackButtonIcon:nil withColor:nil title:title.get];
[self.uut applyOptionsOnWillMoveToParentViewController:self.options];
[(id)self.boundViewController verify];
}

- (void)testApplyOptionsOnWillMoveToParent_shouldSetBackButtonOnBindedViewController_withHideTitle {
- (void)testApplyOptionsOnWillMoveToParent_shouldSetBackButtonOnBoundViewController_withHideTitle {
Text* title = [[Text alloc] initWithValue:@"Title"];
self.options.topBar.backButton.title = title;
self.options.topBar.backButton.showTitle = [[Bool alloc] initWithValue:@(0)];
Expand All @@ -189,24 +190,64 @@ - (void)testApplyOptionsOnWillMoveToParent_shouldSetBackButtonOnBindedViewContro
[(id)self.boundViewController verify];
}

- (void)testApplyOptionsOnWillMoveToParent_shouldSetBackButtonOnBindedViewController_withIcon {
- (void)testApplyOptionsOnWillMoveToParent_shouldSetBackButtonOnBoundViewController_withIcon {
Image* image = [[Image alloc] initWithValue:[UIImage new]];
self.options.topBar.backButton.icon = image;
[[(id) self.boundViewController expect] setBackButtonIcon:image.get withColor:nil title:nil];
[self.uut applyOptionsOnWillMoveToParentViewController:self.options];
[(id)self.boundViewController verify];
}

- (void)testApplyOptionsOnWillMoveToParent_shouldSetBackButtonOnBindedViewController_withDefaultValues {
- (void)testApplyOptionsOnWillMoveToParent_shouldSetBackButtonOnBoundViewController_withDefaultValues {
[[(id) self.boundViewController expect] setBackButtonIcon:nil withColor:nil title:nil];
[self.uut applyOptionsOnWillMoveToParentViewController:self.options];
[(id)self.boundViewController verify];
}

- (void)testRemoveTitleComponentIfNeeded_componentIsRemovedIfTitleTextIsDefined {
id mockTitle = [OCMockObject niceMockForClass:[RNNReactView class]];
OCMStub([self.componentRegistry createComponentIfNotExists:[OCMArg any] parentComponentId:[OCMArg any] reactViewReadyBlock:nil]).andReturn(mockTitle);

RNNComponentOptions* component = [RNNComponentOptions new];
component.name = [[Text alloc] initWithValue:@"componentName"];
component.componentId = [[Text alloc] initWithValue:@"someId"];
_options.topBar.title.component = component;

[self.uut mergeOptions:_options resolvedOptions:[[RNNNavigationOptions alloc] initEmptyOptions]];
XCTAssertNotNil(self.boundViewController.navigationItem.titleView);
XCTAssertEqual(self.boundViewController.navigationItem.titleView, mockTitle);

[[mockTitle expect] removeFromSuperview];
_options = [[RNNNavigationOptions alloc] initEmptyOptions];
_options.topBar.title.text = [[Text alloc] initWithValue:@""];
[self.uut mergeOptions:_options resolvedOptions:[[RNNNavigationOptions alloc] initEmptyOptions]];
XCTAssertNotEqual(self.boundViewController.navigationItem.titleView, mockTitle);
[mockTitle verify];
}

- (void)testRemoveTitleComponentIfNeeded_componentIsNotRemovedIfMergeOptionsIsCalledWithoutTitleText {
id mockTitle = [OCMockObject niceMockForClass:[RNNReactView class]];
OCMStub([self.componentRegistry createComponentIfNotExists:[OCMArg any] parentComponentId:[OCMArg any] reactViewReadyBlock:nil]).andReturn(mockTitle);

RNNComponentOptions* component = [RNNComponentOptions new];
component.name = [[Text alloc] initWithValue:@"componentName"];
component.componentId = [[Text alloc] initWithValue:@"someId"];
_options.topBar.title.component = component;

[self.uut mergeOptions:_options resolvedOptions:[[RNNNavigationOptions alloc] initEmptyOptions]];
XCTAssertNotNil(self.boundViewController.navigationItem.titleView);
XCTAssertEqual(self.boundViewController.navigationItem.titleView, mockTitle);


_options = [[RNNNavigationOptions alloc] initEmptyOptions];
_options.bottomTabs.visible = [[Bool alloc] initWithBOOL:NO];
[self.uut mergeOptions:_options resolvedOptions:[[RNNNavigationOptions alloc] initEmptyOptions]];
XCTAssertEqual(self.boundViewController.navigationItem.titleView, mockTitle);
}

- (RNNLayoutInfo *)createLayoutInfoWithComponentId:(NSString *)componentId {
RNNLayoutInfo* layoutInfo = [[RNNLayoutInfo alloc] init];
layoutInfo.componentId = @"componentId";
layoutInfo.componentId = componentId;
return layoutInfo;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ - (void)setUp {
[super setUp];
self.uut = [[RNNSideMenuPresenter alloc] init];
self.bindedViewController = [OCMockObject partialMockForObject:[RNNSideMenuController new]];
[self.uut bindViewController:self.bindedViewController];
[self.uut boundViewController:self.bindedViewController];
self.options = [[RNNNavigationOptions alloc] initEmptyOptions];
}

Expand Down
2 changes: 1 addition & 1 deletion lib/ios/ReactNativeNavigationTests/RNNStackPresenterTest.m
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ - (void)setUp {
[super setUp];
self.uut = [[RNNStackPresenter alloc] init];
self.boundViewController = [OCMockObject partialMockForObject:[RNNStackController new]];
[self.uut bindViewController:self.boundViewController];
[self.uut boundViewController:self.boundViewController];
self.options = [[RNNNavigationOptions alloc] initEmptyOptions];
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ - (void)setUp {
[super setUp];
self.uut = [OCMockObject partialMockForObject:[RNNBottomTabsPresenter new]];
self.boundViewController = [OCMockObject partialMockForObject:[RNNBottomTabsController new]];
[self.uut bindViewController:self.boundViewController];
[self.uut boundViewController:self.boundViewController];
self.options = [[RNNNavigationOptions alloc] initEmptyOptions];
}

Expand Down
2 changes: 1 addition & 1 deletion lib/ios/UIViewController+LayoutProtocol.m
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ - (instancetype)initWithLayoutInfo:(RNNLayoutInfo *)layoutInfo
[self performSelector:@selector(setViewControllers:) withObject:childViewControllers];
}
self.presenter = presenter;
[self.presenter bindViewController:self];
[self.presenter boundViewController:self];
[self.presenter applyOptionsOnInit:self.resolveOptions];

return self;
Expand Down

0 comments on commit 6d446a8

Please sign in to comment.