From 0ffe400bb56fff798f32701a2e40cfad495eabef Mon Sep 17 00:00:00 2001 From: Tim Kelty Date: Thu, 23 Apr 2026 08:52:51 -0400 Subject: [PATCH] Fix image transform editing in the control panel --- src/Module.php | 15 +++-- ...ansform.php => ImageTransformBehavior.php} | 59 ++++++----------- src/imagetransforms/ImageTransformer.php | 32 +++++---- tests/unit/ImageTransformTest.php | 65 ++++++++++++++----- 4 files changed, 101 insertions(+), 70 deletions(-) rename src/imagetransforms/{ImageTransform.php => ImageTransformBehavior.php} (76%) diff --git a/src/Module.php b/src/Module.php index 6e60a26..7fb77ea 100644 --- a/src/Module.php +++ b/src/Module.php @@ -6,13 +6,14 @@ use craft\base\Event; use craft\base\Model; use craft\cloud\fs\AssetsFs; -use craft\cloud\imagetransforms\ImageTransform; +use craft\cloud\imagetransforms\ImageTransformBehavior; use craft\cloud\imagetransforms\ImageTransformer; use craft\cloud\twig\TwigExtension; use craft\cloud\web\assets\uploader\UploaderAsset; use craft\cloud\web\ResponseEventHandler; use craft\console\Application as ConsoleApplication; use craft\elements\Asset; +use craft\events\DefineBehaviorsEvent; use craft\events\DefineRulesEvent; use craft\events\GenerateTransformEvent; use craft\events\RegisterComponentTypesEvent; @@ -85,10 +86,6 @@ public function bootstrap($app): void ), ]); - // Replace ImageTransform with cloud ImageTransform via DI - // We do this here and not in AppConfig, because non-Cloud envs need it to support non-standard transform props - Craft::$container->set(CraftImageTransform::class, ImageTransform::class); - if (Helper::isCraftCloud()) { $this->bootstrapCloud($app); } @@ -176,6 +173,14 @@ protected function bootstrapCloud(ConsoleApplication|WebApplication $app): void protected function registerGlobalEventHandlers(): void { + Event::on( + CraftImageTransform::class, + Model::EVENT_DEFINE_BEHAVIORS, + static function(DefineBehaviorsEvent $event) { + $event->behaviors['cloud'] = ImageTransformBehavior::class; + } + ); + Event::on( ImageTransforms::class, ImageTransforms::EVENT_REGISTER_IMAGE_TRANSFORMERS, diff --git a/src/imagetransforms/ImageTransform.php b/src/imagetransforms/ImageTransformBehavior.php similarity index 76% rename from src/imagetransforms/ImageTransform.php rename to src/imagetransforms/ImageTransformBehavior.php index 4f69dc2..345b392 100644 --- a/src/imagetransforms/ImageTransform.php +++ b/src/imagetransforms/ImageTransformBehavior.php @@ -3,13 +3,17 @@ namespace craft\cloud\imagetransforms; use Craft; +use craft\models\ImageTransform; use Illuminate\Support\Collection; +use yii\base\Behavior; /** * @see https://developers.cloudflare.com/images/transform-images/transform-via-workers/#fetch-options * @see https://github.com/cloudflare/workerd/blob/main/types/defines/cf.d.ts + * + * @property ImageTransform $owner */ -class ImageTransform extends \craft\models\ImageTransform +class ImageTransformBehavior extends Behavior { public ?bool $anim = null; public ?string $background = null; @@ -59,11 +63,6 @@ class ImageTransform extends \craft\models\ImageTransform */ public ?string $flip = null; - /** - * @var 'auto'|'avif'|'webp'|'jpeg'|'baseline-jpeg'|'json'|string|null - */ - public ?string $format = null; - /** * @var float|null */ @@ -74,8 +73,6 @@ class ImageTransform extends \craft\models\ImageTransform */ public string|array|null $gravity = null; - public ?int $height = null; - /** * @var 'keep'|'copyright'|'none'|null */ @@ -106,11 +103,9 @@ class ImageTransform extends \craft\models\ImageTransform */ public null|string|array $trim = null; - public ?int $width = null; - public ?float $zoom = null; - public function toOptions(): array + public function toOptions(array|string|null $gravity = null): array { $reflection = new \ReflectionClass($this); @@ -124,35 +119,29 @@ public function toOptions(): array $options['format'] = $this->computeFormat(); $options['fit'] = $this->computeFit(); $options['background'] = $this->computeBackground(); - $options['gravity'] ??= $this->computeGravity(); + $options['gravity'] ??= $gravity ?? $this->computeGravity(); + $options['height'] = $this->owner->height; + $options['width'] = $this->owner->width; return Collection::make($options) ->filter(fn($value) => $value !== null) ->all(); } - /** - * Compute the Cloudflare format from the base format and interlace settings. - * - * @return string|null - */ private function computeFormat(): ?string { - if ($this->format === 'jpg' && $this->interlace === 'none') { + if ($this->owner->format === 'jpg' && $this->owner->interlace === 'none') { return 'baseline-jpeg'; } - return match ($this->format) { + return match ($this->owner->format) { 'jpg' => 'jpeg', - default => $this->format, + default => $this->owner->format, }; } /** - * Compute the Cloudflare fit mode from the base mode and upscale settings. - * * @see https://developers.cloudflare.com/images/transform-images/transform-via-url/#fit - * @return string */ private function computeFit(): string { @@ -160,33 +149,26 @@ private function computeFit(): string return $this->fit; } - return match ($this->mode) { - 'fit' => $this->upscale ? 'contain' : 'scale-down', + return match ($this->owner->mode) { + 'fit' => $this->owner->upscale ? 'contain' : 'scale-down', 'stretch' => 'squeeze', 'letterbox' => 'pad', - default => $this->upscale ? 'cover' : 'crop', + default => $this->owner->upscale ? 'cover' : 'crop', }; } - /** - * Compute the Cloudflare background color from the base mode and fill settings. - * - * @return string|null - */ private function computeBackground(): ?string { if ($this->background !== null) { return $this->background; } - return $this->mode === 'letterbox' - ? $this->fill ?? '#FFFFFF' + return $this->owner->mode === 'letterbox' + ? $this->owner->fill ?? '#FFFFFF' : null; } /** - * Compute the Cloudflare gravity from the base position setting. - * * @return array{x: float, y: float}|null|'face' */ private function computeGravity(): array|null|string @@ -195,12 +177,11 @@ private function computeGravity(): array|null|string return $this->gravity; } - if ($this->position === 'center-center') { + if ($this->owner->position === 'center-center') { return null; } - // TODO: maybe just do this in Craft - $parts = explode('-', $this->position); + $parts = explode('-', $this->owner->position); try { $x = match ($parts[1] ?? null) { @@ -214,7 +195,7 @@ private function computeGravity(): array|null|string 'bottom' => 1, }; } catch (\UnhandledMatchError $e) { - Craft::warning("Invalid position value: `{$this->position}`", __METHOD__); + Craft::warning("Invalid position value: `{$this->owner->position}`", __METHOD__); return null; } diff --git a/src/imagetransforms/ImageTransformer.php b/src/imagetransforms/ImageTransformer.php index b84ce8f..6296a2a 100644 --- a/src/imagetransforms/ImageTransformer.php +++ b/src/imagetransforms/ImageTransformer.php @@ -9,6 +9,7 @@ use craft\elements\Asset; use craft\helpers\Assets; use craft\helpers\Html; +use craft\models\ImageTransform; use League\Uri\Components\Query; use League\Uri\Contracts\UriInterface; use League\Uri\Modifier; @@ -23,7 +24,7 @@ class ImageTransformer extends Component implements ImageTransformerInterface public const SUPPORTED_IMAGE_FORMATS = ['jpg', 'jpeg', 'gif', 'png', 'avif', 'webp']; private const SIGNING_PARAM = 's'; - public function getTransformUrl(Asset $asset, \craft\models\ImageTransform $imageTransform, bool $immediately): string + public function getTransformUrl(Asset $asset, ImageTransform $imageTransform, bool $immediately): string { if (version_compare(Craft::$app->version, '5.0', '>=')) { // @phpstan-ignore argument.type, arguments.count (Craft 5 compatibility) @@ -44,13 +45,9 @@ public function getTransformUrl(Asset $asset, \craft\models\ImageTransform $imag throw new NotSupportedException('SVG files shouldn’t be transformed.'); } - // ImageTransform DI will not work on Craft 4, so we convert the object. - // @see https://github.com/craftcms/cms/pull/15646 - $imageTransform = Craft::createObject(ImageTransform::class, [$imageTransform->toArray()]); + $gravity = $this->applyAssetFocalPointGravity($asset, $imageTransform); - $this->applyAssetFocalPointGravity($asset, $imageTransform); - - $query = Query::fromVariable($imageTransform->toOptions()); + $query = Query::fromVariable($this->behavior($imageTransform)->toOptions($gravity)); $uri = Modifier::wrap(Uri::new($assetUrl)) ->mergeQuery($query) ->unwrap(); @@ -62,13 +59,26 @@ public function invalidateAssetTransforms(Asset $asset): void { } - protected function applyAssetFocalPointGravity(Asset $asset, ImageTransform $imageTransform): void + protected function applyAssetFocalPointGravity(Asset $asset, ImageTransform $imageTransform): array|string|null + { + $behavior = $this->behavior($imageTransform); + + if (!$asset->getHasFocalPoint() || isset($behavior->gravity)) { + return null; + } + + return $asset->getFocalPoint(); + } + + private function behavior(ImageTransform $imageTransform): ImageTransformBehavior { - if (!$asset->getHasFocalPoint() || isset($imageTransform->gravity)) { - return; + $behavior = $imageTransform->getBehavior('cloud'); + + if (!$behavior instanceof ImageTransformBehavior) { + throw new \RuntimeException('Cloud image transform behavior is not attached.'); } - $imageTransform->gravity = $asset->getFocalPoint(); + return $behavior; } private function sign(UriInterface $uri): UriInterface diff --git a/tests/unit/ImageTransformTest.php b/tests/unit/ImageTransformTest.php index fb4bc49..aa2be32 100644 --- a/tests/unit/ImageTransformTest.php +++ b/tests/unit/ImageTransformTest.php @@ -3,16 +3,24 @@ namespace craft\cloud\tests\unit; use Codeception\Test\Unit; -use craft\cloud\imagetransforms\ImageTransform; +use Craft; +use craft\cloud\Module as CloudModule; +use craft\cloud\imagetransforms\ImageTransformBehavior; use craft\cloud\imagetransforms\ImageTransformer; use craft\elements\Asset; +use craft\models\ImageTransform; class ImageTransformTest extends Unit { - /** - * @var \UnitTester - */ - protected $tester; + protected function _before(): void + { + parent::_before(); + + if (CloudModule::getInstance() === null) { + $module = new CloudModule('cloud'); + $module->bootstrap(Craft::$app); + } + } public function testCropModeWithExplicitGravityPreservesItInOptions(): void { @@ -34,7 +42,7 @@ public function testCropModeWithExplicitGravityPreservesItInOptions(): void ], 'height' => 750, 'width' => 1200, - ], $transform->toOptions()); + ], $this->behavior($transform)->toOptions()); } public function testCropModeWithoutGravityUsesPositionMapping(): void @@ -54,7 +62,7 @@ public function testCropModeWithoutGravityUsesPositionMapping(): void ], 'height' => 750, 'width' => 1200, - ], $transform->toOptions()); + ], $this->behavior($transform)->toOptions()); } public function testFocalPointGravityPassesThroughUnchanged(): void @@ -67,12 +75,28 @@ public function testFocalPointGravityPassesThroughUnchanged(): void 'height' => 750, ]); - (new TestImageTransformer())->applyFocalPointGravity($asset, $transform); + $gravity = (new TestImageTransformer())->applyFocalPointGravity($asset, $transform); $this->assertSame([ 'x' => 0.474, 'y' => 0.3064, - ], $transform->gravity); + ], $gravity); + $this->assertNull($this->behavior($transform)->gravity); + } + + public function testInlineCloudPropertyDoesNotBreakBaseTransform(): void + { + $transform = new ImageTransform([ + 'width' => 200, + 'blur' => 5, + ]); + + $this->assertSame(5, $this->behavior($transform)->blur); + $this->assertSame([ + 'blur' => 5, + 'fit' => 'cover', + 'width' => 200, + ], $this->behavior($transform)->toOptions()); } public function testGetTransformUrlDoesNotLeakGravityBetweenAssets(): void @@ -179,23 +203,34 @@ public function getMimeType(mixed $transform = null): ?string }; } + private function behavior(ImageTransform $transform): ImageTransformBehavior + { + $behavior = $transform->getBehavior('cloud'); + + $this->assertInstanceOf(ImageTransformBehavior::class, $behavior); + + return $behavior; + } + } class TestImageTransformer extends ImageTransformer { - public function applyFocalPointGravity(Asset $asset, ImageTransform $imageTransform): void + public function applyFocalPointGravity(Asset $asset, ImageTransform $imageTransform): array|string|null { - $this->applyAssetFocalPointGravity($asset, $imageTransform); + return $this->applyAssetFocalPointGravity($asset, $imageTransform); } } class UrlTestImageTransformer extends ImageTransformer { - public function buildTransformQuery(Asset $asset, \craft\models\ImageTransform $imageTransform): string + public function buildTransformQuery(Asset $asset, ImageTransform $imageTransform): string { - $imageTransform = \Craft::createObject(ImageTransform::class, [$imageTransform->toArray()]); - $this->applyAssetFocalPointGravity($asset, $imageTransform); + $gravity = $this->applyAssetFocalPointGravity($asset, $imageTransform); + + /** @var ImageTransformBehavior $behavior */ + $behavior = $imageTransform->getBehavior('cloud'); - return (string) \League\Uri\Components\Query::fromVariable($imageTransform->toOptions()); + return (string) \League\Uri\Components\Query::fromVariable($behavior->toOptions($gravity)); } }