aryehraber / statamic-responsive-img

Statamic v2 Addon that automatically handles responsive images using srcset.
MIT License
12 stars 1 forks source link

No support for PNG images? #8

Closed sxdj closed 6 years ago

sxdj commented 6 years ago

Great addon! Though it doesn't seem to work for PNG images (since it gets transformed to JPGs in the output), is that a bug or just not supported (yet)?

We use transparent PNGs massively on our site, so the addon will only be usable when this is supported.

Thanks in advance!

aryehraber commented 6 years ago

Yeah, this was done by design because Glide can't compress PNG images (quality param) but in retrospect this should probably be handled a little more gracefully.

In a future version there are plans to accept all Glide params so that this can be handled by devs instead of being "hardcoded" within the Addon, but for now you can make a minor change yourself:

In ResponsiveImg.php line 121:

$default = ['fm' => 'jpg', 'q' => $this->quality];

Change to:

$default = [];
sxdj commented 6 years ago

If I change to $default = []; I get the following error:

FatalErrorException in Parser.php line 789:
Method Illuminate\View\View::__toString() must not throw an exception, caught ErrorException: 
htmlentities() expects parameter 1 to be string, object given (View:
/Users/sxdj/Code/www/site/addons/ResponsiveImg/resources/views/img.blade.php)

But it works for now when I change jpg to png in $default = ['fm' => 'png', 'q' => $this->quality]; 😄 Thanks for your quick support!

aryehraber commented 6 years ago

Ah right, I hadn't tested the change...

Perhaps just $default = ['q' => $this->quality]; will do the trick.

Either way, glad it's working for you now 😊 Closing as this FR is already on the roadmap. Thanks!

ryandc commented 4 years ago

Is this still the recommended fix?