feat(gatsby-transformer-sharp): add inside and outside fit options

[chris-hammond]

Description

Added INSIDE and OUTSIDE options for for gatsby-transformer-sharp. Here's a use case for inside #13078 (comment).
This includes changing gatsby-image to use contain instead of cover for object-fit, which does not affect the existing fit options.
Updated documentation for gatsby-transformer-sharp to clarify the fit options and show that they are available for all three functions.

NOTE: This is an extension of PR #13393 by @VGoose , which was closed because he didn't have time to complete the work.

Related Issues

Addresses #12972
Related to PR #13393

[marcysutton]

Hi there! Would you mind updating the Gatsby Image API doc as well? That page is where the options from multiple packages all come together (gatsby-image, gatsby-plugin-sharp and gatsby-transformer-sharp), and I think it would help in learning how this all works to keep that up to date with improvements. https://www.gatsbyjs.org/docs/gatsby-image

[chris-hammond]

Will do! Looks like both that and the gatsby-plugin-sharp readme need some cleanup unrelated to my changes...

[chris-hammond]

@marcysutton Did these changes to the Gatsby Image API look sufficient? I updated what was there, but it didn't seem like a good idea to add a new section for a deep dive on cropping options...

[marcysutton]

This looks like an improvement to me, at least from a docs perspective! Thanks for your work here. I think the functionality still needs a review from @gatsbyjs/core.

[chris-hammond]

Any update from the core team? I understand that you've got a ton of PRs, but I would love to get this merged before there are more conflicts with master... :-) Let me know if there's anything I can do to help move this PR forward!

Thanks!

[pieh]

Hey @chris-hammond

I don't quite follow why in gatsby-image we are changing object-fit here?

Also inside and outside in gatsby-plugin-sharp require more work than just passing this option to sharp.

We do need to calculate width/height for fixed/resize and aspectRatio for fluid differently. Right now our calculations assume that image will be cropped - but it's not cropped for inside and outside - it's scaled

[sidharthachatterjee]

We do need to calculate width/height for fixed/resize and aspectRatio for fluid differently. Right now our calculations assume that image will be cropped - but it's not cropped for inside and outside - it's scaled

From @pieh

[chris-hammond]

In the end, I wanted to make fixed(fit: OUTSIDE) render properly, and that meant implementing your suggestions for all of the INSIDE and OUTSIDE fits. I don't love it, as it means I'll have to do more work in my template files to USE the new fit options (since I can't use the bounding box to center-align the images in both dimensions), but I don't feel strongly enough to continue arguing my point.

I reverted all the changes from cover to contain in gatsby-image, more to reduce the size of this PR than because I think that's the best approach...

This should now be ready for review.

[chris-hammond]

@sidharthachatterjee Can you confirm that the requested changes have been made?

[pieh]

Ok this is great, me & @madalynrose tried to do extensive testing of this and just are not sure if fluid should even have those extra fit options. For resize and fixed they make perfect sense and they are working as expected.

See the screenshots:

portrait input image:

landscape input image:

square input image:

The "Outside" fit option also produces very weird sizes breakpoints than will often not align with maxWidth, meaning that they won't match with container's maxWidth which will result in fuzzy images (as browser need to pick one that's has width closest to container width and rescale it).

There doesn't seem to be difference between using "Inside" and just skipping maxHeight argument.

Is there scenario where having those fit options for fluid will allow to do something that users can't do now?

[chris-hammond]

The use case linked in the description from PR #13078 (comment) applies to both the fixed and fluid methods.

In all of your test examples, you are using source images with a "shorter" aspect ratio than the bounding box. Suppose instead that you have a square bounding box with a random assortment of portrait and landscape photos displayed in a grid. In this case, the "no height" option is no longer viable. This is exactly the situation that led me to author this PR in the first place; for responsive design reasons I am using fluid images, not fixed images, and thus definitely need the INSIDE/OUTSIDE options for fluid in addition to fixed!

I'm not sure what you mean about OUTSIDE fit resulting in fuzzy images; in the three screenshots above, the OUTSIDE fit image is the least fuzzy to my eye. This makes sense, as it' the highest-resolution image in each case.

[chris-hammond]

Looks like the label on this PR is out of date; I'm not aware of anything more needed from me prior to review.

Also, what is going on with the new Netlify "Deploy failed" messages? Is there something I should be doing to fix this?

Thanks!
-Chris

[chris-hammond]

Any update on this PR? @pieh or @madalynrose ?

I believe this should no longer be labeled as "awaiting author response", as I responded to the open questions.

Thanks!

[pieh]

My fault here.

Only thing missing to me is some concrete examples when one would want to use INSIDE or OUTSIDE option for fluid/fix variants (as in - what kinds of html/css layouts each combination is good for).

I have hard time coming up with those. It doesn't even have to be in this PR as long as we have information (can be in comment here) to follow up to document this properly

[pieh]

Also, what is going on with the new Netlify "Deploy failed" messages? Is there something I should be doing to fix this?

That's relict from old setup - you can safely ignore those

[GriffinJohnston]

@pieh a use case for inside fit that came up for me recently was an artist's website. The site has photo gallery pages with a grid of clickable artwork thumbnails. Images of the artwork could not be cropped, but came in a wide variety of aspect ratios, and needed to fit within the bounds of an n x n square.

More generally, after building a few Gatsby sites this year, I've found that I actually need inside fit more than any other option and think it should probably be the default. It's to the point where I've only been able to use gatsby-image for half or less of my image placements. Anyone who's had to hand CMS control over to a non-technical client knows that requiring CMS-uploaded images to fit a certain aspect ratio is usually unrealistic. In those circumstances, it's often good to be able to ensure that an image will fit within a maximum height and width, and cropping or letterboxing are commonly unacceptable (any time faces are involved, artwork, photos of a product, images with a logo in them, etc.)

Hope that helps!

[GriffinJohnston]

To be more concise: inside/outside are useful when the source image has an unpredictable aspect ratio, and cropping, letterboxing and stretching are all unacceptable.

The choice of fixed or fluid has more to do with the exact placement of the image and responsive requirements.

[chris-hammond]

Hi @pieh,

My use case is similar to @GriffinJohnston. I've got source images ranging from 1:2 aspect ratios to 2:1 aspect ratios, all of which need to fit in squares in a responsive layout. We're currently using fluid(INSIDE) to achieve this, and are happy with the results.

For non-grid applications of the same group of images (e.g. when using them for a product-detail page, rather than the product-list page), I switch to fixed(INSIDE).

We've been discussing switching from fluid(INSIDE) to fluid(OUTSIDE) for our gallery pages to help preserve the resolution of our more extreme aspect ratio images with less impact on the size of our images that are closer to a 1:1 aspect ratio, but we haven't made the jump yet.

Hope that helps!

[chris-hammond]

@sidharthachatterjee Can you confirm that the changes you requested have been implemented?

Thanks!
-Chris

[wardpeet]

Looks great! I'll build it on gatsby-cloud just to see a live preview of it and afterwards merge it.

Thanks @chris-hammond for adding this to gatsby and for your patient. This is solid! 💯

fixed

[gatsbot]

Holy buckets, @chris-hammond — we just merged your PR to Gatsby! 💪💜

Gatsby is built by awesome people like you. Let us say “thanks” in two ways:

1. We’d like to send you some Gatsby swag. As a token of our appreciation, you can go to the Gatsby Swag Store and log in with your GitHub account to get a coupon code good for one free piece of swag. We’ve got Gatsby t-shirts, stickers, hats, scrunchies, and much more. (You can also unlock even more free swag with 5 contributions — wink wink nudge nudge.) See gatsby.dev/swag for details.
2. We just invited you to join the Gatsby organization on GitHub. This will add you to our team of maintainers. Accept the invite by visiting https://github.com/orgs/gatsbyjs/invitation. By joining the team, you’ll be able to label issues, review pull requests, and merge approved pull requests.

If there’s anything we can do to help, please don’t hesitate to reach out to us: tweet at @gatsbyjs and we’ll come a-runnin’.

Thanks again!

[GriffinJohnston]

@chris-hammond Thanks so much for making this happen!