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

Fixes image cropping #1321

Merged
merged 7 commits into from
Nov 5, 2017
Merged

Conversation

tvdeyen
Copy link
Member

@tvdeyen tvdeyen commented Nov 3, 2017

Includes #1320
Closes #1279

Content's settings - which we use to merge with the default EssencePicture `picture_url` options - are a `HashWithIndifferentAccess`. But the default url options were a simple `Hash` instead. Merging these two leads to url options having the `format` and `size` keys duplicated once as `String` once as `Symbol`. By using a `HashWithIndifferentAccess` for the `picture_url_options` we fix that.
Content's settings - which we use to merge with the EssencePictureView default options - are a `HashWithIndifferentAccess`. But the default options were a simple `Hash` instead. Merging these two leads to url options having the `format` and `size` keys duplicated once as `String` once as `Symbol`. By using a `HashWithIndifferentAccess` for the `DEFAULT_OPTIONS` we fix that.
An image can always be cropped if upsample is set to true. We do not need to make the sizes from string conversion on each render of an essence picture when we know it can be upsampled.
EssencePicture crop values (`crop_from` and `crop_size`) should not be converted to a string if they are not set (`nil`) on save.
Having empty strings as EssencePicture crop value (`crop_from` or `crop_size`) causes imagemagick to not crop the image but resize only.

Fixes AlchemyCMS#1279
Copy link
Contributor

@robinboening robinboening left a comment

Choose a reason for hiding this comment

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

I have nothing to complain about. Thanks for your work!

Only if the crop option is set explicitly to true we crop the image, even if `crop_from` or `crop_size` values are given.

This is a change to current behavior where the image was always cropped if `crop_from` or `crop_size` values were given, even if `crop` was set to `false` or left `nil`. This is a very confusing behavior.

Fixes AlchemyCMS#934
@tvdeyen tvdeyen merged commit f170b95 into AlchemyCMS:master Nov 5, 2017
@tvdeyen tvdeyen deleted the fixes-image-cropping branch November 5, 2017 19:39
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