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

some minor updates #6

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

some minor updates #6

wants to merge 4 commits into from

Conversation

musaffa
Copy link

@musaffa musaffa commented Oct 8, 2014

  • In my project I use this gem to crop images even before being uploaded. So I've changed before_update to before_save which makes it more generic. Now it works before upload and update after upload.
  • For naming consistency hidden_fields should contain crop_box. I've also replaced string with ActiveSupport::SafeBuffer to loop through the crop fields.

@kirtithorat
Copy link
Owner

@musaffa Thanks for the input. Appreciate it.

Got a question for you. When you say "I use this gem to crop images even before being uploaded...", how exactly are you implementing it? How do you retrieve that image before uploading it for cropping. Also, are you considering versions of the image to be uploaded and cropped? My original intent of this gem is to crop the uploaded images which is why I have used before_update .

Regarding the naming suggestion, I am going to stick with the current field names prefixed with crop_ because they are related to the image (dimensions) and not related to the dimensions of cropbox .

As for the ActiveSupport::SafeBuffer suggestion goes, I would like to merge it with the current code but for that you would need to revert back the hidden field naming change to the original naming and submit again.

@musaffa
Copy link
Author

musaffa commented Oct 29, 2014

ok i've removed cropbox suggestion.

To retrieve image before upload, you can use javascript's File Api. Just search 'javascript preview an image before it is uploaded' in google. There are tons of examples there. As soon as an image tag is created for a preview, an event handler should catch that change in dom and apply jcrop on it.

@kirtithorat
Copy link
Owner

@musaffa Thanks a lot! Could you please rebase and squash these commits after reverting the callback change. I think once that is done this should be good to go! Also, regarding the part of cropping images even before they are uploaded, may be you could think of opening it as a new pull request and we will treat it as a new feature or something.

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

Successfully merging this pull request may close these issues.

2 participants