Improved Image Fit with Fastly #1976
Conversation
|
| @@ -15,6 +15,7 @@ | |||
| display: block; | |||
| height: 5rem; | |||
| object-fit: cover; | |||
| object-position: center; | |||
There was a problem hiding this comment.
This isn't strictly necessary I suppose, the initial value of 50% 50% will have the same result.
There was a problem hiding this comment.
Unless you think it's better for readability (ie. the common developer doesn't know centering is the default), let's remove it. Not going to block my approval though.
| @@ -21,7 +21,8 @@ const generateURL = (imageURL, mediaBase) => (width, height) => { | |||
| const url = resourceUrl(imageURL, { | |||
| type: mediaBase, | |||
| width, | |||
| height | |||
| height, | |||
| fit: 'cover' | |||
There was a problem hiding this comment.
This is the change that actually fixes the issue.
supernova-at
added
the
version: Patch
| @@ -15,6 +15,7 @@ | |||
| display: block; | |||
| height: 5rem; | |||
| object-fit: cover; | |||
| object-position: center; | |||
There was a problem hiding this comment.
Unless you think it's better for readability (ie. the common developer doesn't know centering is the default), let's remove it. Not going to block my approval though.
Description
tl;dr
This PR adds a
fit=coverimage optimization parameter to thesrcseturls of resource images. This will be ignored byexpress-sharp, which already produces the intended behavior, but will instruct Fastly to resize the image to entirely cover the givenwidth/heightdimensions.See https://docs.fastly.com/api/imageopto/fit for more details.
Overview
The
CategoryTileimages on the homepage of our staging site (upward-php + fastly) did not look good:Compare that to the latest
develop(upward-js):Investigation
We were sending the same image resize parameters (
width=80,height=100) in both places but getting much different responses back:upward-php + fastly:
80x100
upward-js:
178x100
Resolution
This is because our onboard image optimization middleware (powered by
express-sharp) was handling things differently than Fastly when bothwidthandheightresize parameters were present. Since we wanted Fastly to emulate theexpress-sharpway, this PR updates the urls in resource imagesrcsets to includefit=cover, which will resize the image to entirely cover the givenwidth&heightdimensions.Support for this property was already present in our makeUrl utility, with the following comment:
See also https://docs.fastly.com/api/imageopto/fit.
Related Issue
Closes PWA-159.
Acceptance
Verification Stakeholders
@tjwiebell @zetlen
Specification
Verification Steps
Screenshots / Screen Captures (if appropriate)
Checklist