Automattic / Gravatar-SDK-iOS

Gravatar SDK is a Swift library that allows you to integrate Gravatar features into your own iOS applications.
https://gravatar.com
Mozilla Public License 2.0
36 stars 1 forks source link

Add image cropper #407

Closed pinarol closed 5 days ago

pinarol commented 5 days ago

Closes part of https://github.com/Automattic/Gravatar-SDK-iOS/issues/381 . This is necessary to be able to use PHPickerViewController.

Description

Adding an image cropper to be used with PHPhotoPicker since it doesn't give us a cropper out of the box.

https://github.com/user-attachments/assets/11765ec2-4796-47de-8b3c-faf3dcd17596

Testing Steps

wpmobilebot commented 5 days ago
Gravatar UIKit Prototype Build📲 You can test the changes from this Pull Request in Gravatar UIKit Prototype Build by scanning the QR code below to install the corresponding build.
App NameGravatar UIKit Prototype Build Gravatar UIKit Prototype Build
Build Number1252
Version1.0
Bundle IDcom.automattic.gravatar-sdk-demo-uikit.prototype-build
Commit715c1e53e72335c786f179b2fe090b8c5e343cf6
App Center BuildGravatar SDK Demo - UIKit #128
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.
wpmobilebot commented 5 days ago
Gravatar SwiftUI Prototype Build📲 You can test the changes from this Pull Request in Gravatar SwiftUI Prototype Build by scanning the QR code below to install the corresponding build.
App NameGravatar SwiftUI Prototype Build Gravatar SwiftUI Prototype Build
Build Number1252
Version1.0
Bundle IDcom.automattic.gravatar-sdk-demo-swiftui.prototype-build
Commit715c1e53e72335c786f179b2fe090b8c5e343cf6
App Center BuildGravatar SDK Demo - SwiftUI #127
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.
etoledom commented 5 days ago

On the UI side, I like that it looks close to the system cropper 🎉

I'd suggest two small changes, which are personal preferences (so feel free to disagree 😅 )

I think it would be nice to have the action buttons on a toolbar at the bottom, as it is in the system cropper:

CleanShot 2024-09-13 at 15 22 02@2x

And the second one: This one is a bit more tricky, since this behavior is defined in the Demo app. We can achieve the presentation of the cropper to be at the top of the picker (again, as in the system cropper). This I believe feels more fluid than dismissing the picker and presenting the cropper as starting a different flow.

Something like this:

CleanShot 2024-09-13 at 15 31 19

I was thinking that ideally, we could have a controller which creates one single flow composing the picking with PHPickerViewController and our custom cropper. In this way we can control the presentation animation internally, and this probably will also make it easier to bring it to SwiftUI as just one component which behaves as we want.

What do you think?

etoledom commented 5 days ago

One small detail: When I open a portrait image with the device oriented on landscape, it seems that the image is out of center:


This one is curious 🤔
There seems to be a difference on the scale depending on the device orientation. The resulting images seem to be the same though, so maybe is not a problem. I'm just curious to see the difference.

pinarol commented 5 days ago

When I open a portrait image with the device oriented on landscape, it seems that the image is out of center:

Should be fixed now.

pinarol commented 5 days ago

And the second one: This one is a bit more tricky, since this behavior is defined in the Demo app. We can achieve the presentation of the cropper to be at the top of the picker (again, as in the system cropper).

We'll need to do that when we integrate this cropper to the avatar picker together with the PHPhotoPicker. The image cropper doesn't contain any present&dismiss calls. So it is all up to the presenting party.

pinarol commented 5 days ago

There seems to be a difference on the scale depending on the device orientation.

The cropping rect can be slightly smaller in landscape mode, layout constraints make it so that the cropper always fits in the view. When there's not much space left, the cropping frame will shrink a bit. Which might result in a small difference in the cropped area. The important thing is, does it actually crop the area inside the crop frame...

As for the difference between scale:

scale: 1 means we had to adjust the image. this can happen if:

scale: 3 means we didn't need to adjust it. it was already a perfect square and its dimensions are under the limit.

pinarol commented 5 days ago

I think it would be nice to have the action buttons on a toolbar at the bottom, as it is in the system cropper:

Yeah let's do this on a follow up PR. I created an issue for that. https://github.com/Automattic/Gravatar-SDK-iOS/issues/408