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

Feature image crop #3380

Merged
merged 8 commits into from
Jul 31, 2018
Merged

Feature image crop #3380

merged 8 commits into from
Jul 31, 2018

Conversation

bricesanchez
Copy link
Member

@bricesanchez bricesanchez commented Jul 13, 2018

  • Use Cropper.js lib (https://github.com/fengyuanchen/cropperjs)
  • No dependency to jQuery
  • Add Ability to write multiple crops in Admin Image Edit form
  • Use pre-defined or custom crop ratios
  • Add Ability to use crop in WYSIWYG editors and in Image insert dialog

screencapture-localhost-3000-refinery-images-7-edit-2018-07-12-22_14_18

screencapture-localhost-3000-refinery-pages-test-edit-2018-07-12-22_15_00

screencapture-localhost-3000-refinery-pages-test-edit-2018-07-12-22_15_24

No tests added for this feature, sorry :(

@parndt
Copy link
Member

parndt commented Jul 13, 2018

That's pretty exciting @bricesanchez

Copy link
Member

@parndt parndt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you say it works in all the major browsers, I believe you. 😄

@parndt
Copy link
Member

parndt commented Jul 13, 2018

keen for @anitagraham to take a look


<% content_for :stylesheets do %>
<%= stylesheet_link_tag('refinery/plugins/cropper') %>
<% end %>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if it's just one, you can normally in-line it:

<% content_for :stylesheets, stylesheet_link_tag('refinery/plugins/cropper') -%>

@bricesanchez
Copy link
Member Author

bricesanchez commented Jul 17, 2018

@parndt I've made some change to support a message if there is no crop. Could you review it please?

</ul>
<% if @image.crops.present? %>
<ul id="crop-list" class="clearfix">
<% @image.crops.each do |crop| %>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does <%= render @image.crops %> work here?

Or

<%= render partial: 'crop', collection: @image.crops

I think Rails handles this with syntax like one of the above, provided it can find the partial.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it works :)

@bricesanchez
Copy link
Member Author

Hi @anitagraham, do you have the time to check this PR?

@bricesanchez
Copy link
Member Author

With this latest commit, this feature works well on all major browsers including Edge :)

@bricesanchez
Copy link
Member Author

Could we merge it @parndt?

@parndt
Copy link
Member

parndt commented Jul 31, 2018

Yep 😄

@bricesanchez bricesanchez merged commit d1f9faf into master Jul 31, 2018
@bricesanchez bricesanchez deleted the feature/image-crop branch July 31, 2018 11:45
@anitagraham
Copy link
Contributor

This looks great. I am sorry I wasn't around to help review it - about to use it now, instead.

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

Successfully merging this pull request may close these issues.

None yet

3 participants