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

Specifying both width and height incorrectly applies rect #32

Open
coreyward opened this issue Feb 12, 2021 · 2 comments
Open

Specifying both width and height incorrectly applies rect #32

coreyward opened this issue Feb 12, 2021 · 2 comments

Comments

@coreyward
Copy link

This library incorrectly sets the rect parameter when both width and height are specified. This prevents many fit modes from working properly. An in-depth visual comparison and breakdown of this bug is available in this CodeSandbox project. At a glance, this screenshot from the linked breakdown demonstrates the issue:

Screen Shot 2021-02-12 at 4 25 13 PM

I considered stripping the rect prop from the returned urls, but it still needs to be present in two scenarios:

  1. When a source has an associated crop
    The rationale here should be self-evident: rect is used to apply crop values.
  2. When the fit mode is min and the source has a focal point
    This library transforms hotspot values into focal point parameters fp-x and fp-y, but due to a bug/oversight in the Images API, these parameters are only used when the fit mode is set to crop. Since the min mode is identical to crop except that the image will not be scaled up, the focal point needs to be emulated via rect (or the bug in the Images API needs to be fixed)

I believe this library should only add a rect parameter in these two scenarios, and to try to pre-compute and apply the focal point in the second case.

coreyward added a commit to coreyward/gatsby-plugin-sanity-image that referenced this issue Mar 3, 2021
Due to sanity-io/image-url#32, setting a
`height` prop will cause unexpected behavior.
@johnnywalker
Copy link

Since the API is modeled after imgix's API, I believe rect is calculated incorrectly when determined from a source with crop dimensions. Sanity's API appears to treat rect as the source rectangle, and urlForImage.ts constrains the source's crop to the final aspect ratio instead.

Reference: https://docs.imgix.com/apis/rendering/size/rect

@modulareverything
Copy link

Sharing this here as it solved the problem for me:

#48 (comment)

In summary, try adding ignoreImageParams() to your image URL like so:

imageUrlBuilder.image(source).ignoreImageParams().width(300).height(50).fit('clip').url()

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

No branches or pull requests

3 participants