cloudinary-community / next-cloudinary

⚡️ High-performance image delivery and uploading at scale in Next.js powered by Cloudinary.
https://next.cloudinary.dev
MIT License
251 stars 71 forks source link

[Feature] Picture Tag #42

Open colbyfayock opened 1 year ago

colbyfayock commented 1 year ago

Feature Request

Is your feature request related to a problem? Please describe.

The library currently supports CldImage giving developers a way to use the Next.js Image component extended with Cloudinary.

An alternative image-related element is the picture element which allows for different capabilities.

Having this would allow developers to use Cloudinary in a way that makes more sense for Picture-related usage.

Describe the solution you'd like

A basic usage pulled from MDN is:

<picture>
    <source
        srcset="/media/cc0-images/surfer-240-200.jpg"
        media="(min-width: 800px)">
    <img src="/media/cc0-images/painted-hand-298-332.jpg" alt="">
</picture>

Theoretically, it would make sense that the <img above would be the <CldImg, where <picture becomes a wrapper with an additional <source element, leaving something along hte lines of:

<CldPicture>
    <CldSource
        srcset={[{ src: 'cc0-images/surfer', width: 240, height: 200 }]}
        media="(min-width: 800px)"
    />
    <CldImg width="298" height="332" src="cc0-images/painted-hand" alt="" />
</CldPicture>

Where inside we would use constructCloudinaryUrl to build an image URL

This is just an idea and am open to ideas and suggestions.

harry-gocity commented 6 months ago

Just come across the need for this exact api. We have different images to show at different breakpoints, and right now are relying on using window.matchMedia to determine the id to pass to CldImage. A CldPicture and CldSource would make this much simpler to do.

Assuming a lot has changed in this library in the last 18 months, do you know how easy this would be to support @colbyfayock ?

harry-gocity commented 6 months ago

I've found that I can get this behaviour with getCldImageUrl and standard img / picture / source elements - roughly like:

 <picture>
    <source
        srcSet={getCldImageUrl({ src: 'gc-v1/static/404-lg' })}
        media={`(min-width: ${theme.viewport.lg})`}
    />
    <source
        srcSet={getCldImageUrl({ src: 'gc-v1/static/404-md' })}
        media={`(min-width: ${theme.viewport.md})`}
    />
    <img
        src={getCldImageUrl({ src: 'gc-v1/static/404-sm' })}
        alt=""
    />
</picture>
colbyfayock commented 6 months ago

im actually not familiar with matchMedia, will need to look into that

But still haven't quite thought through how exactly id want to approach this

you're right in that example that you shared works well, this is how Next.js is currently handling it: https://github.com/vercel/next.js/discussions/19880#discussioncomment-6310787

but that doesn't handle responsive sizing within the bounds of the sources

With a little exploration on potential options for supporting this...

Unpic handles this with Source components, but not a Picture component: https://unpic.pics/blog/introducing-unpic-picture-tags/

it feels weird nesting a CldSource inside of a native <picture (at least to me), but there wouldn't be any functionality that would make sense in creating a CldPicture, which is likely why they didnt do it that way

Nuxt has a NuxtPicture, but my understanding is it's really meant for formats, and the syntax is encapsulated into the main component, not sources like Unpic: https://image.nuxt.com/usage/nuxt-picture

Astro similarly does what NuxtPicture does, really relying on it only for formats: https://docs.astro.build/en/guides/images/#picture-

I think as far as this library goes, i like the idea of supporting a more native-like syntax which gives people the freedom to create the sources as they'd like, so that means <picture><CldSource. I would expect the other frameworks (Nuxt) may be more inclined to follow the same pattern as the official Nuxt package, but worth a discussion, especially given the functionality is limiting in the current solution

the only trouble i really see is not being able to tap into the responsive sizing from Next.js that the CldImage component is currently able to. given this component wraps Next Image, it's able to utilize the built-in resizing, i wonder if it was intentional not to include responsive sizing in their solution 🤔

so following along with your example, i think the solution that i could support, without writing my own responsive size generator, would look along the lines of:

 <picture>
    <CldSource
        width
        height
        srcset={[{
            src: '<Lg Public ID>',
            ...transformations
        }]}
        // or src={['<Lg Public ID>']}
        media={`(min-width: <Lg Size>`}
    />
    <CldSource
        width
        height
        srcset={[{
            src: '<Md Public ID>',
            ...transformations
        }]}
        // or src={['<Md Public ID>']}
        media={`(min-width: <Md Size>`}
    />
    <CldImage
        width
        height
        src="<Public ID>"
        alt=""
    />
</picture>

perhaps srcset could also support a property such as size to allow it to correlate to a sizes property:

 <picture>
    <CldSource
        width
        height
        srcset={[
            {
                src: '<Lg Public ID>',
                ...transformations,
                size: '256w'
            },
            ....
        ]}
        sizes="(max-width: 768px) 100vw, (max-width: 1200px) 50vw, 33vw"
        media={`(min-width: <Lg Size>`}
    />
</picture>

i'll put together an RFC for this and share ti in the Discussions area to potentially gain more thoughts on it in that forum

charliemourant commented 1 month ago

Hey @colbyfayock, did you ever make in progress with this? The further we dive into responsive images the more it feels like an API like this would be extremely useful.

colbyfayock commented 1 month ago

hey @charliemourant i hadn't thought much about this since the last post, lost track of this while i was working on other things. i'll work on putting together an RFC for this, as i do think it would be helpful to have in some form, just want to make sure we do it the right way. thanks for resurfacing this

colbyfayock commented 1 month ago

hey all, created an RFC here, would love some thoughts: https://github.com/cloudinary-community/next-cloudinary/discussions/511