Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added shouldDismissPresentedViewController property to options #33

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Added shouldDismissPresentedViewController property to options #33

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
612a661
added shouldDismissPresentedViewController property to options
tspecht Apr 1, 2015
391197c
fixed presentation target
tspecht Apr 1, 2015
213ef9f
implemented PR comments from @clayallsopp
tspecht Apr 1, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions Routable/Routable.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,21 @@ typedef void (^RouterOpenCallback)(NSDictionary *params);
defaultParams: (NSDictionary *)defaultParams
isRoot: (BOOL)isRoot
isModal: (BOOL)isModal;
/**
@return A new instance of `UPRouterOptions` with its properties explicitly set
@param presentationStyle The `UIModalPresentationStyle` attached to the mapped `UIViewController`
@param transitionStyle The `UIModalTransitionStyle` attached to the mapped `UIViewController`
@param defaultParams The default parameters which are passed when opening the URL
@param isRoot The boolean `shouldOpenAsRootViewController` property is set to
@param isModal The boolean that sets a modal presentation format
@param shouldDismissPresentedViewController The boolean indicating whether an already existing presentedViewController should be dismissed before presenting a new one
*/
+ (instancetype)routerOptionsWithPresentationStyle: (UIModalPresentationStyle)presentationStyle
transitionStyle: (UIModalTransitionStyle)transitionStyle
defaultParams: (NSDictionary *)defaultParams
isRoot: (BOOL)isRoot
isModal: (BOOL)isModal
shouldDismissPresentedViewController: (BOOL)shouldDismissPresentedViewController;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we make another method, keeping the original signature (without the shouldDismissPresentedViewController arg), and have it propagate the default YES value to this new constructor?

just for backwards-compat

/**
@return A new instance of `UPRouterOptions` with its properties set to default
*/
Expand Down Expand Up @@ -108,6 +123,10 @@ typedef void (^RouterOpenCallback)(NSDictionary *params);
@return A new instance of `UPRouterOptions`, setting the `shouldOpenAsRootViewController` property to `YES`
*/
+ (instancetype)routerOptionsAsRoot;
/**
@return A new instance of `UPRouterOptions`, setting the `shouldDismissPresentedViewController` property to `NO`
*/
+ (instancetype)routerOptionsAsKeepPresentedViewController;

//previously supported
/**
Expand Down Expand Up @@ -138,6 +157,11 @@ typedef void (^RouterOpenCallback)(NSDictionary *params);
@return A new instance of `UPRouterOptions`, setting the `shouldOpenAsRootViewController` property to `YES`
*/
+ (instancetype)root;
/**
@remarks not idiomatic objective-c naming for allocation and initialization, see +routerOptionsAsRoot
@return A new instance of `UPRouterOptions`, setting the `shouldDismissPresentedViewController` property to `NO`
*/
+ (instancetype)keepPresentedViewController;

/**
@remarks not idiomatic objective-c naming; overrides getter to wrap around setter
Expand Down Expand Up @@ -167,6 +191,11 @@ typedef void (^RouterOpenCallback)(NSDictionary *params);
@return A new instance of `UPRouterOptions`, setting the `shouldOpenAsRootViewController` property to `YES`
*/
- (UPRouterOptions *)root;
/**
@remarks not idiomatic objective-c naming; wraps around setter
@return A new instance of `UPRouterOptions`, setting the `shouldDismissPresentedViewController` property to `NO`
*/
- (UPRouterOptions *)keepPresentedViewController;

///-------------------------------
/// @name Properties
Expand All @@ -192,6 +221,10 @@ typedef void (^RouterOpenCallback)(NSDictionary *params);
The property determining if the mapped `UIViewController` instance should be set as the root view controller of the router's `UINavigationController` instance.
*/
@property (readwrite, nonatomic, assign) BOOL shouldOpenAsRootViewController;
/**
The property determining if a possibly already presented view controller should be dismissed before presenting the new one. Defaults to 'YES'.
*/
@property (readwrite, nonatomic, assign) BOOL shouldDismissPresentedViewController;

@end

Expand Down
66 changes: 55 additions & 11 deletions Routable/Routable.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,18 +78,35 @@ @interface UPRouterOptions ()

@implementation UPRouterOptions

//Backwards compatibility
+ (instancetype)routerOptionsWithPresentationStyle: (UIModalPresentationStyle)presentationStyle
transitionStyle: (UIModalTransitionStyle)transitionStyle
defaultParams: (NSDictionary *)defaultParams
isRoot: (BOOL)isRoot
isModal: (BOOL)isModal
{
return [self routerOptionsWithPresentationStyle:presentationStyle
transitionStyle:transitionStyle
defaultParams:defaultParams
isRoot:isRoot
isModal:isModal
shouldDismissPresentedViewController:YES];
}
//Explicit construction
+ (instancetype)routerOptionsWithPresentationStyle: (UIModalPresentationStyle)presentationStyle
transitionStyle: (UIModalTransitionStyle)transitionStyle
defaultParams: (NSDictionary *)defaultParams
isRoot: (BOOL)isRoot
isModal: (BOOL)isModal {
isModal: (BOOL)isModal
shouldDismissPresentedViewController: (BOOL)shouldDismissPresentedViewController
{
UPRouterOptions *options = [[UPRouterOptions alloc] init];
options.presentationStyle = presentationStyle;
options.transitionStyle = transitionStyle;
options.defaultParams = defaultParams;
options.shouldOpenAsRootViewController = isRoot;
options.modal = isModal;
options.shouldDismissPresentedViewController = shouldDismissPresentedViewController;
return options;
}
//Default construction; like [NSArray array]
Expand All @@ -98,7 +115,8 @@ + (instancetype)routerOptions {
transitionStyle:UIModalTransitionStyleCoverVertical
defaultParams:nil
isRoot:NO
isModal:NO];
isModal:NO
shouldDismissPresentedViewController:YES];
}

//Custom class constructors, with heavier Objective-C accent
Expand All @@ -107,35 +125,48 @@ + (instancetype)routerOptionsAsModal {
transitionStyle:UIModalTransitionStyleCoverVertical
defaultParams:nil
isRoot:NO
isModal:YES];
isModal:YES
shouldDismissPresentedViewController:YES];
}
+ (instancetype)routerOptionsWithPresentationStyle:(UIModalPresentationStyle)style {
return [self routerOptionsWithPresentationStyle:style
transitionStyle:UIModalTransitionStyleCoverVertical
defaultParams:nil
isRoot:NO
isModal:NO];
isModal:NO
shouldDismissPresentedViewController:YES];
}
+ (instancetype)routerOptionsWithTransitionStyle:(UIModalTransitionStyle)style {
return [self routerOptionsWithPresentationStyle:UIModalPresentationNone
transitionStyle:style
defaultParams:nil
isRoot:NO
isModal:NO];
isModal:NO
shouldDismissPresentedViewController:YES];
}
+ (instancetype)routerOptionsForDefaultParams:(NSDictionary *)defaultParams {
return [self routerOptionsWithPresentationStyle:UIModalPresentationNone
transitionStyle:UIModalTransitionStyleCoverVertical
defaultParams:defaultParams
isRoot:NO
isModal:NO];
isModal:NO
shouldDismissPresentedViewController:YES];
}
+ (instancetype)routerOptionsAsRoot {
return [self routerOptionsWithPresentationStyle:UIModalPresentationNone
transitionStyle:UIModalTransitionStyleCoverVertical
defaultParams:nil
isRoot:YES
isModal:NO];
isModal:NO
shouldDismissPresentedViewController:YES];
}
+ (instancetype)routerOptionsAsKeepPresentedViewController {
return [self routerOptionsWithPresentationStyle:UIModalPresentationNone
transitionStyle:UIModalTransitionStyleCoverVertical
defaultParams:nil
isRoot:NO
isModal:NO
shouldDismissPresentedViewController:NO];
}

//Exposed methods previously supported
Expand All @@ -154,6 +185,9 @@ + (instancetype)forDefaultParams:(NSDictionary *)defaultParams {
+ (instancetype)root {
return [self routerOptionsAsRoot];
}
+ (instancetype)keepPresentedViewController {
return [self routerOptionsAsKeepPresentedViewController];
}

//Wrappers around setters (to continue DSL-like syntax)
- (UPRouterOptions *)modal {
Expand All @@ -176,6 +210,10 @@ - (UPRouterOptions *)root {
[self setShouldOpenAsRootViewController:YES];
return self;
}
- (UPRouterOptions *)keepPresentedViewController {
[self setShouldDismissPresentedViewController:NO];
return self;
}
@end

@interface UPRouter ()
Expand Down Expand Up @@ -268,15 +306,21 @@ - (void)open:(NSString *)url animated:(BOOL)animated {

UIViewController *controller = [self controllerForRouterParams:params];

if (self.navigationController.presentedViewController) {
if (self.navigationController.presentedViewController && [options shouldDismissPresentedViewController]) {
[self.navigationController dismissViewControllerAnimated:animated completion:nil];
}

if ([options isModal]) {
if ([controller.class isSubclassOfClass:UINavigationController.class]) {
[self.navigationController presentViewController:controller
animated:animated
completion:nil];
UIViewController* controllerToPresentOn = self.navigationController;

if (self.navigationController.presentedViewController) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we use an additional option for this new behavior? like options.shouldPresentOnPresentedViewController

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this check is actually just present to avoid not showing the new controller at all. The problem is that iOS doesn't allow to push an a controller which is already presenting a controller. So if at this point the navigationController still has a presentedViewController, shouldDismissPresentedViewController must have been false so we really want to push the new controller on top of the presentedViewController

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup that can happen, but "shouldDismissPresentedViewController must have been false..." makes an assumption that everyone is using Routable for all their modal controllers which may not be true

I'd rather opt-in to this behavior, and if the user hasn't opt'd in and encounters this case (navigationController.presentedViewController != nil) then log a message but keep the old behavior (where nothing happens)

controllerToPresentOn = self.navigationController.presentedViewController;
}

[controllerToPresentOn presentViewController:controller
animated:animated
completion:nil];
}
else {
UINavigationController *navigationController = [[UINavigationController alloc] initWithRootViewController:controller];
Expand Down