Closed fernandolucchesi closed 1 year ago
Hi! thanks for sharing your solution. I'm just wondering, what and why you're doing
const { imageProps, objectPosition } = getImageProps(image)
the solution works for me, except, that it generates multi srcsets with the same size. feels like this could be the missing piece?
also, I got it to work the way it's described in the docs. just make sure you pass the image source and not only asset and it automatically respects the hotspot. but same issue with the srcset for me with that.
Hi! thanks for sharing your solution. I'm just wondering, what and why you're doing
const { imageProps, objectPosition } = getImageProps(image)
the solution works for me, except, that it generates multi srcsets with the same size. feels like this could be the missing piece?
also, I got it to work the way it's described in the docs. just make sure you pass the image source and not only asset and it automatically respects the hotspot. but same issue with the srcset for me with that.
Ops, that is a bug in the code, I had the code extracted as an outer function before, but then edited my comment/code and forgot to get rid of that line, thank you for pointing it out, I will edit it. As for the srcset, you are right, seems this solution is not 100% correct yet :|
I just investigated this further and it seems correct.
This has to do with the way Next handles the srcSet. Basically, if you start from a large viewport, it will download the large image and use it for all other srcSets, because there is no need to download a smaller resolution image for smaller screens if the large one is already downloaded, that is why all the URLs are the same.
But if you start from a small viewport and resize it bigger, Next will download the better resolution images (you can see this happening through the Inspect Element -> Network tab). And if you refresh the page, the URLs will get updated.
I just investigated this further and it seems correct.
This has to do with the way Next handles the srcSet. Basically, if you start from a large viewport, it will download the large image and use it for all other srcSets, because there is no need to download a smaller resolution image for smaller screens if the large one is already downloaded, that is why all the URLs are the same.
But if you start from a small viewport and resize it bigger, Next will download the better resolution images (you can see this happening through the Inspect Element -> Network tab). And if you refresh the page, the URLs will get updated.
yes you're right, i noticed that too, but it's still not how its supposed to be though. I just couldn't figure out if it's a issue related to this module or next/image
I just investigated this further and it seems correct.
This has to do with the way Next handles the srcSet. Basically, if you start from a large viewport, it will download the large image and use it for all other srcSets, because there is no need to download a smaller resolution image for smaller screens if the large one is already downloaded, that is why all the URLs are the same.
But if you start from a small viewport and resize it bigger, Next will download the better resolution images (you can see this happening through the Inspect Element -> Network tab). And if you refresh the page, the URLs will get updated.
Landed on another solution that seems to be working better (looks like the responsive srcset doesn't work with the approach above 🤷♂️ ). Probably a lot of over-engineering :
import { useNextSanityImage } from 'next-sanity-image'
import Img, { ImageProps } from 'next/image'
import { Image } from '@types'
import client from 'client'
type Props = Omit<ImageProps, 'src' | 'alt'> & {
image: Image
maxWidth?: number
aspectRatio?: number
}
export enum Ratios {
THREE_TO_TEN = 0.3,
NINETEEN_TO_FORTY = 0.475,
ONE_TO_TWO = 0.5,
NINE_TO_SIXTEEN = 0.5625,
THREE_TO_FOUR = 0.75,
FOUR_TO_FIVE = 0.8,
ONE_TO_ONE = 1,
}
const DEFAULT_SIZES = '(max-width: 800px) 100vw, 800px'
const DEFAULT_MAX_WIDTH = 1440
const useSanityLoader = (
image: Image,
maxWidth: number,
aspectRatio: number | undefined,
) =>
useNextSanityImage(client, image, {
imageBuilder: (imageUrlBuilder, options) => {
const { width: imageWidth, croppedImageDimensions: cropped } = options
// We do not want to allow gigantic images to exist due to performance
const width = Math.round(imageWidth || Math.min(maxWidth, cropped.width))
const height = aspectRatio
? Math.round(width * aspectRatio)
: Math.round(width * (cropped.height / cropped.width))
return imageUrlBuilder.width(width).height(height).quality(70)
},
})
const Image = ({
image,
aspectRatio,
sizes = DEFAULT_SIZES,
maxWidth = DEFAULT_MAX_WIDTH,
fill,
style,
...rest
}: Props) => {
const imageProps = useSanityLoader(image, maxWidth, aspectRatio)
if (!image?.asset) return <></>
const { width, height, src } = imageProps
let props = {}
if (fill) {
// Layout fill
props = {
fill,
style: { ...style, objectFit: 'cover' },
}
} else {
// Layout responsive
props = {
width,
height,
style: { ...style, width: '100%', height: 'auto' },
}
}
return (
<Img
{...props}
{...rest}
src={src}
alt={image.alt ?? ''}
sizes={sizes}
/>
)
}
export default Image
Usage:
{data.image && (
<Image
sizes="100px"
className="rounded-xl"
aspectRatio={Ratios.ONE_TO_TWO}
image={data.image}
/>
)}
Result:
And using sizes="800px" instead of 100px:
The big con of this approach IMO: Cannot use CSS media queries to control image width and height, since it is being controlled by the aspect ratio prop.
After trying for a long time to make the crop and hotspot work, I finally landed on a solution.
This ticket aims to help others facing the same issue, but I also believe that the docs should be updated accordingly.
For the hotspot I managed to get it working using the solution by @danieljb https://github.com/bundlesandbatches/next-sanity-image/issues/32#issuecomment-1016298735
And for the crop, I got it working by changing the default
fit
behavior from the lib to'crop'
:Based on this solution, I created this generic Image component that is working perfectly for me so far:
Hopefully this will help others :)