Integrate PHPickerViewController

kean commented 1 year ago

Integrate PHPickerViewController.

Changes

Out of the Scope:

Aztec (deprecated, won't risk introducing regressions)
Stories (low usage, integration is more complicated than on other screens meaning high risk of regressions)

Fixes

These fixes were made in the scope of the task:

Advantages

No need to request access to the photo library
Allows you to see your albums and favorites
Zoom
Search (if you have enough photos)
Consistent UI with the Photos app
Apple maintains and improves it every year
Allows excluding location metadata from the assets (iOS 17)
Runs out of process
Should be much better at supporting massive photo libraries
Modernize the codebase by removing the legacy Objective-C code
The new system for adding assets to the post eliminates memory spikes (more in https://github.com/wordpress-mobile/WordPress-iOS/pull/21360)

Limitations

There is no way to add custom UI to PHPickerViewController, so it won't be possible to add "Edit" feature to the picker. Fortunately, the app already allows the users to edit the photos after selection, and based on Tracks, the number of users who take advantage of editing is low (<2% of total uploads).
I'm yet to find a way to export full-size RAW images. NSFileProvider servers a downscaled version. More info here.
We can no longer show the video duration upload limit for free plan directly in the picker. More info here.

Closed Defects

Replacing the custom picker with a native allowed us to close the following issues:

More Info

kean commented 1 year ago

@staskus , @Guarani, thoughts on this one?

It fixes a number of issues, not to mention the massive UX improvements.

kean commented 1 year ago

One of the other benefits of using the native picker is that it runs out of process, so it doesn't contribute to the app's memory usage. It can be hugely beneficial, for example, if you are editing a post that already requires a lot of memory. It will also never bring the app down if it crashes.

Uploading media from the device must be one of the major reasons users will want the app on their phones. It has to work flawlessly.

staskus commented 1 year ago

@kean, thanks for the proposal!

I agree with your reasoning, especially given the benefits of memory management. Plus, since it's native it will require less maintenance compared to maintaining our own library MediaPicker-iOS.

PHPickerViewController is fairly new, when starting fresh I doubt we would choose to create our own MediaPicker-iOS.

To make a transparent transition, I think it would be good to map the features that we use of MediaPicker-iOS and PHPickerViewController to be transparent about what can be supported out of the box and what we would lose out (if anything) if we make the transition. A quick search shows that the picker is used not only for picking the photo from the device but also for WordPress Media Library, Free Photo Library, and Free GIF Library, at least from the editor (TenorPicker, StockPhotosPicker, MediaLibraryPicker).

osullivanchris commented 1 year ago

Came across this PR from your comment re; KPIs. Please push to make this happen and advocate for same on Android. Sounds great.

osullivanchris commented 1 year ago

This approach 100% makes sense when the user wants to access all their device images. I just wanted to check something. We have some explorations where we're showing media in line.

In the first example, the user has added an image block. Before opening the full library of photos on the device, Paul tried showing some recent images within the sheet
In the second example, I was playing around with showing images in the editor empty state. So a user could add an image in one tap, before even choosing an image block.

in-line media examples

Of course in both cases we'd need an entry point to the full 'phone library' of images. But I'm just wondering does what we're doing in these cases play well with what you're proposing here, or does it contradict what you're suggesting?

kean commented 1 year ago

@osullivanchris, the iOS has the APIs to do both:

When you enter the fullscreen experience, we can show the native picker
There is also a way to display the recent local assets in any way you want in the app

kean commented 1 year ago

I agree with your reasoning, especially given the benefits of memory management. Plus, since it's native it will require less maintenance compared to maintaining our own library MediaPicker-iOS.

@staskus we are on track to completely eliminate the need for the MediaPicker repo: the Photos picker is already in the works, and the Media screen will be re-written to Swift in phase 2. It will be a huge win to help modernize the codebase. The only missing part is the "Free" libraries that, based on Tracks, virtually nobody use.

staskus commented 1 year ago

Thanks for sharing @kean, that's great news! 👍

mokagio commented 1 year ago

I bumped some of the PRs in this issue from 23.1 to 23.2 because they were not ready to merge and the 23.1 code freeze will start shortly. I'll do the same with this issue, for consistency.

If this cannot wait two weeks and it's important that it makes it into this release, let me know and we'll organize a new beta once ready.

kean commented 1 year ago

It's done.

wordpress-mobile / WordPress-iOS