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.

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

Storyboard support + storyboard example #33

Closed
wants to merge 2 commits into from

Conversation

TomSwift
Copy link
Contributor

I added basic storyboard support via a category extension on MMDrawerController. No changes were required to the core MMDrawerController itself. I also included a new example to showcase this new capability.

Apple doesn't support programatic "embed" segue outlet definition, which would be desirable. Because of this we have to use custom segue's, and this places one extra work item on the user - they must set a segue identifier for each "center", "left", and "right" segue connector.

This could be considered a "hack", but I think it addresses the current desire by some developers to be able to define their interface 100% in a storyboard.

I'm happy to address any issues or desired changes.

TomSwift added 2 commits May 28, 2013 13:30
Storyboard support via custom segue and category extension.  No changes
to core MMDrawerController required.  Added new example project to
demonstrate capability.
@larsacus
Copy link
Contributor

Yeah that's definitely not what I meant by "close". This is interesting, we'll have to take a look.

@ghost
Copy link

ghost commented May 28, 2013

While I was excited to get this e-mail, I've dug too deep into programmatically coding my app. In any case i will have a look at this :D

@kcharwood
Copy link
Contributor

I wonder if this would be better off as an extension, rather than a first party member of the library... I would rather try to offload that to @TomSwift rather than bring it in the main library since it looks like there isn't a publicly supported way to support relationships in a storyboard for custom container view controllers.

Also, since we don't use story boards, it keeps it out of the core...

@TomSwift would you be willing to maintain it in your own namespace and releasing a podspec for it?

@TomSwift
Copy link
Contributor Author

Well, I guess I could do that. But I explicitly did this in a category extension so that it wouldn't impact your core (you don't need to include the category module in your target if you don't want the pollution). (An improved version would require a change to your core; specifically by exposing an init method that doesn't require the centerViewController to be supplied up front.)

There's really nothing wrong with supporting Storyboards in this manner. The technique isn't ideal but it's not depending on any undocumented behaviors.

I'd also ague that discoverability will suffer. Folks who want to use MMDrawerController with their storyboard will have to somehow discover my repository/podspec.

If/when Apple supports embed-relationships for custom containers (they actually do, but not programmatically) then the core could adopt that support making this extension moot - unless the developer is targeting a lesser SDK version. In this case the extension could ultimately it could be deprecated from the library.

@kcharwood
Copy link
Contributor

@TomSwift You actually listed several reasons to include it as an extension :)

  • Hopefully the core is designed in a way that allows developers to create extensions without modifying the core, which is what you have done here.
  • You aren't using any undocumented methods, but at the same time Apple has not exposed this functionality officially for custom view controller containers, so it's viewed as a hack to try and get it close to a UINavigationController or UITabBarController. I think this is the biggest reason why I'd like to leave it out of the core.
  • If Apple ever does provide explicit support for this, we can bring that functionality directly into the core without worring about deprecation.

In terms of discoverability, I have no problem creating a section in the readme directing folks to your podspec/repo for that support. CocoaPods makes it EASY to bring it in, so I'm not worried about that.

I'm leaning towards declaring Storyboards aren't officially supported due to the lack of available API from Apple, but direct them to your podspec for a workaround in the mean time.

Thoughts?

@TomSwift
Copy link
Contributor Author

Okay fine. I'll create a podspec and post my extension as a separate project.

Apple does have support in Storyboards for view controller containment via "embed segues". You could leverage this by exposing the centerContainerView as an IBOutlet, and creating new leftContainerView and rightContainerView IBOutlet properties. In this manner, I believe you'd be able to have "native" storyboard support. To use, the developer would have to drop a UIViewController on the Storyboard and set its class to MMDrawerController. Then they'd drop 3 container views on the controller and hook up the left/center/right outlets. Then they'd drag Embed Segue lines from each container to their left/center/right controllers. The core would have to add an initWithCoder: method to complete initialization and some mechanism to sus out the left/center/right view controllers from the 'childViewControllers' property (could be done by comparing each child controller view's superview).

This would use Apple's Storyboard support for view controller containment as intended. It differs from "native container controllers" (i.e. UITabController and UINavigationController) in that those use "relationship segues" (vs. "embed segues"), which are currently unavailable to us.

Whether this is better than my current extension is arguable. It's a little more work for the user/developer, but it makes use of "embed segues" as intended vs overloading the meaning of standard "storyboard segues".

If you like this approach better and think you'd integrate it into the core then I'd be happy to get it working and make a new pull request. Otherwise we'll just do the separate extension that I already created.

For my own purposes I'm happy with what I've got. I'd only explore this new route if you thought you'd integrate it in core.

@kcharwood
Copy link
Contributor

Could you point me to some documentation from Apple where it says that is the way to do it for custom container view controllers? I'd like to give it a read through to make sure I understand that.

Also, bare with me on this issue. You are talking to someone who preaches "No Storyboard or Nibs!!!!", so I'm a little hard headed when it comes to this topic 👴

@TomSwift
Copy link
Contributor Author

I understand where you're coming from - I've taken your stance before and still do on certain projects. Especially if you're on a team, Storyboards can be problematic. But they (and nibs) can make layout tasks and maintenance simpler esp. for things like table view cells. Contrary to expectations, they are often more efficient than hand-coded UI. And...people use them. 'Nuff said on that.

Embed segues are not well documented.

From the XCode 4.5 release notes:

Editing User Interfaces: Storyboards
Storyboards now support view controller containment. You can add child view controllers to a parent view controller in a storyboard. At runtime, when the viewDidLoad method is called on the parent controller, its view hierarchy (composed of the view hierarchies of its child controllers) is already loaded. 9630246
To add a view controller as the child of another view controller:

Add a container view from the Object library.
Connect the container view to the child view controller with an embed segue.

And, from WWDC 2012: https://developer.apple.com/videos/wwdc/2012/?id=407

@kcharwood
Copy link
Contributor

It looks like Embed Segues are iOS 6 only, right?

@TomSwift
Copy link
Contributor Author

No, I don't think so. You need Xcode 4.5 or better for the Storyboard editor support, but as long as your target supports view controller containment (which is iOS 5 or better) I think you're good to go.

@kcharwood
Copy link
Contributor

I haven't had a chance to listen to the WWDC video yet, but it looks like it is in the "New in iOS 6" section of the slides.

@larsacus
Copy link
Contributor

Watched the storyboard video linked above yesterday. Embedding segues looks like an iOS 6+ feature (see 32:40 in video). I'm going to take a closer look at storyboards today to learn some more.

@TomSwift
Copy link
Contributor Author

You are correct. I made a test app using embed segue's and it runs in iOS6 but not in iOS5.1. So this approach would be 6.x only.

And here's some confirmation: http://stackoverflow.com/questions/12868255/nsinvalidunarchiveoperationexception-with-containerview

@cytryn
Copy link

cytryn commented Jun 4, 2013

Wow thats exactly what I am looking for... It is so much easier to get things done on IB... mocking screens are just ready.... but I not gonna start a discussion about this here.

Any updates on this? I want to see if this is worth better than the JASidePanels... but I need it to work with storyboards.

@kcharwood
Copy link
Contributor

@cytryn I'm planning on talking with some Apple peeps at WWDC to find out the 'best' way to do this for a custom container view controller.

It looks like Embed Segues are currently the 'best' option, but they are iOS 6 only so that would need to go into an extension to allow the core to continue to work with iOS 5.

I asked @TomSwift to see if he could roll his solution into an extension for the time being, until I can get more clarity on how this should be supported in the core going forward.

@rtamesis
Copy link

I am building an app that initially used JASidePanels with its unconnected storyboards, but I switched over to MMDrawerController after I discovered Tom Swift's categories for using storyboards and segues.

@ArtFeel
Copy link

ArtFeel commented Jun 18, 2013

Maybe it is better to create subspec for this patch?

@kcharwood
Copy link
Contributor

Hey @ArtFeel,

I'd like to see it for now an official extension, and leave it out of the subspec. Revving a subspec requires revving the whole podspec, to I would like to limit that if possible.

@TomSwift
Copy link
Contributor Author

@kcharwood - what did the Apple peeps say at WWDC? To me it looks like there's no new support in XCode5/iOS7 for embed or relationship segues...

@kcharwood
Copy link
Contributor

There is not. Several made the comment that they don't use storyboards :)

I feel like the best way would be Embed Segues, and since they are iOS 6 only, it should live as an external dependent podspec.

@kcharwood
Copy link
Contributor

@TomSwift Did you ever make any progress on this? Have you published an extension for this? I'd love to link to it.

@jonasman
Copy link

I like Tom's approach. IB is used by everyone and makes things easier to see.
I wouldnt mind to include only the initWithCoder to the core which is the key to the storyboard support. The segues would also be cool.

@TomSwift
Copy link
Contributor Author

@kcharwood, @jonasman -

No, I haven't yet posted a separate repository for this. Sorry.

Mainly I think that my MMDrawerController+Storyboard is a bit fragile on its own. Because it depends on a copy/paste of code from the initWithCenterViewController:leftDrawerController:rightDrawerController, and on your non-public method -setupGestureRecognizers. If you change any of that then the extension will likely break.

In fact, having just looked, it seems you HAVE changed the init sequence, and now the extension IS broke. Easy to fix, but this is what my concern is about.

As @jonasman suggests, if you implemented initWithCoder: in the core then all that is required of MMDrawerController+Storyboard is to implement awakeFromNib to trigger the segues.

@kcharwood
Copy link
Contributor

I'd be willing to take a Pull Request that just implements initWithCoder: Feel free to submit one.

@jonasman
Copy link

@TomSwift since you dont have issues enable in your repo i write it here.

In your prepare to segue your assert is breaking apps that want to segue to other VCs from the Drawer.
I suggest to change it to this:

-(void)prepareForSegue:(UIStoryboardSegue *)segue sender:(id)sender
{


if ( [segue.identifier isEqualToString: @"mm_center"] )
{
    NSParameterAssert( [segue isKindOfClass: [MMDrawerControllerSegue class]]);
    [self setCenterViewController: segue.destinationViewController];
}
else
if ( [segue.identifier isEqualToString: @"mm_left"] )
{
    NSParameterAssert( [segue isKindOfClass: [MMDrawerControllerSegue class]]);
    [self setLeftDrawerViewController: segue.destinationViewController];
}
else 
if ( [segue.identifier isEqualToString: @"mm_right"] )
{
    NSParameterAssert( [segue isKindOfClass: [MMDrawerControllerSegue class]]);
    [self setRightDrawerViewController: segue.destinationViewController];
}
}  

@kcharwood
Copy link
Contributor

@TomSwift is there anything else here that needs to be done for now? Have you podspec'd your solution? I'd love to drop it in the readme.

@TomSwift TomSwift closed this Aug 13, 2013
@TomSwift
Copy link
Contributor Author

Okay, I finally got around to doing this.

I submitted a pull request for adding initWithCoder: support into the core library.

And I created a github repo for my MMDrawerController+Storyboard category, with a podspec.

I haven't submitted the podspec yet; I'd prefer to do that once the initWithCoder: pull is accepted and a new version tagged. With the version tag I can make my podspec reference a version dependency on MMDrawerController to the version that has initWithCoder.

My repo is here: https://github.com/TomSwift/MMDrawerController-Storyboard

@averydev
Copy link

averydev commented Nov 1, 2013

@TomSwift thanks for your work on this. Any progress on getting this included or added to the core, or as a separate pod?

@larsacus
Copy link
Contributor

larsacus commented Nov 1, 2013

It looks like the pod was merged into the cocoa pods spec repo a couple of weeks ago: http://cocoapods.org/?q=MMDrawer

@TomSwift
Copy link
Contributor Author

TomSwift commented Nov 1, 2013

correct. My first podspec!

@ajsharp
Copy link

ajsharp commented Jan 8, 2014

I'd also ague that discoverability will suffer. Folks who want to use MMDrawerController with their storyboard will have to somehow discover my repository/podspec.

This was / is certainly my case. It's not clear whether MMDrawerController supports storyboards on it's own (guessing it doesn't considering @kcharwood's stance) thus making the whole ordeal pretty confusing.

So, here's a vote to have this library support storyboards. It seems clear the community is continuing to move in that direction. A hard stance of "no storyboards" seems less than ideal.

@cliftonlabrum
Copy link

MMDrawerController looks really nice. It's a bit of a let-down to discover this thread and the lack of built-in Storyboard support. I'll add my vote here for integrating the Storyboard goods by default. :)

@KanybekMomukeyev
Copy link

Did something like this, and its working.

UIStoryboard* mainStoryboard = [UIStoryboard storyboardWithName:@"Main" bundle: nil];
UITableViewController* menuController = [mainStoryboard instantiateViewControllerWithIdentifier:@"OFMenuTVC"];
    UIViewController* orderedItemsVC = [mainStoryboard instantiateViewControllerWithIdentifier:@"OFOrderedItemsVC"];
    UINavigationController* navigationController = (UINavigationController *) self.window.rootViewController;


    self.drawerController = [[MMDrawerController alloc] initWithCenterViewController:navigationController
                                                          leftDrawerViewController:menuController
                                                         rightDrawerViewController:orderedItemsVC];
    self.window.rootViewController = self.drawerController;

@vincilbishop
Copy link

Thanks @KanybekMomukeyev, that works great with storyboard. No extension needed :)

@mrtristan
Copy link

for those of you who want to implement something like what @KanybekMomukeyev posted here for your initial UIViewController in a storyboard, have a look here:
http://stackoverflow.com/a/14926009/1301349

to be clear, this doesn't require the category by @TomSwift mentioned above. not sure which way is "better"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.