guoyingtao / Mantis

An iOS Image cropping library, which mimics the Photo App written in Swift.
MIT License
907 stars 180 forks source link

Make aspect ratio functions public #347

Closed xinatanil closed 11 months ago

xinatanil commented 11 months ago

The only limitation that doesn't allow us to build a custom UI wrapper around CropViewController is that aspect ratio functions are not public. I suggest making CropViewController.setFixedRatio and CropViewController.setFreeRatio functions public.

guoyingtao commented 11 months ago

Mantis uses one ratio selection button to toggle fixed ratios and free ratio. Generally users don't need to use setFixedRatio and setFreeRatio. Can you give me more details about your custom UI wrapper? @xinatanil

xinatanil commented 11 months ago

We're just trying to replicate Photos.app UI, which allows user to choose between fixed ratios and freeform

Simulator Screenshot - iPhone 15 - 2023-11-01 at 21 10 06

xinatanil commented 11 months ago

And we make the functions public, we can have quite a simple switch that allows us to select aspect ratios

func applyRatio(_ newRatio: AspectRatio) {
        switch newRatio {
        case .original:
            let size = image!.size
            cropController?.setFixedRatio(size.width / size.height, zoom: false)
        case .square:
            cropController?.setFixedRatio(1, zoom: false)
        case .freeform:
            cropController?.setFreeRatio()
        case .r_16_by_9:
            cropController?.setFixedRatio(16 / 9, zoom: false)
        case .r_5_by_4:
            cropController?.setFixedRatio(5 / 4, zoom: false)
        case .r_7_by_5:
            cropController?.setFixedRatio(7 / 5, zoom: false)
        case .r_4_by_3:
            cropController?.setFixedRatio(4 / 3, zoom: false)
        case .r_3_by_2:
            cropController?.setFixedRatio(3 / 2, zoom: false)
        case .r_10_by_16:
            cropController?.setFixedRatio(10 / 16, zoom: false)
        case .r_9_by_16:
            cropController?.setFixedRatio(9 / 16, zoom: false)
        case .r_4_by_5:
            cropController?.setFixedRatio(4 / 5, zoom: false)
        case .r_5_by_7:
            cropController?.setFixedRatio(5 / 7, zoom: false)
        case .r_3_by_4:
            cropController?.setFixedRatio(3 / 4, zoom: false)
        case .r_2_by_3:
            cropController?.setFixedRatio(2 / 3, zoom: false)
        case .r_16_by_10:
            cropController?.setFixedRatio(16 / 10, zoom: false)
        }
    }
guoyingtao commented 11 months ago

@xinatanil Got it. Do you still use CropToolbar with custom icons?

guoyingtao commented 11 months ago

If you check CustomizedCropToolbarWithoutList in Example project, you can see it uses delegate?.didSelectRatio(self, ratio: 9 / 16) to do the similar thing. CropToolbarDelegate also provides didSelectFreeRatio.

Can you check if you can use CropToolbarDelegate as code in CustomizedCropToolbarWithoutList?

xinatanil commented 11 months ago

Yeah, the first thing I tried to do is to use CropToolBar methods, but the API was too complicated, and if I remember correctly, it still didn't allow us to add freeform ratio to the list (which was essential to us). I think that CropViewController doesn't need to provide such complicated APIs. I think that there should be only two ways to use CropViewController:

  1. Use default UI that provides little configuration.
  2. Build your own custom UI around embedded CropViewController. It's impossible to support everyone's UI needs, so it'd be easier to provide an API so that anyone could build any UI he wants.
guoyingtao commented 11 months ago

@xinatanil Thanks for your suggestion. CropViewController conforms to CropToolbarDelegate so it has some public functions already like didSelectFreeRatio and didSelectRatio which actually call private functions like setFixedRatio and setFreeRatio. Do you think you can directly use didSelectFreeRatio and didSelectRatio?

xinatanil commented 11 months ago

Yes, actually, I can! Thank you for the hint, I don't know how I was unable to notice them in the first place! Thank you for the library!

guoyingtao commented 11 months ago

@xinatanil I just noticed didSelectRatio does not provide zoom parameter as setFixedRatio does. In your sample code, you always set zoom to false in setFixedRatio. If you still want zoom to be false, then I need to add zoom parameter to didSelectRatio.

Update zoom parameter was added to fix a bug and only is only used inside Mantis. So if you use setFixedRatio, you don't need to care about it.