-
Notifications
You must be signed in to change notification settings - Fork 640
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
[WIP] Add extraOptions property to transforms #15062
Conversation
I’m also looking to add extra properties to transforms in my Cloudinary plugin that won't be removed during normalization. Ideally, I’d like to achieve this without using an |
@thomasvantuycom I'm not sure it can be done without |
@brandonkelly Any thoughts on this? |
@leevigraham to make sure we're on the same page, can you briefly describe your use-case and exactly what you're trying to do, outside of implementation? Am I correct in saying that you're trying to have transforms that include non-native transform params when building the URL?
|
@timkelty basically yes that's the outcome. The thing I'm trying to do at a more conceptual level is add extra properties to the transform so they can be picked up later in the before transform event. Currently they are lost in the The idea was that adding a single |
@leevigraham this is definitely something we want to refactor for Craft 6. Something like An alternative idea is to use create your own |
The proposed solution works really well! |
@timkelty I'll take a look at this today / tomorrow. I like the solution using a custom image transform class and getting the attributes. |
Sounds good! Its just a proof of concept as is, but probably pretty close to what you'd need. Let me know how it goes. |
Additionally, if a refactor is planned for Craft 6, it might be worth considering a broader approach by generalizing the concept of image transforms to asset transforms. This would allow for more flexibility, as services like Cloudinary offer transformations for videos, PDFs, and various other file types. |
Closed in favour of #15646 |
Description
This PR adds a new
extraOptions
property toImageTransforms
.Why is this needed?
While creating a custom Imgix module I wanted to pass imgix parameters into the transform. To achieve this I added a new behaviour to the
ImageTransform
I then added an
EVENT_BEFORE_GENERATE_TRANSFORM
event listener that created an Imgix url.This worked fine until I added
srcset
behaviour.The current
Asset::getUrlsBySize()
normalises the current image transform, creates a newImageTransform
by copying the properties and generating the URL.The issue form me was the
imgixParams
value defined by the custom behaviour was not passed to the new sizeTransforms.Solution
This PR adds a new
extraOptions
property toImageTransforms
. This property is passed down to thesizeTransforms
and therefore can be accessed in theAsset::EVENT_BEFORE_GENERATE_TRANSFORM
event or anywhere else the transform is accessed.TODO
extraOptions
to the GraphQL code / implementationextraOptions
toImageTransforms:createTransformFromString
andImageTransforms:parseTransformString
… I assume these are used in filenames, db columns… probably need to base64 the serialised array content.