Skip to content

Safe area APIs availability #227

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.

Sign up for GitHub

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

Jump to bottom
Merged
lwdupont merged 3 commits into from
Oct 21, 2018
Merged

Safe area APIs availability #227

lwdupont merged 3 commits into from
Oct 21, 2018

Conversation

@toohotz
Copy link
Member

@toohotz toohotz commented Sep 13, 2018

So this PR is meant to apply some restrictions that weren't imposed on PR #203 but began started throwing warnings with my PR #214. These will resolve the warnings stated in issue #226 with API availability for the safe area insets as well.

So per our discussions, I have decided to attempt resolving this by restricting the API requiring the user of the API to do an @available check if they are targeting below iOS 9.

An example will be the code updated in the demo to work as follows (if targeting below iOS 9 again):

if (@available(iOS 9.0, *)) {
             [self.scrollView autoPinEdgesToSuperviewSafeAreaWithInsets:UIEdgeInsetsZero];
         } else {
             [self.scrollView autoPinEdgesToSuperviewEdgesWithInsets:UIEdgeInsetsZero];
         }

Feel free to suggest your thoughts and opinions on this.

@toohotz toohotz self-assigned this Sep 13, 2018
lwdupont
lwdupont approved these changes Sep 13, 2018
View reviewed changes
Copy link
Contributor

@lwdupont lwdupont left a comment

Choose a reason for hiding this comment

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

Looks great to me, thanks. 👍

@lwdupont
Copy link
Contributor

lwdupont commented Sep 15, 2018

@revolter your thoughts? :)

@lwdupont lwdupont merged commit 2bc3366 into master Oct 21, 2018
@lwdupont lwdupont deleted the safeAreaAvailability branch October 21, 2018 15:38
@lwdupont
Copy link
Contributor

lwdupont commented Oct 22, 2018

@toohotz Hey, when I tried to release this, one of the unit tests caused an exception right at the end of the tests.. do you have some time to look at it?

2018-10-21 19:09:28.443791-0700 Example-iOS[96139:4819677] [LayoutConstraints] The view hierarchy is not prepared for the constraint: <NSLayoutConstraint:0x600001725c70 V:[_UILayoutSpacer:0x600000bb83c0'UIVC-topLayoutGuide']-(50@50)-[UIView:0x7fa702d64590] priority:50 (inactive)>
When added to a view, the constraint's items must be descendants of that view (or the view itself). This will crash if the constraint needs to be resolved before the view hierarchy is assembled. Break on -[UIView(UIConstraintBasedLayout) _viewHierarchyUnpreparedForConstraint:] to debug.

@lwdupont
Copy link
Contributor

lwdupont commented Oct 22, 2018

@toohotz Hey, I tried to release this tonight, and it turns out at the end the unit tests are causing an autolayout exception - do you have some time to look into it?

@lwdupont
Copy link
Contributor

lwdupont commented Oct 22, 2018

Yikes, all those were posting during githubs outage (I kept getting errors!). Will delete a few shortly.

@toohotz
Copy link
Member Author

toohotz commented Oct 22, 2018

@lwdupont I just tested it out on my Xcode 10 and the tests seem to be working fine, (noted your message about the Github outages but that should not be related I assume?). Are you still having the exception thrown when running the tests on your machine still?

@lwdupont
Copy link
Contributor

lwdupont commented Oct 22, 2018

@toohotz Ya, still getting them. I just pulled master, opened it up in Xcode Version 10.0 (10A255), chose iPhone XR sim, and did a Command U to run tests. Right at the end of the tests I am getting that exception.

I'll try my work laptop later today.

@revolter revolter removed their request for review January 2, 2019 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@lwdupont lwdupont lwdupont approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@toohotz toohotz

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@toohotz @lwdupont