Skip to content
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

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

Open
kevpug opened this issue Aug 27, 2022 · 5 comments
Open

Comments

@kevpug
Copy link

kevpug commented Aug 27, 2022

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
Copy link
Collaborator

@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
Copy link
Collaborator

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

@Norserium
Copy link
Collaborator

Norserium commented Aug 27, 2022

@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
Copy link
Author

kevpug commented Aug 27, 2022

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
Copy link
Collaborator

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants