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

Proposal: Remove cropSize prop or allow null Override #194

Closed
lgants opened this issue Aug 11, 2022 · 4 comments
Closed

Proposal: Remove cropSize prop or allow null Override #194

lgants opened this issue Aug 11, 2022 · 4 comments

Comments

@lgants
Copy link

lgants commented Aug 11, 2022

The cropSize prop can become very problematic for images with certain dimensions - equidistant height/width works well, but others can be problematic. The react-easy-crop documentation on cropSize states (in bold text): "You should probably not use this option and should rely on aspect instead. See ValentinH#186". For example, the cropper overlay for some portrait images is zoomed in so far that this great library is essentially unusable 🙁. What are your thoughts on moving the spread cropperProps to the last position rather than the first - thereby allowing overrides? Seems like a very quick fix. I can raise a quick PR if so.

Screen Shot 2022-08-10 at 9 59 04 PM

@lgants
Copy link
Author

lgants commented Aug 11, 2022

Also, forgot to mention that I'm using version 4.2.3 and here's the relevant code

<ImgCrop
  modalWidth={416}
>
 ...

@nanxiaobei
Copy link
Owner

cropSize is a very core prop, and the impact of deletion is uncontrollable.

@lgants
Copy link
Author

lgants commented Aug 11, 2022

hmm is there any willingness to support an override/escape hatch of some sort? My application needs to support user generated content, including brand logos. And, as you can see from the screen shoot, that is exceedingly hard/borderline impossible with this library as currently configured. I would think others might have similar needs.

Also, I'm not saying cropSize should be removed. I just users should have the ability to override. Does that make sense?

Thanks for the timely response btw!

@nanxiaobei
Copy link
Owner

There is not enough time and sufficient reason to make this modification.
Maybe it will be added later, and of course, PR welcome.

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