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

Use a container VC to make sure UIKit owned insets do not go wild. #48

Merged
merged 1 commit into from
Nov 24, 2017

Conversation

andersio
Copy link
Contributor

@andersio andersio commented Nov 24, 2017

Fixes #31.

TL;DR

If the drawer content is laid out against (1) safe area top or (2) top layout guide, these are now in effect throughout the entire presentation time, regardless of the drawer being fully expanded or not.

Details

The safe area mechanism (and its earlier variant (top|bottom)LayoutGuide) appears to be dependent on the view origin. If the view does not lie at (0, 0) of the screen, UIKit would not consider the view overlapping with top system bars, thus having a safe area top inset of zero (or top layout guide length).

Jeez. ZERO.

Fortunately, child VCs inherit safe area insets (and layout guides) from their parent VCs. So a full-screen transparent container VC is added. This container VC manages the presented drawer, so that the drawer now gets system layout insets at all phases of presentation. 💁‍♂️

Note

This PR makes the drawer content in the Demo app layouts against the top layout guide.

GIF

Before After (Reversed)
drawer_kit_top_inset_before drawer_kit_top_inset

@andersio andersio added the bug label Nov 24, 2017
Copy link
Contributor

@wltrup wltrup left a comment

Choose a reason for hiding this comment

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

LGTM. Minor changes suggested.

@@ -21,11 +21,11 @@ private extension PresenterViewController {
// what you need to configure differently. They're all listed here just so you
// can see what can be configured. The values listed are the default ones,
// except where indicated otherwise.
configuration.totalDurationInSeconds = 3 // default is 0.4
configuration.totalDurationInSeconds = 0.4//3 // default is 0.4
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest to remove the //3 part.

configuration.durationIsProportionalToDistanceTraveled = false
// default is UISpringTimingParameters()
configuration.timingCurveProvider = UISpringTimingParameters(dampingRatio: 0.8)
configuration.fullExpansionBehaviour = .leavesCustomGap(gap: 100) // default is .coversFullScreen
configuration.fullExpansionBehaviour = .coversFullScreen//.leavesCustomGap(gap: 100) // default is .coversFullScreen
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest to remove the //.leavesCustomGap(gap: 100) part.

@andersio andersio force-pushed the anders/system-top-insets branch from 0b34c0d to cb357d7 Compare November 24, 2017 16:01
@wltrup wltrup merged commit 9d9bf1f into master Nov 24, 2017
@wltrup wltrup deleted the anders/system-top-insets branch November 24, 2017 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants