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

Support the situation when VC is pushed by UINavigationController, not adding Done button #3

Merged
merged 2 commits into from
Feb 17, 2016
Merged

Support the situation when VC is pushed by UINavigationController, not adding Done button #3

merged 2 commits into from
Feb 17, 2016

Conversation

innocarpe
Copy link
Contributor

  1. I found a bug(maybe) when I use viewController(:sender:) to push CarteViewController.
    It couldn't find a back button when in the left side of UINavigationBar.
    It removes the back button made by UINavigationBar so that I can't back to previous screen.
    When I tap the done button, whole UINavigationViewController stack is dismissed.

I tried self.navigationItem.backBarButtonItem it also returned nil.

I suggest this PR to avoid this situation. I can't use now this because of this situation.

If this Done button would be an option, it has more chance to be used by other programmers.

  1. I added super.viewWillAppear(). As I know, this is basically recommended.
    http://stackoverflow.com/questions/3348204/what-does-super-viewwillappear-do-and-when-is-it-required

@devxoul
Copy link
Owner

devxoul commented Feb 17, 2016

  1. Thank you for pointing it out :) How about setting leftBarButtonItem along if the view controllers is pushed or presented? This approach is what I'm using in my company project. I think it's better than creating an extra property.

    // pushed
    if self.navigationController?.viewControllers.count > 1 {
        self.navigationItem.leftBarButtonItem = nil
    }
    // presented
    else if self.presentingViewController != nil && self.navigationItem.leftBarButtonItem == nil {
        self.navigationItem.leftBarButtonItem = UIBarButtonItem(
            barButtonSystemItem: .Done,
            target: self,
            action: "doneButtonDidTap"
        )
    }
  2. Thank you. How about splitting commits? 1) and 2) are obviously different.

Thanks for your work :)

@innocarpe
Copy link
Contributor Author

@devxoul Thank you for your feedback. With your code programmers doesn't need to consider both situations. I like the 'It just works' ways. I couldn't split the commit 1) and 2) because it is already uploaded to github.

I reset the commits to develop and made a commits, and merged to the one in github. Is there better way to accomplish this?

@devxoul
Copy link
Owner

devxoul commented Feb 17, 2016

@kwosu87, thanks. It'll be better rebase commits.

@innocarpe innocarpe changed the title Provide an option for adding 'Done Button' Support the situation when VC is pushed by UINavigationController, not adding Done button Feb 17, 2016
devxoul added a commit that referenced this pull request Feb 17, 2016
Provide an option for adding 'Done Button'
@devxoul devxoul merged commit b8aa6d6 into devxoul:master Feb 17, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants