nuxt-modules / cloudinary

Cloudinary Module for Nuxt
https://cloudinary.nuxtjs.org
MIT License
245 stars 31 forks source link

Missing effect options #119

Closed colbyfayock closed 11 months ago

colbyfayock commented 1 year ago

Missing what i'm expecting to be a lot of the effects props.

The ones I've tried so far:

Version

@nuxtjs/cloudinary 2.0.0-rc.3 nuxt: 3.5.2

Reproduction Link

Not a link, but simply the code below in a fresh Nuxt app, let me know if you need an actual link for now.

<CldImage
      src="images/turtle"
      width="600"
      height="600"
      crop="fill"
      :pixelate="true"
    />

And the URL is confirmed not including the option:

/image/upload/c_fill,w_600,h_600,g_auto/f_auto/q_auto/v1/images/turtle?_a=E

colbyfayock commented 1 year ago

this could "technically" be considered an enhancement but the documentation currently reads that these things are possible and from an MVP perspective the hope was that all of these would work out of the box 🤷

Baroshem commented 1 year ago

@colbyfayock

I have just tested the effects and they are working like following:

<CldImage
      src="images/turtle"
      width="600"
      height="600"
      crop="fill"
      :effects="[{ pixelate: true }]"
    />

https://stackblitz.com/edit/github-9tztxo?file=.stackblitz%2Fapp.vue

colbyfayock commented 1 year ago

that option, along with the others, are supposed to be available at the top level per the example i shared

effects is intended to give the ability to apply a series of effects, not the prime mechanism

Baroshem commented 1 year ago

@colbyfayock

I know why it is not being passed. Basically, I am creating the props from the following interface:

interface AssetOptions {
    assetType?: string;
    crop?: string;
    deliveryType?: string;
    effects?: Array<any>;
    flags?: Array<string> | object;
    format?: string;
    gravity?: string;
    height?: string | number;
    overlays?: Array<any>;
    quality?: number | string;
    rawTransformations?: string[];
    removeBackground?: boolean;
    sanitize?: boolean;
    resize?: AssetOptionsResize;
    seoSuffix?: string;
    src: string;
    text?: any;
    transformations?: Array<string>;
    underlay?: string;
    underlays?: Array<any>;
    version?: number | string;
    width?: string | number;
    widthResize?: string | number;
    zoom?: string;
}

That I am using from you cloudinary loader and it does not include the props options for the previously mentioned effects. Because of that, even when user passes a correct prop for pixelate, it is still not being used as it is not part of this interface. In the NextCloudinary, I can only see the effect props of pixelate being mentioned as part of the effects array AFAIK.

What we can do here is we can leave the option to pass the effects as an array or I can add manually few props for the effects you mentioned but this option can cause some potential conflicts.

Let me know what you think about it.

colbyfayock commented 1 year ago

it sounds like its a probelm with the underlaying types i have set up

we can always release the first version without it, ill work to get this fixed, and then it should "just work" once its working on my end, is that right?

colbyfayock commented 1 year ago

fwiw, there is AssetOptions but there's also ImageOptions which should be used for the image component

ImageOptions inherits from AssetOptions, but adds Image specific stuff, which right now is really just panloop i think

Baroshem commented 1 year ago

image

This should work but due to the issues with Vue 3.3 and now with Nuxt 3.5 I cannot use the type imported from the library. So I needed yo make a local copy of it. But in the future this should be the way to go.

I will check out the imageProps you mentioned and then decide what to do next with this issue.

Baroshem commented 1 year ago

I checked the image props:

interface ImageOptions extends AssetOptions {
    zoompan?: string | boolean | ImageOptionsZoomPan;
}

I don't think that I can use it right now as it contains only zoompan AFAIK.

So we have two options:

  1. Add effects manually in the Nuxt Cloudinary package based on Next Cloudinary
  2. Do not implement this functionality for now (as it can be added by the effects array)

Let me know which option do you prefer :)

colbyfayock commented 1 year ago

one clarification, ImageOptions extends AssetOptions, meaning all the properties of AssetOptions apply to ImageOptions. the idea is that AssetOptions is a core type definition that have shared rules for both Image and Video (VideoOptions), which isnt applicable yet here

id say we can just put a hold on this for now (#2) and come back to it once i fix the underlaying package

colbyfayock commented 1 year ago

created a ticket to track this: https://github.com/colbyfayock/cloudinary-util/issues/53

Baroshem commented 1 year ago

Ok, I have added it to the 2.1 release plan :)

Baroshem commented 12 months ago

@colbyfayock any news regarding the issue in cloudinary-util for support of other effects?

colbyfayock commented 12 months ago

not yet. to be clear, the util does support it, its just not typed, and as you infer the props from the types, thats where you run into the issue