-
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
Allow DI on ImageTransform #15646
base: 5.x
Are you sure you want to change the base?
Allow DI on ImageTransform #15646
Conversation
'quality', | ||
'width', | ||
'fill', | ||
]) : []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Getting all attributes instead of specifying, so we pick up any defined by a different class.
Alternatively, we could introduce something like \craft\models\ImageTransform::getImageAttributes
that returned the attribute names involved in the actual image manipulation.
@timkelty Finally got round to testing this and it's perfecto! I forked the 5.x branch and rebased your changes on top. Everything worked fine. This looks like a simple enough change it could be merged into Craft 4 as well? |
@timkelty What's needed to get this merged :) |
@timkelty I had another play with this PR and I can reduce it to just removing the filtering on the I can pass through the behaviours custom property by using the Event::on(
ImageTransform::class,
Model::EVENT_DEFINE_FIELDS,
function (DefineFieldsEvent $event) {
$event->fields['imgixParams'] = 'imgixParams';
}
); |
Description
The goal is to allow developers to write an
ImageTransformer
that builds URLs using properties other than those included in\craft\models\ImageTransform
, egblur
.This PR does 2 things:
createObject
so\craft\models\ImageTransform
can be customized through DI.\craft\models\ImageTransform::toArray
without any args, so that all properties are returned.Related issues
#15062
Example usage
Module/Plugin
Template