search

infinitered / ProMotion-menu

RubyMotion gem allowing you to easily setup a facebook or Path style hidden slide menu easily with the ProMotion gem.
74 stars 29 forks source link

Feature right or left side menu #7

Closed ryanlntn closed 11 years ago

ryanlntn commented 11 years ago

Hi Matt. I work for @jamonholmgren and I've been working on an app that required a right side slide menu. Thinking that this would be a common scenario and taking inspiration from ProMotion's set_nav_bar_button I modified open_slide_menu to take :left or :right as it's first argument. I updated the test suite and README accordingly.

jamonholmgren commented 11 years ago

Maybe (so this change doesn't break the legacy API) the :left should be the third (optional) argument.

ryanlntn commented 11 years ago

I personally like the consistency with set_nav_bar_button but another thing I was considering is that PKRevealController allows you to optionally set both a rightViewController and leftViewController so maybe something like

open_slide_menu(ContentScreen.new(nav_bar: true), left: LeftMenuScreen, right: RightMenuScreen)

would be an even better option.

jamonholmgren commented 11 years ago

That would be best, I agree. @macfanatic, your thoughts?

macfanatic commented 11 years ago

@ryanlntn & @jamonholmgren - I like the :left & :right options better, since it does allow you to use the underlying cocoapod more thoroughly.

jamonholmgren commented 11 years ago

@ryanlntn , you'll also want to update the README in your branch reflecting these changes.

ryanlntn commented 11 years ago

I can't seem to get Travis working. The tests are passing though.

ProMotionSlideMenu::AppDelegate
  - should have a 'slide_menu' attribute
  - should not have a slide menu by default
  - should respond to 'open_slide_menu'
  - #open_slide_menu should return a SlideMenuScreen
  - should have a SlideMenuScreen as the rootViewController

ProMotionSlideMenu::SlideMenuScreen
  - should return an instance of SlideMenuScreen
  - should store menu & content controllers
  - should allow you to pass class instances
  - should present the menu controller when requested
  - should present the content controller when requested
  - should let me wrap the content controller in a UINavigationController
  - should let me set the title on the content controller during creation
  - should accept a plain UIViewController
  - should allow you to set both a right and left menu controller

14 specifications (19 requirements), 0 failures, 0 errors
ryanlntn commented 11 years ago

Hey @macfanatic. Any feedback?

macfanatic commented 11 years ago

Sorry @ryanlntn, slammed here. This is on my list to review & get moving, b/c I love the feature for sure.

macfanatic commented 11 years ago

@ryanlntn This code looks great, few things other than my couple comments on the code:

  1. Updated master with some fixes for Travis CI, so strip that out please.
  2. Believe your branch fails on Travis CI with the "[!] Unable to find a specification for PKRevealController (= 1.0b2)" b/c you're using a newer version of cocoapods than I am locally.
  3. Can we get all this rebased to 1 or 2 commits, very noisy.

Thanks to you and @jamonholmgren for the contribution, definitely like the work!

ryanlntn commented 11 years ago

Thanks @macfanatic! I made your requested changes, stripped out the Travis CI commits and rebased the rest. Let me know if there's anything else. :smile:

macfanatic commented 11 years ago

Awesome @ryanlntn, I'm going to merge this in and continue trying to get Travis working in PR #14 with the latest cocoapods.