-
Notifications
You must be signed in to change notification settings - Fork 677
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
Add iPhoneX layout support #372
Conversation
Move topview down to the safe area guides and increase bottomview height by the safe area height.
attribute: .notAnAttribute, | ||
multiplier: 1, | ||
constant: BottomContainerView.Dimensions.height)) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! I was also about to add the iPhoneX support, but I'm glad that you were quicker ;-)
LGTM 👍
Just as an idea: You could add the new constraints in a bit more compact way like this:
var topViewToItem: Any? = view
var bottomContainerHeight = BottomContainerView.Dimensions.height
if #available(iOS 11.0, *) {
topViewToItem = view.safeAreaLayoutGuide
bottomContainerHeight += UIApplication.shared.keyWindow!.safeAreaInsets.bottom
}
view.addConstraint(NSLayoutConstraint(item: topView, attribute: .top,
relatedBy: .equal, toItem: topViewToItem,
attribute: .top,
multiplier: 1, constant: 0))
view.addConstraint(NSLayoutConstraint(item: bottomContainer, attribute: .height,
relatedBy: .equal, toItem: nil,
attribute: .notAnAttribute,
multiplier: 1,
constant: bottomContainerHeight)
)
Happy to make this change and resubmit if you like, just LMK.
…On Fri, Jan 12, 2018 at 2:05 PM Jörn Schoppe ***@***.***> wrote:
***@***.**** approved this pull request.
------------------------------
In Source/Extensions/ConstraintsSetup.swift
<#372 (comment)>:
> + }
+
+ if #available(iOS 11.0, *) {
+ let heightPadding = UIApplication.shared.keyWindow!.safeAreaInsets.bottom
+ view.addConstraint(NSLayoutConstraint(item: bottomContainer, attribute: .height,
+ relatedBy: .equal, toItem: nil,
+ attribute: .notAnAttribute,
+ multiplier: 1,
+ constant: BottomContainerView.Dimensions.height + heightPadding))
+ } else {
+ view.addConstraint(NSLayoutConstraint(item: bottomContainer, attribute: .height,
+ relatedBy: .equal, toItem: nil,
+ attribute: .notAnAttribute,
+ multiplier: 1,
+ constant: BottomContainerView.Dimensions.height))
+ }
Nice! I was also about to add the iPhoneX support, but I'm glad that you
were quicker ;-)
LGTM 👍
Just as an idea: You could add the new constraints in a bit more compact
way like this:
var topViewToItem: Any? = view
var bottomContainerHeight = BottomContainerView.Dimensions.height
if #available(iOS 11.0, *) {
topViewToItem = view.safeAreaLayoutGuide
bottomContainerHeight += UIApplication.shared.keyWindow!.safeAreaInsets.bottom
}
view.addConstraint(NSLayoutConstraint(item: topView, attribute: .top,
relatedBy: .equal, toItem: topViewToItem,
attribute: .top,
multiplier: 1, constant: 0))
view.addConstraint(NSLayoutConstraint(item: bottomContainer, attribute: .height,
relatedBy: .equal, toItem: nil,
attribute: .notAnAttribute,
multiplier: 1,
constant: bottomContainerHeight)
)
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#372 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAf6aTa2tdYYG-MNMS4p5LOWa1T4zTeTks5tJ9cbgaJpZM4RXEqz>
.
|
I'd go for the more compact version, but your version is also fine. You decide ;-) |
Let's go with mine just because I have it committed, testing and working in my fork - will save me backing it out and reapplying. Thanks for taking the time to provide feedback! |
Any reason why this PR is not merged? |
@zenangst this PR needs to be merged please. The iPhone X layout issue is definitely going to cause App Store rejection. |
@valeriyvan @annjawn Sorry for late response. Thanks for the PR 🚀 |
@onmyway133 awesome! 👏🏼 |
Move topview down to the safe area guides and increase bottomview height by the safe area height.