Skip to content
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

Added extension parameter to ImageResizer #177

Merged
merged 1 commit into from
Feb 29, 2024

Conversation

WebVPF
Copy link
Contributor

@WebVPF WebVPF commented Feb 28, 2024

@AIC-BV
Copy link
Contributor

AIC-BV commented Feb 29, 2024

😱 Since when is this possible? I've been using JPGs for the last few years!

@LukeTowers Is it still good practice to use TinyPNG to make the images smaller first? Or is webp so small that this isn't needed anymore?

@AIC-BV
Copy link
Contributor

AIC-BV commented Feb 29, 2024

I assume a fallback to JPG is no longer needed? https://caniuse.com/webp

@AIC-BV
Copy link
Contributor

AIC-BV commented Feb 29, 2024

This enhancement is no longer needed for me because of the new resize parameter
wintercms/wn-tinypng-plugin#2

@WebVPF
Copy link
Contributor Author

WebVPF commented Feb 29, 2024

😱 Since when is this possible? I've been using JPGs for the last few years!

This was available even before the fork.

Example:

<img src="{{ user.avatar.getThumb(90, 90, {'mode': 'exact', 'quality': 80, 'extension': 'webp'}) }}" width="90" height="90" alt="{{ user.name }}" />

or

<img src="{{ item.foto.thumb(400, 300, {'extension': 'webp'}) }}" />

@LukeTowers LukeTowers merged commit 180cbbb into wintercms:develop Feb 29, 2024
1 check passed
@WebVPF WebVPF deleted the patch-1 branch February 29, 2024 15:15
@damsfx
Copy link
Contributor

damsfx commented Mar 1, 2024

😱 Since when is this possible? I've been using JPGs for the last few years!

One more for me!
I was completely unaware of this parameter.

Is this option related to other platform requirements?

@WebVPF
Copy link
Contributor Author

WebVPF commented Mar 1, 2024

Is this option related to other platform requirements?

The only requirement is that the source image file must have the extension jpg, jpeg, png, gif, webp.
https://github.com/wintercms/storm/blob/dd89dc140bfdb0e209437b2263b711b8fa655981/src/Database/Attach/File.php#L65


All parameters: https://github.com/wintercms/storm/blob/dd89dc140bfdb0e209437b2263b711b8fa655981/src/Database/Attach/File.php#L622-L629

Default values: https://github.com/wintercms/storm/blob/dd89dc140bfdb0e209437b2263b711b8fa655981/src/Database/Attach/File.php#L689-694


There is one more parameter that is not specified in the documentation. This is interlace.

interlace: Interlace image, Boolean: false (disabled: default), true (enabled)
https://github.com/wintercms/storm/blob/dd89dc140bfdb0e209437b2263b711b8fa655981/src/Database/Attach/Resizer.php#L156-L176

In my opinion, this is something outdated for today's Internet.


I don't see in the code whether there is a parameter to set an SEO-friendly name for the file. Lighthouse doesn't like the generated file names.

@damsfx
Copy link
Contributor

damsfx commented Mar 1, 2024

The only requirement is that the source image file must have the extension jpg, jpeg, png, gif, webp. https://github.com/wintercms/storm/blob/dd89dc140bfdb0e209437b2263b711b8fa655981/src/Database/Attach/File.php#L65

I'd never realized before that this amazing feature was available!

There is one more parameter that is not specified in the documentation. This is interlace.
...
In my opinion, this is something outdated for today's Internet.

Totally agree.

I don't see in the code whether there is a parameter to set an SEO-friendly name for the file. Lighthouse doesn't like the generated file names.

Yes, this is a serious concern for good image SEO.

@LukeTowers
Copy link
Member

The ImageResizer's image URLs for all file types except for FileAttachements should be SEO friendly, is it not?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants