advanced-cropper / vue-advanced-cropper

The advanced vue cropper library that gives you opportunity to create your own croppers suited for any website design
https://advanced-cropper.github.io/vue-advanced-cropper/
Other
931 stars 130 forks source link

When you rotate an image, the flip function reverse horizontal and vertical #211

Open kevpug opened 1 year ago

kevpug commented 1 year ago

When you rotate an image, the flip function of the cropper doesn't adapt to the rotated image. So flip function with true on horizontal flips horizontally until you, for example, rotate right. After that, it will rotate vertically.

I made a codesandbox with buttons to test the rotate/flip functions. https://codesandbox.io/s/eager-chatelet-jhnm5d

Steps to repeat the problem:

  1. Go on my codesandbox
  2. Try the buttons flip horizontal / flip vertical to see them working fine
  3. Rotate the image one time either to the right or the left
  4. Try the buttons flip horizontal / flip vertical to see them flipping the wrong way.
Norserium commented 1 year ago

@kevpug, it's kind of intentional decision.

If you look at the different croppers in common websites / applications you will find out that croppers resolves this case differently.

Look at the screenshot: image

It's not obvious how flip should behave in this particular case.

So I decide to not make the decision for developers and implement the flipping in the most obvious way: if you flip the image horizontal/vertical it will be flipped horizontal/vertical just like it was flipped before image loading.

Think of it as a image transformation.

How to get the desired result in this conditions?

flip(horizontal, vertical) {
    const { image } = this.$refs.cropper.getResult();
    if (image.transforms.rotate % 180 !== 0) {
        this.$refs.cropper.flip(!horizontal, !vertical);
    } else {
        this.$refs.cropper.flip(horizontal, vertical);
    }
}

Probably, I will try to make it a default normalization (that can be turned off) for flip method in the next major version, but I still need to think about that.

Norserium commented 1 year ago

@kevpug, if you have your own opinion on this matter, don't hesitate to write it. It's important to me.

Norserium commented 1 year ago

@kevpug, I assume that it will look in the next release like:

cropper.flipImage(horizontal, vertical, {
   normalize
})

Actually, I've implemented it in react-advanced-cropper now. Feel free to play with it.

kevpug commented 1 year ago

Firstly, thanks for all the answers that's some QUALITY service ahah

For my opinion, I like to put myself in the place of a user of your cropper... you would want it to be as straight forward as possible. So, you wouldn't want the user to need to imagine the initial picture in his mind to flip it correctly when the one he is working with is rotated.

I feel that you did it perfectly with the react-advanced-cropper !

Normally I would feel that it is supposed to be the default way of working but here I think it should just be an option that is off and needs to be turned on just in case people used the same logic as the piece of code you gave me in your first answer. This way, it shall not disrupt the code of nobody.

In any case, thanks a lot, I will use the code you gave me to achieve my desired outcome.

Norserium commented 1 year ago

@kevpug, you are welcome!

Thank you for your feedback.

I assume that it could well be an another breaking change, so it may be enabled by default. There are a lot of them already, because I need to make API of vue-advanced-cropper match to react-advanced-cropper that pretty different, but better in many aspects.

I will leave this issue open until I will think out something. Perhaps, until the new release will come.