-
Notifications
You must be signed in to change notification settings - Fork 35
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
Revert changes done in #216 #238
Conversation
This reverts the changes made in #216 and brings back the functionality for Basically that function is using the dimensions of the image size passed in instead of the SVG dimensions (basically functions the way I've added E2E tests in this PR for both |
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.
Code changes look good to me and it tests well. Thanks @dkotter
Description of the Change
As reported in #237, the changes made in #216 are causing issues for anyone using the
wp_get_attachment_image
function to output SVGs. As described in that Issue, we need to be smarter in how we determine the height and width attributes to use instead of just relying on the dimensions for each registered size (since SVGs don't get cropped the same as normal images by WordPress).This PR reverts the changes introduced in #216 and also adds some additional E2E tests to help us catch these sorts of issues in the future.
Note there is additional work we could look to do here, both fixing what we were attempting to do in #216 and being smarter with how we determine the SVG sizes, as described in this comment.
Closes #237
How to test the Change
wp_get_attachment_image
and ensure that no matter what image size you pass in, the full SVG dimensions are used instead (which is previous behavior)Changelog Entry
Credits
Props @dkotter, @martinpl, @subfighter3, @smerriman, @gigatyrant
Checklist: