Add partial support for iPhone X #57
Conversation
There was a problem hiding this comment.
Could you please fix issues that I pointed out or allow editing of this pull request so that I could do it myself? (See https://help.github.com/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork)
| @@ -43,6 +43,9 @@ | |||
| 25A489CC1E656B2D00CC431B /* Latoto-SemiboldItalic.ttf in Resources */ = {isa = PBXBuildFile; fileRef = 251E57EC1E6565890009A288 /* Latoto-SemiboldItalic.ttf */; }; | |||
| 2A0B5C0CB29FF72D8233F563 /* Pods_PaparazzoExample_Storyboard.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 0197BFD7D8A9DC3C06FA3EB3 /* Pods_PaparazzoExample_Storyboard.framework */; }; | |||
| 38D34B668F9788158C7228EA /* Pods_PaparazzoExample.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 11AE831F8581BCFBA632B260 /* Pods_PaparazzoExample.framework */; }; | |||
| BB0265081FA86FB4002A5561 /* LaunchScreen.storyboard in Resources */ = {isa = PBXBuildFile; fileRef = BB0265061FA86FB4002A5561 /* LaunchScreen.storyboard */; }; | |||
| BB0265091FA86FB4002A5561 /* LaunchScreen.storyboard in Resources */ = {isa = PBXBuildFile; fileRef = BB0265061FA86FB4002A5561 /* LaunchScreen.storyboard */; }; | |||
| BB02650A1FA86FB4002A5561 /* LaunchScreen.storyboard in Resources */ = {isa = PBXBuildFile; fileRef = BB0265061FA86FB4002A5561 /* LaunchScreen.storyboard */; }; | |||
There was a problem hiding this comment.
Adding LaunchScreen is unnecessary for the purpose of this pull request. Please remove it.
There was a problem hiding this comment.
Per Apple’s guide to Update Apps for iPhone X, apps need either a LaunchScreen.storyboard or an iPhone X launch image (1125px × 2436px). Since Paparazzo already included the LaunchScreen.storyboard for the storyboard-based example project, I moved the storyboard to the shared directory (Example/PaparazzoExample_Storyboard/Base.lproj/LaunchScreen.storyboard → Example/Shared/Base.lproj/LaunchScreen.storyboard) and included it in the other two targets. This diff hunk shows the file being included in all three targets, not a duplication of the file.
If you prefer not to include even a LaunchScreen.storyboard in the non-storyboard-based targets, please provide a 1125px × 2436px launch screen image.
There was a problem hiding this comment.
Oh, I didn't know that, thanks for the explanation!
|
|
||
| let minimumYOffset: CGFloat | ||
| if #available(iOS 11.0, *) { | ||
| minimumYOffset = -safeAreaInsets.top |
There was a problem hiding this comment.
I think it's better to use -max(contentInset.top, safeAreaInsets.top) here.
| topMargin = safeAreaInsets.top | ||
| } else { | ||
| topMargin = 0 | ||
| } |
There was a problem hiding this comment.
To avoid constant checking for the availability of safeAreaInsets I would add a UIView extension like that:
extension UIView {
var paparazzoSafeAreaInsets: UIEdgeInsets {
if #available(iOS 11.0, *) {
return safeAreaInsets
} else {
return .zero
}
}
}| @@ -21,8 +21,6 @@ final class PhotoLibraryViewController: PaparazzoViewController, PhotoLibraryVie | |||
| super.viewWillAppear(animated) | |||
|
|
|||
| navigationController?.setNavigationBarHidden(false, animated: animated) | |||
| hideNavigationBarShadow() | |||
There was a problem hiding this comment.
What is the purpose of this change?
There was a problem hiding this comment.
One of the navigation bar changes in hideNavigationBarShadow causes the table view’s contents to appear in the iPhone X status bar. (See the first image from this PR, reproduced below.) I’m not sure whether it’s setting the background image, the background color, or the shadow image. If you prefer to still set some of those, while removing the offending ones, some experimentation could determine which are causing the visual error below.
|
I can make the requested changes, but won’t be able to get back to this for a few days. If you’d like the changes sooner, this PR is actually already allows edits from maintainers of this project:
|
# Conflicts: # Example/PaparazzoExample.xcodeproj/project.pbxproj
Partial support for #48.
Photo library picker:
Old:
(Automatically scrolls too far, and shows photos between navigation bar and top of screen)
New:
Camera
Old:
(Buttons and “Photo 1” text too high)
New:
Same photo on iPhone 6s, for comparison:
Already supporting iPhone X:
Circle cropper:
Still not supporting iPhone X:
Photo library in landscape:
(Photos too close to left and right edges)
Camera in landscape:
(Buttons to close to “top” [physical top of phone, not virtual top of screen])
Edit tool: