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.

Sign up for GitHub

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

Jump to bottom

Update how custom svg dimesions are applied #216

Merged
merged 13 commits into from
Nov 20, 2024
Merged

Update how custom svg dimesions are applied #216

merged 13 commits into from
Nov 20, 2024

Conversation

gabriel-glo
Copy link
Contributor

@gabriel-glo gabriel-glo commented Jul 22, 2024 •
edited by dkotter
Loading

Description of the Change

  • Update how custom svg dimesions are applied for get_image_tag and wp_get_attachment_image functions
  • Separate the logic for defining custom dimensions in a set_svg_dimension function

Closes #6

How to test the Change

In any template output the image markup either by using get_image_tag or wp_get_attachment_image functions.
Set the custom image size to either one of the registered image size names or a custom one via array [$width, $height].
Output should match the set dimensions.

Changelog Entry

Added - added set_svg_dimension function in order to reduce repetition
Changed - changed how image dimensions were passed in get_image_tag_override and one_pixel_fix methods

Credits

Props @gabriel-glo, @jeremymoore, @iamdharmesh, @dkotter

Checklist:

  • I agree to follow this project's Code of Conduct.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests pass.

@gabriel-glo
Update how custom svg dimesions are applied
567c4be 
Update how custom svg dimesions are applied for `get_image_tag` and `wp_get_attachment_image` functions
Separate the logic for defining custom dimensions in a `set_svg_dimension` function
@gabriel-glo gabriel-glo self-assigned this Jul 22, 2024
@gabriel-glo gabriel-glo linked an issue Jul 22, 2024 that may be closed by this pull request
SVG height and width attributes #6
Open
@github-actions github-actions bot added this to the Future Release milestone Jul 22, 2024
@github-actions github-actions bot added the needs:code-review This requires code review. label Jul 22, 2024
@github-actions github-actions bot removed the needs:code-review This requires code review. label Jul 22, 2024
@github-actions GitHub Actions
Copy link

@gabriel-glo thanks for the PR! Could you please fill out the PR template with description, changelog, and credits information so that we can properly review and merge this?

@github-actions github-actions bot added the needs:feedback This requires feedback to determine next steps. label Jul 22, 2024
@jeffpaul jeffpaul requested review from iamdharmesh and removed request for dkotter and jeffpaul August 14, 2024 01:51
iamdharmesh
iamdharmesh previously approved these changes Aug 24, 2024
View reviewed changes
Copy link
Member

@iamdharmesh iamdharmesh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for the PR @gabriel-glo. Code looks good and it tests well. (Added one minor doc update suggestion)

@dkotter The PR looks good to me. However, since we had 2-3 size/dimension-related PRs on this plugin earlier and you have better context on those, I wanted to have a quick review from your side to ensure these changes align with those PRs.

Thank you.

safe-svg.php Outdated Show resolved Hide resolved
@iamdharmesh iamdharmesh requested a review from dkotter August 24, 2024 17:37
@jeffpaul jeffpaul modified the milestones: Future Release, 2.3.0 Aug 26, 2024
dkotter
dkotter requested changes Aug 27, 2024
View reviewed changes
safe-svg.php Outdated Show resolved Hide resolved
safe-svg.php Outdated Show resolved Hide resolved
safe-svg.php Outdated Show resolved Hide resolved
dkotter
dkotter requested changes Sep 6, 2024
View reviewed changes
safe-svg.php Outdated Show resolved Hide resolved
@gabriel-glo @dkotter
Update safe-svg.php
6c701db 
Co-authored-by: Darin Kotter <[email protected]>
@jeffpaul jeffpaul requested a review from dkotter September 12, 2024 18:12
@github-actions github-actions bot added needs:code-review This requires code review. and removed needs:feedback This requires feedback to determine next steps. labels Nov 20, 2024
dkotter
dkotter approved these changes Nov 20, 2024
View reviewed changes
Copy link
Collaborator

@dkotter dkotter left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me now. Tested with both get_image_tag and wp_get_attachment_image and verified things worked as expected with both. Testing on the latest release and on this PR, get_image_tag returns the exact same output. Testing wp_get_attachment_image, on the latest release, if you pass in an array of sizes, those values aren't being used. On this PR, those sizes are now respected and set properly as the height and width attributes.

@dkotter dkotter merged commit d5b1a52 into develop Nov 20, 2024
15 of 16 checks passed
@dkotter dkotter deleted the 6-svg-height-and-width-attributes branch November 20, 2024 18:37
@martinpl
Copy link

This breaks behavior in wp_get_attachment_image():
before: any thumbnail size will inherit SVG width, height attributes
now: only 'full' thumbnail will inherit SVG width, height attributes

Reverting this helps:
https://github.com/10up/safe-svg/pull/216/files#diff-05bf19b873cfb6358cd7c239366db6675635bf4b03a4b56353102e359d5c2859L447

@martinpl martinpl mentioned this pull request Nov 27, 2024
Closed
1 task
@dkotter dkotter mentioned this pull request Dec 3, 2024
Merged
4 tasks
dkotter added a commit that referenced this pull request Dec 5, 2024
@dkotter
Merge pull request #238 from 10up/fix/revert-216
13777ce 
Revert changes done in #216
This was referenced Dec 5, 2024
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dkotter dkotter dkotter approved these changes

@iamdharmesh iamdharmesh Awaiting requested review from iamdharmesh

Assignees

@gabriel-glo gabriel-glo

Labels
needs:code-review This requires code review.
Projects
None yet
Milestone
2.3.0
Development

Successfully merging this pull request may close these issues.

SVG height and width attributes
5 participants
@gabriel-glo @martinpl @dkotter @iamdharmesh @jeffpaul