Skip to content
This repository has been archived by the owner on Apr 15, 2019. It is now read-only.

Improved modal detection #36

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

Conversation

harlanhaskins
Copy link

Trying to push this view controller onto a navigation stack that's already being presented causes a 'Done' button. This new method only puts on a done button if the FSImageViewerViewController is the only view controller in the navigation stack.

@x2on
Copy link
Owner

x2on commented Jul 5, 2015

Hi @harlanhaskins,

i think this could lead to problems, if you use a a other presentation style, or?

Best,

Felix

@harlanhaskins
Copy link
Author

Hi x2on,

I'm not sure the modalPresentationStyle will affect this...

Right now, I'm using FSImageViewerViewController inside a modal that was presented using UIModalPresentationStyleOverCurrentContext.

This only checks to make sure it's the only thing in the navigation hierarchy before presenting the 'Done' button vs. back button.

@x2on
Copy link
Owner

x2on commented Jul 5, 2015

Wouldn't it be better to check if presentingViewController exists and the style is UIModalPresentationFullScreen or UIModalPresentationStyleOverCurrentContext and your change?

@harlanhaskins
Copy link
Author

That's a good point -- I can make that change

@x2on
Copy link
Owner

x2on commented Sep 3, 2015

Any news?

…ck for number of view controllers in navigation controller.
@harlanhaskins
Copy link
Author

Totally forgot! I updated the PR.

@x2on
Copy link
Owner

x2on commented Sep 4, 2015

The travis build is broken - i will have a look at it.

@harlanhaskins
Copy link
Author

Any chance we can merge this?

@x2on
Copy link
Owner

x2on commented Sep 21, 2015

Could you please merge current master into your branch?

@iamroberthood
Copy link

I was running across this as well. A done button would be incorrectly presented in certain circumstances. After making this change, the issue has gone away. Has this change been successfully merged into master?

@x2on
Copy link
Owner

x2on commented Sep 1, 2016

Any news about merging?

@harlanhaskins
Copy link
Author

Oh, my apologies. I'll do this later today!

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

Successfully merging this pull request may close these issues.

3 participants