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

Present modal with custom transition #1

Merged
merged 9 commits into from Dec 10, 2015
Merged

Present modal with custom transition #1

merged 9 commits into from Dec 10, 2015

Conversation

mluisbrown
Copy link
Contributor

Changed to make view controller presentable as modal VC using custom transition.
Only made a custom transition for the presentation, not the dismissal, where it's not necessary (uses the existing animation).

@RuiAAPeres
Copy link
Collaborator

I was testing this on a real device, and there is a point when you dismiss the ImageViewer, and the entire UI stops receiving touches.With the Close Button action to dismiss, it works. When you flip to dismiss, the rest of the UI stops responding.

@RuiAAPeres
Copy link
Collaborator

Ok, so with the Close Button action to dismiss, it works. When you flip to dismiss, the rest of the UI stops responding.

@mluisbrown
Copy link
Contributor Author

Ok sorry, forgot to adjust the swipe to dismiss code to handle VC being presented. Now fixed.

@RuiAAPeres
Copy link
Collaborator

It's working now. Do you think it would take a lot of work to use transitions for the dismiss? This way we would close the circle.

@RuiAAPeres
Copy link
Collaborator

@mluisbrown Btw, do you still need this code?

        guard let rootController = self.applicationWindow?.rootViewController else { return }

        self.view.frame = UIScreen.mainScreen().bounds
        self.applicationWindow!.addSubview(self.view)
        rootController.addChildViewController(self)
        self.didMoveToParentViewController(rootController)

@mluisbrown
Copy link
Contributor Author

Shouldn't be too much work to use transitions for the dismiss too. Regarding that code block, it depends if you want to be maintain the ability to use show() instead of presentViewController. If not, then we can remove the show method altogether (and a few other ifs where we check if the VC was presented or not)

@RuiAAPeres
Copy link
Collaborator

I was thinking about adding an extension to a UIViewController:

extension UIViewController {

   func presentImageViewer(imageViewer: ImageViewer, completion: Void -> Void = {}) {
      self.presentModalViewController(imageViewer, completion: completion)
  }
}

The thing is, if you don't remove the show, then you can't remove this as well:

-                    self.view.removeFromSuperview()                         
-                    self.removeFromParentViewController()

By removing the show, it makes the inner logic simpler.

… Now only possible to display using `presentViewController`
@RuiAAPeres
Copy link
Collaborator

@mluisbrown what's your feeling about the extension on an UIViewController?

@mluisbrown
Copy link
Contributor Author

I think it's a great idea.

2. Fix not removing NSNotification observer on swipe to dismiss
3. Added UIViewController extension method `presentImageViewer` as suggested by @RuiAAPeres
4. Made ImageViewer the `UIViewControllerTransitioningDelegate` (it decides which transition to use)
@@ -280,7 +285,7 @@ public final class ImageViewer: UIViewController, UIScrollViewDelegate {
}
}

private func showAnimation() {
internal func showAnimation(duration: NSTimeInterval, completion: ((Bool) -> Void)?) {

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can remove internal, it is by default.

@RuiAAPeres
Copy link
Collaborator

A small bug with the status bar:
status

…all status bar is horrible. Could not figure out a fix. Changing level of application window also doesn't resolve.
@mluisbrown
Copy link
Contributor Author

Made changes as per your suggestions. Status bar jump is fixed, but in-call status bar behaviour remains awful. I couldn't find a solution. Using windowLevel doesn't work either and is actually worse (ImageView window shifts down with no visible status bar)

RuiAAPeres added a commit that referenced this pull request Dec 10, 2015
Present modal with custom transition
@RuiAAPeres RuiAAPeres merged commit 1a8d0f2 into Krisiacik:master Dec 10, 2015
RuiAAPeres pushed a commit that referenced this pull request Dec 14, 2015
Merge changes from original
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.

None yet

2 participants