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 modal presentation on top of the drawer. #56

Merged
merged 2 commits into from
Apr 6, 2018

Conversation

andersio
Copy link
Contributor

@andersio andersio commented Apr 5, 2018

  1. Support modal presentation over the drawer.

    The presented view controller must however use overFullscreen for the drawer content to be placed correctly. This is due to the default fullScreen presentation style not asking DrawerKit for the correct frame before the dismissal animation begins. This causes the drawer being visible in the incorrect position until the dismissal animation completes.

  2. Update the demo app to showcase the support.

Modal over Partially Expanded Drawer Modal over Fully Expanded Drawer
partially_expanded fully-expanded

@@ -22,6 +22,10 @@ class PresentedViewController: UIViewController {
}
}
}

@IBAction func unwindFromModal(with segue: UIStoryboardSegue) {
print("Unwound from the modal.")
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this going to be removed at some point?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. It is the Storyboard unwind point for dismissing the modal in the demo.

Copy link
Contributor

Choose a reason for hiding this comment

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

I meant the print statement.

containerViewHeight: containerViewHeight,
configuration: configuration,
clampToNearest: true)
break
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand why you're no longer setting lastDrawerState here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is not necessary to update it during the course of the pan.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

lastDrawerState is not really consumed by anything until this PR, by the way.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it might be, for the benefit of along-side animations performed by the presenting and/or presented view controllers.

Copy link
Contributor Author

@andersio andersio Apr 6, 2018

Choose a reason for hiding this comment

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

After looking into this, changed is actually updating the targetDrawerState (renamed from lastDrawerState).

drawerPartialHeight: drawerPartialHeight,
containerViewHeight: containerViewHeight,
configuration: configuration,
clampToNearest: true)
currentDrawerY += panGesture.translation(in: view).y
Copy link
Contributor

Choose a reason for hiding this comment

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

Or here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ditto.

@@ -48,9 +40,6 @@ extension PresentationController {
configuration: configuration)
animateTransition(to: endingState)

case .cancelled:
animateTransition(to: lastDrawerState)
Copy link
Contributor

Choose a reason for hiding this comment

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

And, likewise, I don't understand the reason for removing this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cancelled is treated as completed now, and picks the next state based on current drawer state plus velocity.

Copy link
Contributor

Choose a reason for hiding this comment

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

So, if the user starts dragging the drawer up then changes his mind, the drawer continues up anyway? That doesn't sound right to me.