[feature]: Add height to makeUrl and enforce crop in middleware if provided

[sirugh]

Description

The product images from sample data are 25:31 ratio.
The placeholder image we use is 4:5.

This is a difference of only a few pixels but it leads to a poor UX where we sometimes have a placeholder image that is too large (ie. on the category page) or we have a UI that "shifts" down/up when we render a placeholder and then replace it with the product image (ie. on the pdp).

I made a change here to the makeUrl to accept a height param. I also added some logic to our middleware to now enforce crop if both height and width are provided.

To understand these changes consider the following. Say we have an image with ratio 25:31 (as is our sample data) and we want to size it. The following now can happen:

a) Using width=640, sharp and fastly will scale to 640x796 because they retained the aspect ratio.
b) Using width=640&height=800, fastly will scale to 640x800 but sharp will scale to 645x800 because of the fit: outside prop used. Now, with these changes sharp will scale and then crop to cover the provided height and width.

If a user wants to retain the aspect ratio of the image using either fastly or our express-sharp, they can still just pass width.

Read more here:
sharp resize
fastly

Related Issue

Closes #1359 .

Verification Steps

1. Go to the category page and notice that the images are sized correctly and there is no overflow from the placeholder!
2. Update your backend url to one of our cloud instances with fastly enabled. I can give you a URL if you need one. Then set IMAGE_OPTIMIZING_ORIGIN="backend" in your .env and re-run your server.
3. Go back to the category page and images should still be sized correctly.

How have YOU tested this?

I made a change to the category page GalleryItem which is only one implementation of makeUrl. Ideally we would just us these changes elsewhere when we identify a problem.

Screenshots / Screen Captures (if appropriate):

In the BEFORE screenshot notice the grey rectangle below the product images.
BEFORE

AFTER

Proposed Labels for Change Type/Package

• major (e.g x.0.0 - a breaking change)
• minor (e.g 0.x.0 - a backwards compatible addition)
• patch (e.g 0.0.x - a bug fix)

Checklist:

• I have updated the documentation accordingly, if necessary.
• I have added tests to cover my changes, if necessary.

[vercel]

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

Latest deployment for this branch: https://venia-git-fix-image-resize-height.magento-research1.now.sh

[PWAStudioBot]

	Messages
📖	We are currently working on automating the PR metadata checks. Until that time, you may see failures related to labels/description/linked issues/etc even if you have fixed the problem. Failures will persist until the next push (assuming they are fixed).

Generated by 🚫 dangerJS against 9fb64a0

[zetlen]

The URLs made by makeUrl need to adhere to the Fastly image opto query parameter API. That's why we have the rewrite function in addImgOptMiddleware that transforms makeUrl URLs into the kind that express-sharp can understand.

The reason we follow Fastly's image opto API is that we want our customers to be on Enterprise Cloud and to have Fastly IO enabled; that should be the "happy path" and the simplest thing. In that simplest case, Fastly takes over all the image manipulation (and it's super fast at it). Since Fastly is an external service and our express-sharp implementation is an internal one, we can freely mess with the addImgOptMiddleware public API.

So you just gotta change so makeUrl outputs a crop parameter adhering to the Fastly spec linked above. It can be internal to makeUrl if you want.

Test it by connecting your local instance to one of our Fastly-enabled Enterprise Cloud shops. Then set the env var IMAGE_OPTIMIZING_ORIGIN=backend and it'll render media URLs at the attached store base domain, instead of your local. Then you should see Fastly in operation (and you can confirm it by looking at response headers).

[sirugh]

Looks like Fastly is actually a good citizen when it comes to resizing. When you give a height and width that don't match the aspect ratio they must resize the image automatically whereas express-sharp retains the aspect ratio and matches the latter param (height). So what I need to do is figure out if there is a way to get express-sharp to resize in the same way that fastly does (looks like crop is the answer right now).

[tjwiebell]

Should be no need to throw error, then this looks good to me.

[sirugh]

@tjwiebell addressed feedback. Please move to Ready for QA if it looks good!

[dpatil-magento]

@sirugh Other than below observations rest all looks good.

1. Image size is now 300x375 vs 300x372 on develop branch.
2. Horizontal border between image and the product name is missing on this branch compared to develop

[sirugh]

Image size is now 300x375 vs 300x372 on develop branch.

Expected, we are now requesting height and width. Glad you caught it though ;)

Horizontal border between image and the product name is missing on this branch compared to develop

So that's not actually an intentional border. It is the overlap of the placeholder that is sized slightly too large and ends up showing a bit below the full image. The fact that it is no longer visible is intentional, and is in fact one of the primary reasons I wrote this PR :)