Allow rectangular product images

[BenSturmfels]

Hi Folks,

It looks as though commit 20a67e7 removed the ability to have rectangular product images (rather than the square defaults). The get_thumbnail tag used to support a size like 100x200, eg.

{% get_thumbnail product.get_first_image method="thumbnail" size="100x200" %}

Now it only takes a single size integer and automatically squares it as in the following, meaning that the above usage fails.

{% get_thumbnail product.get_first_image method="thumbnail" size=255 %}

From a design perspective, having rectangular images was a useful feature, since photographers tend to set up their shots to get the best out of a rectangular frame, similarly most mobile devices have rectangular screens.

Regards,
Ben

[BenSturmfels]

This is made harder still by the Javascript in static/js/components/misc.js:36 that applies CSS to force the image height to be set to the width:

$('.product-image').css({'height': productImageWidth + 'px'});

And by switching from method crop to thumbnail (see above), rectangular images now don't thumbnail to a consistent size, making it fairly easy to mess up a float layout.

[patrys]

Dashboard code will center any image within a square box and that's by design as we have to accommodate all sorts of ratios there. For your own storefront you are not limited to any shape. The size param specifies the limit on the longer side, we don't crop (or at least we shouldn't, if we do it anywhere, it's a bug) images so the resulting thumbnail is going to have the same aspect ratio as the original.

[BenSturmfels]

@patrys, sorry this is relating to the frontend only. I think I understand it a bit better now. When I set my settings and template to "thumbnail" it works and I get a rectangular image.

The issue there is that "thumbnail" doesn't produce consistent heights for rectangular images, which means lots of CSS fiddling and media queries to produce neat rows of images. That's why I'd switched back to "crop", but after #2496, crop seems to only produce square images for me.

[patrys]

We try to avoid crop as it can do more harm than good, chopping off parts of the product in the process.

[BenSturmfels]

Preferring thumbnail is fine, it's just confusing that there's a method argument to the get_thumbnail template tag which really only works properly for the thumbnail method.

Perhaps the method argument should be dropped? At least this would send a clear signal that if you want something else, you need to do it yourself (as I've reallised I have to do in this case).

[maarcingebala]

I agree - we should drop the method argument to avoid confusion.

[NyanKiyoshi]

Looks like it's safe to drop, there is no code using crop for thumbnails. Maybe we could add a constant setting the default method to thumbnail? (e.g.: on my production fork, we use a custom method than thumbnail, which required code refactoring).

[raybesiga]

Just to get this straight @BenSturmfels how did you re-implement the rectangular dimensions for the images? I am stuck with the square dimensions as in the example below:

<div class="carousel-item{% if forloop.first %} active{% endif %}">
                  <div class="product-image">
                    <img class="d-block img-fluid lazyload lazypreload"
                         data-src="{% get_thumbnail image.image method="thumbnail" size=540 %}"
                         data-srcset="{% get_thumbnail image.image method="thumbnail" size=540 %} 1x, {% get_thumbnail image.image method="thumbnail" size=1080 %} 2x"
                         alt=""
                         src="{% placeholder size=540 %}">
                  </div>
                </div>

How might i go about having images with 400x600 dimensions with this change? I have been struggling with this. Any help will be appreciated! @patrys @maarcingebala @NyanKiyoshi

[BenSturmfels]

@raybesiga I've attached a patch showing how I did it. I've modified this by hand to remove some irrelevant bits, so it probably won't apply cleanly with patch. You'll get the idea anyway. I modified the template tag to allow it to take width/height, passed the height across in the template, and removed the "product-image" class that the JavaScript code was looking for and automatically making the image square.

rectangular-images.diff

[NyanKiyoshi]

Can you test on latest master without the patch? This commit (November 2018) should allow getting rectangular sizes.

[raybesiga]

Hi @NyanKiyoshi let me update my git log and pull. Will revert with the outcome. Thank you.

[patrys]

@raybesiga If you specify 600 as size, it should give you 400×600 assuming that the uploaded aspect ration is correct. We no longer crop any images when creating thumbnails.

[raybesiga]

@patrys how does it know to make the image 400x600 if I only specify the size as 600? Is there a way to make an explicit declaration, say 400x600? I ask this because it may not always be the case that the height of an uploaded image is 600. Also, when we declare the size, is that strictly for the height?

[patrys]

@raybesiga It will never crop any images so it will calculate the shorter dimension based on the longer dimension (the size you give) and the original aspect ratio. Square images will remain squares, 16:9 images will remain 16:9, and so on.

[raybesiga]

Oh wow. This is fabulous @patrys Just pulled and now installing the required Python and Node packages. Will let you know how it goes.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.