-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Integrate PHPickerViewController in Media library #21343
Merged
Merged
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
67a22de
Add .nativePhotoPicker feature flag
kean 29b10fc
Integrate PHPickerController on the Media screen
kean 7274402
Add WebP support and cleanup
kean b336e4c
Add more unit tests
kean c317cef
Update error handling
kean 2a19f6a
Remove selection limit
kean 0a680ab
Merge branch 'trunk' into task/phpicker-media-screen
kean 2e23696
Fix typos
kean 7cc3ee8
Fix media export unit tests by isolating them
kean File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
117 changes: 117 additions & 0 deletions
117
WordPress/Classes/Utility/Media/ItemProviderMediaExporter.swift
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,117 @@ | ||
import Foundation | ||
import PhotosUI | ||
|
||
/// Manages export of media assets: images and video. | ||
final class ItemProviderMediaExporter: MediaExporter { | ||
var mediaDirectoryType: MediaDirectory = .uploads | ||
var imageOptions: MediaImageExporter.Options? | ||
var videoOptions: MediaVideoExporter.Options? | ||
|
||
private let provider: NSItemProvider | ||
|
||
init(provider: NSItemProvider) { | ||
self.provider = provider | ||
} | ||
|
||
func export(onCompletion: @escaping (MediaExport) -> Void, onError: @escaping (MediaExportError) -> Void) -> Progress { | ||
let progress = Progress.discreteProgress(totalUnitCount: MediaExportProgressUnits.done) | ||
|
||
// It's important to use the `MediaImageExporter` because it strips the | ||
// GPS data and performs other image manipulations before the upload. | ||
func processImage(at url: URL) throws { | ||
let exporter = MediaImageExporter(url: url) | ||
exporter.mediaDirectoryType = mediaDirectoryType | ||
if let imageOptions { | ||
exporter.options = imageOptions | ||
} | ||
// If image format is not supported, switch to `.heic`. | ||
if exporter.options.exportImageType == nil, | ||
let type = provider.registeredTypeIdentifiers.first, | ||
!ItemProviderMediaExporter.supportedImageTypes.contains(type) { | ||
exporter.options.exportImageType = UTType.heic.identifier | ||
} | ||
let exportProgress = exporter.export(onCompletion: onCompletion, onError: onError) | ||
progress.addChild(exportProgress, withPendingUnitCount: MediaExportProgressUnits.halfDone) | ||
} | ||
|
||
// `MediaImageExporter` doesn't support GIF, so it requires special handling. | ||
func processGIF(at url: URL) throws { | ||
let pixelSize = url.pixelSize | ||
let media = MediaExport(url: url, fileSize: url.fileSize, width: pixelSize.width, height: pixelSize.height, duration: nil) | ||
onCompletion(media) | ||
} | ||
|
||
func processVideo(at url: URL) throws { | ||
let exporter = MediaVideoExporter(url: url) | ||
exporter.mediaDirectoryType = mediaDirectoryType | ||
if let videoOptions { | ||
exporter.options = videoOptions | ||
} | ||
let exportProgress = exporter.export(onCompletion: onCompletion, onError: onError) | ||
progress.addChild(exportProgress, withPendingUnitCount: MediaExportProgressUnits.halfDone) | ||
} | ||
|
||
let loadProgress = provider.loadFileRepresentation(forTypeIdentifier: UTType.data.identifier) { url, error in | ||
guard let url else { | ||
onError(ExportError.underlyingError(error)) | ||
return | ||
} | ||
DDLogDebug("Loaded file representation (filename: '\(url.lastPathComponent)', types: \(self.provider.registeredTypeIdentifiers)") | ||
|
||
// Retaining `self` on purpose. | ||
do { | ||
let copyURL = try self.mediaFileManager.makeLocalMediaURL(withFilename: url.lastPathComponent, fileExtension: url.pathExtension) | ||
try FileManager.default.copyItem(at: url, to: copyURL) | ||
|
||
if self.hasConformingType(.gif) { | ||
try processGIF(at: copyURL) | ||
} else if self.hasConformingType(.image) { | ||
try processImage(at: copyURL) | ||
} else if self.hasConformingType(.movie) || self.hasConformingType(.video) { | ||
try processVideo(at: copyURL) | ||
} else { | ||
onError(ExportError.unsupportedContentType) | ||
} | ||
} catch { | ||
onError(ExportError.underlyingError(error)) | ||
} | ||
} | ||
progress.addChild(loadProgress, withPendingUnitCount: MediaExportProgressUnits.halfDone) | ||
return progress | ||
} | ||
|
||
/// The list of image formats supported by the backend. | ||
/// See https://wordpress.com/support/accepted-filetypes/. | ||
/// | ||
/// One notable format missing from the list is `.webp`, which is not supported | ||
/// by `CGImageDestinationCreateWithURL` and, in turn, `MediaImageExporter`. | ||
/// | ||
/// If the format is not supported, the app fallbacks to `.heic` which is | ||
/// similar to `.webp`: more efficient than traditional formats and supports | ||
/// opacity, unlike `.jpeg`. | ||
private static let supportedImageTypes: Set<String> = Set([ | ||
UTType.png, | ||
UTType.jpeg, | ||
UTType.gif, | ||
UTType.heic, | ||
UTType.svg | ||
].map(\.identifier)) | ||
|
||
private func hasConformingType(_ type: UTType) -> Bool { | ||
provider.hasItemConformingToTypeIdentifier(type.identifier) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is a nice UTType-based API but it’s available only in iOS 16 :( |
||
} | ||
|
||
enum ExportError: MediaExportError { | ||
case unsupportedContentType | ||
case underlyingError(Error?) | ||
|
||
var description: String { | ||
switch self { | ||
case .unsupportedContentType: | ||
return NSLocalizedString("mediaExporter.error.unsupportedContentType", value: "Unsupported content type", comment: "An error message the app shows if media import fails") | ||
case .underlyingError(let error): | ||
return error?.localizedDescription ?? NSLocalizedString("mediaExporter.error.unknown", value: "The item could not be added to the Media library", comment: "An error message the app shows if media import fails") | ||
} | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,6 @@ | ||
import Foundation | ||
import CoreGraphics | ||
import UIKit | ||
import MobileCoreServices | ||
import UniformTypeIdentifiers | ||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
14 changes: 14 additions & 0 deletions
14
WordPress/Classes/ViewRelated/Media/NSIterProvider+Exportable.swift
kean marked this conversation as resolved.
Show resolved
Hide resolved
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
import UIKit | ||
|
||
extension NSItemProvider: ExportableAsset { | ||
public var assetMediaType: MediaType { | ||
if hasItemConformingToTypeIdentifier(UTType.image.identifier) { | ||
return .image | ||
} else if hasItemConformingToTypeIdentifier(UTType.video.identifier) || | ||
hasItemConformingToTypeIdentifier(UTType.movie.identifier) { | ||
return .video | ||
} else { | ||
return .document | ||
} | ||
} | ||
} |
13 changes: 13 additions & 0 deletions
13
WordPress/Classes/ViewRelated/Media/PHPickerContoller+Extensions.swift
kean marked this conversation as resolved.
Show resolved
Hide resolved
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
import UIKit | ||
import PhotosUI | ||
|
||
extension PHPickerConfiguration { | ||
/// Returns the picker configuration optimized for the Jetpack app. | ||
static func make() -> PHPickerConfiguration { | ||
var configuration = PHPickerConfiguration(photoLibrary: .shared()) | ||
configuration.preferredAssetRepresentationMode = .compatible | ||
configuration.selection = .ordered | ||
configuration.selectionLimit = 0 // Unlimited | ||
return configuration | ||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
It works nearly identical to the existing
MediaAssetExporter
(PHAsset
support) and shares some of the code with it.Great post on
NSItemProvider
: https://www.humancode.us/2023/07/08/all-about-nsitemprovider.html.