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

Consider a smarter approach to getting SVG dimensions #242

Open
1 task done
dkotter opened this issue Dec 5, 2024 · 0 comments
Open
1 task done

Consider a smarter approach to getting SVG dimensions #242

dkotter opened this issue Dec 5, 2024 · 0 comments

Comments

@dkotter
Copy link
Collaborator

dkotter commented Dec 5, 2024

Is your enhancement related to a problem? Please describe.

At the moment, we default to using the full dimensions as set within the SVG itself, irregardless of whatever image size someone is requesting. As an example, if you're requesting a thumbnail size and your site has this registered at 150x150px and the SVG you're using is 250x250px, the resulting image markup will use 250x250px instead of 150x150px.

An attempt to modify this behavior was done in #216 but that led to some issues (see #237) and was reverted in #238. Because SVGs are different then other image types (like JPG or PNG), WordPress won't generate various cropped image sizes and thus we can't reliably use the passed in image size. As another example, if I have an SVG that is 700x500px and someone wants a 150x150px thumbnail, if I use 150x150 as my dimensions, the SVG won't render properly as the ratio doesn't match.

I believe a correct approach is documented in the super helpful comment from @gigatyrant:

Let’s take a simple example:

To display the image, I use: wp_get_attachment_image( 1, 'large' ), which has a size limit of 1024 x 1024 pixels.

If my client uploads a PNG larger than 1024 x 1024 pixels, WordPress crops it while maintaining the image’s format to avoid it being too heavy. Perfect!
If my client uploads a PNG (e.g., a pictogram) smaller than 1024 x 1024 pixels, WordPress displays the pictogram at its native size. Perfect!
If my client uploads an SVG larger than 1024 x 1024 pixels, Safe-SVG stretches the SVG to 1024 x 1024.
If my client uploads an SVG (e.g., a pictogram) smaller than 1024 x 1024 pixels, Safe-SVG also stretches it to 1024 x 1024.
The correct behavior is WordPress’s handling of PNGs: respecting the maximum size only when necessary, without stretching. In the case of Safe-SVG, the maximum size seems to be forced regardless, which is problematic.

Setting the size to “full” is not a viable solution because clients sometimes have the option to upload either a pictogram or a photo for the same element. This would mean that if a photo is used, the site would load it at its full size, which is not optimal.

In my opinion, the correct solution is the same as WordPress’s approach for PNGs:

If the SVG’s size is smaller than the allowed size, it should be displayed at its native size.
If the SVG exceeds the allowed size, it should be reduced to that size while maintaining its aspect ratio.
As it stands, the plugin causes issues by forcing all SVGs, including icons and pictograms, to display at 1024 x 1024, which disrupts the layout of many websites. My clients were surprised when all their icons suddenly appeared stretched to 1024 x 1024 pixels xD

I do think this will lead to some backwards incompatibility issues (the default image size for wp_get_attachment_image is thumbnail, so if someone hasn't changed this, once we make this change, those images may render smaller than previously), so we'll need to decide if that is worth it. My opinion here is it feels worth the time for someone to explore how best to achieve the above but we'll then need more discussion on if that's an actual change we want to make.

Designs

No response

Describe alternatives you've considered

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@dkotter dkotter added the type:enhancement New feature or request. label Dec 5, 2024
@dkotter dkotter added this to the Future Release milestone Dec 5, 2024
@jeffpaul jeffpaul removed the type:enhancement New feature or request. label Dec 5, 2024
@jeffpaul jeffpaul moved this from Incoming to Backlog in Open Source Practice Dec 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Backlog
Development

No branches or pull requests

2 participants