chore: support for macOS Designed for iPad mode - WPB-25053#4636
chore: support for macOS Designed for iPad mode - WPB-25053#4636
Conversation
|
Test Results1 881 tests 1 854 ✅ 2m 8s ⏱️ Results for commit f1d12c4. Summary: workflow run #24894790188 |
| } | ||
|
|
||
| static func wr_requestOrWarnAboutPhotoLibraryAccess(_ grantedHandler: @escaping (Bool) -> Void) { | ||
| PHPhotoLibrary.requestAuthorization { status in |
There was a problem hiding this comment.
Btw, requestAuthorization(_:) is deprecated.
| PHPhotoLibrary.requestAuthorization(for: .readWrite) { status in |
| <array/> | ||
| <key>UIRequiresFullScreen</key> | ||
| <true/> | ||
| <false/> |
There was a problem hiding this comment.
This is a big change. We need to test resizing thoroughly, especially with horizontalSizeClass changes before disabling this.
There was a problem hiding this comment.
Also any time we are doing UIDevice.current.userInterfaceIdiom we might be making assumptions about screen sizes.
samwyndham
left a comment
samwyndham
left a comment
There was a problem hiding this comment.
Nice work. I really like the idea of getting this working and it's nice that the changes aren't too great.
My main concern is that we are changing a few fundamentals in production. Some questions:
- Will Designed for iPad also appear in production?
- Can we make the change to full screen mode internal/beta only?
| SUPPORTED_PLATFORMS = "iphoneos iphonesimulator"; | ||
| SUPPORTS_MACCATALYST = NO; | ||
| SUPPORTS_MAC_DESIGNED_FOR_IPHONE_IPAD = NO; | ||
| SUPPORTS_MAC_DESIGNED_FOR_IPHONE_IPAD = YES; |
There was a problem hiding this comment.
question: Will this change apply to the production apps too?
| grantedHandler(true) | ||
| case .limited: | ||
| fallthrough | ||
| grantedHandler(true) |
There was a problem hiding this comment.
question: This is a change in behavior on iPhone/iPad too right? Do you know why we previously fell through?
| <key>UIRequiredDeviceCapabilities</key> | ||
| <array/> | ||
| <key>UIRequiresFullScreen</key> | ||
| <true/> |
There was a problem hiding this comment.
question: This is a pretty big change but probably the right direction to go. But have you thought about possible issues here? I guess there must be a bunch of things to test.
There was a problem hiding this comment.
Can we make this change only in non production builds?
| <array/> | ||
| <key>UIRequiresFullScreen</key> | ||
| <true/> | ||
| <false/> |
There was a problem hiding this comment.
Also any time we are doing UIDevice.current.userInterfaceIdiom we might be making assumptions about screen sizes.
| /// A minimal video recorder for Mac (Designed for iPad). | ||
| /// UIImagePickerController with .camera source crashes on Mac because Portrait Effects | ||
| /// tries to create Metal textures with unsupported IOSurface formats. This replaces it. | ||
| final class MacVideoRecorderViewController: UIViewController { |
There was a problem hiding this comment.
question: Did you check if this is possible and simpler in SwiftUI?
3 participants
Issue
In order to use more the Beta builds, this PR improve support for the app running on macOS via iPad design.
Testing
Checklist
[WPB-XXX].UI accessibility checklist
If your PR includes UI changes, please utilize this checklist: