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

Bug with modals #21

Closed
Alex293 opened this issue Oct 28, 2016 · 11 comments
Closed

Bug with modals #21

Alex293 opened this issue Oct 28, 2016 · 11 comments

Comments

@Alex293
Copy link

Alex293 commented Oct 28, 2016

Hi this time It's a more important bug :)

After dismissing a modal displayed over the pulley, the content drawer bugs.
I've managed to use only modal segues over full screen so it keeps my drawer so problem solved but when I don't manage my self the segue (Facebook login for ex) the bug stays. Think there is something to do with define context or something like this but I'm not expert with those.

@amyleecodes
Copy link
Contributor

Looks like it needs -definesPresentationContext set to true. I'll try to push that as a default in the next update. In the mean time, you can try setting the Pulley drawer's -definesPresentationContext property to true and see if that helps.

@Alex293
Copy link
Author

Alex293 commented Oct 30, 2016

Doesn't seems to do anything I'll try to investigate this more

@Alex293
Copy link
Author

Alex293 commented Jan 24, 2017

Hi @Brendan09

I've tried lots of things but nothing seems to solve my issue. I didn't managed to reproduce the bug from the example, however I know what is the source of the issue : the passthrough view (in blue) of the pulley is out of the screen after dismissing view :

screen shot 2017-01-24 at 15 10 51

While this is its position when I first load the view and after a dismiss if my modal is presented over full screen :

screen shot 2017-01-24 at 15 11 04

The thing I don't understand is that this view is not accessible out of the lib so there is no way I've modified it.

@amyleecodes
Copy link
Contributor

If anyone has a reliable way to replicate this, I'm happy to take a look. It works with modal presentations that I've tried (with definesPresentationContext set to true). A release coming out later today defaults definesPresentationContext to true.

@pschneider
Copy link

I may have found a way to reproduce this in our project. Investigating.... will try to encapsulate this into a sample project sometime this week.

@ethansinjin
Copy link
Contributor

I know this issue hasn't been touched in a while, but I'm still seeing this problem. I can try to upload a video of the issue as well as a sample project describing the issue if that would help

@amyleecodes
Copy link
Contributor

@ethansinjin (some things to test, might help narrow it down) In your drawer content or main content VC, what happens if you do this in viewWIllAppear or viewDidAppear?

 self.parent?.view.setNeedsLayout()

If that fixes the issue (even if it has a UI glitch), I have some ideas on what may be wrong.

If you can put together a sample with the issue (even if derived from the sample project in the repo), I'd much appreciate it. I can find you a solution then. I think another user was going to try to upload a sample project, but one never found its way to the issue.

Happy to take a look.

@ruilunli
Copy link

ruilunli commented Aug 3, 2017

@Brendan09 Based on the sample project, I added a view controller before the pulley view controller. And in the drawer content view controller, instead of drawer.setPrimaryContentViewController, I present as a modal. When the modal dismiss, the drawer view controller bugs. (The issue is found on iOS9)
screen shot 2017-08-03 at 7 50 52 pm
screen shot 2017-08-03 at 7 51 06 pm
screen shot 2017-08-03 at 7 52 22 pm

@amyleecodes
Copy link
Contributor

@ruilunli I've tried doing what you described, but haven't been able to replicate it (even on iOS 9). Can you post a download link to the sample project in a broken state (or email it to me: brendan@52inc.com). Once I can replicate it, I can get a fix out for you. Thanks!

@amyleecodes
Copy link
Contributor

@ruilunli Ok, I've found the issue.

The core of the issue is in these 2 lines in your sample:

      self.definesPresentationContext = true

and

      primaryContent.modalPresentationStyle = .currentContext

When you change the context to be a view controller inside of a nested view controller hierarchy, and do a regular (not 'over') presentation style you're creating an issue in how UIKit handles view controller presentation.

When you don't use an 'over' presentation style, the view of the previous view controller that was defining the presentation context is removed from it's superview (which is typically the window). Since you change that to the drawer view controller, the drawer view controller's view is removed from it's superview. When you dismiss the view controller that you presented, UIKit adds it back...but it adds it back in the wrong place. It puts it directly into the window, rather than back into it's nested view inside of Pulley.

This isn't a bug in Pulley, so much as a way that UIKit doesn't intend defining context for presentations to be used.

You can solve this by using the 'overCurrentContext' presentation style:

      primaryContent.modalPresentationStyle = .overCurrentContext

I've also written a patch for Pulley that will prevent this from being an issue. It's a little hacky, but does resolve the problem. I'll push that to CocoaPods this afternoon.

Thanks so much for your help in tracking this down! (Also: Interesting way to prevent secondary 'drawer' views that I hadn't considered.)

amyleecodes added a commit that referenced this issue Aug 11, 2017
amyleecodes added a commit that referenced this issue Aug 11, 2017
* master:
  Fix for Bug with modals #21
  Pod point bump
  support nested PulleyViewControllers (#61)
@ruilunli
Copy link

Thanks for spotting!

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

No branches or pull requests

5 participants