Preserve SVG viewBox when minifying SVGs for production #5662
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Whoop! |
robertknight
force-pushed
the
fix-svg-rendering
branch
from
c030403 to
caf5363
Compare
|
In the Hypothesis client we are not actually minifying images at all in production builds, so the issue doesn't apply. |
By default svgo removes the `viewBox` attribute of the SVG, which breaks scaling of the SVG content when rendering. This caused various icons on the site to render at an incorrect scale. In some cases this was not very noticeable because the natural size of the content was close to the scaled size. In other cases the icon failed to appear at all. To see the difference, compare: ``` rm build/images/icons/* NODE_ENV=production gulp build-images cat build/images/icons/groups.svg ``` Before and after this patch is applied. Fixes #5656
robertknight
force-pushed
the
fix-svg-rendering
branch
from
caf5363 to
a777a03
Compare
lyzadanger
approved these changes
Jul 26, 2019
There was a problem hiding this comment.
Great fix!
I can verify that the viewBox has reappeared in minified SVGs locally. On master (i.e. "before"):
lyza@sweetbirch:~/Projects/h master
$ cat build/images/icons/groups.svg
<svg xmlns="http://www.w3.org/2000/svg" width="120" height="120"><g fill="currentColor" transform="translate(.508 7.627)" fill-rule="evenodd"><circle cx="36" cy="41" r="18"/><circle cx="84" cy="41" r="18"/><path d="M72 97.042h44V85s0-19-32-19c-9.065 0-15.562 1.525-20.218 3.71a24.324 24.324 0 0 1 3.278 3.213c2.135 2.536 3.518 5.274 4.291 8.027.276.984.456 1.908.558 2.756.066.553.091.99.091 1.294v12.042z"/><path d="M4 97.042h64V85s0-19-32-19S4.004 85 4.004 85L4 97.042z"/></g></svg>
On this branch:
lyza@sweetbirch:~/Projects/h ROB-fix-svg-rendering
$ cat build/images/icons/groups.svg
<svg xmlns="http://www.w3.org/2000/svg" width="120" height="120" viewBox="0 0 120 120"><g fill="currentColor" transform="translate(.508 7.627)" fill-rule="evenodd"><circle cx="36" cy="41" r="18"/><circle cx="84" cy="41" r="18"/><path d="M72 97.042h44V85s0-19-32-19c-9.065 0-15.562 1.525-20.218 3.71a24.324 24.324 0 0 1 3.278 3.213c2.135 2.536 3.518 5.274 4.291 8.027.276.984.456 1.908.558 2.756.066.553.091.99.091 1.294v12.042z"/><path d="M4 97.042h64V85s0-19-32-19S4.004 85 4.004 85L4 97.042z"/></g></svg>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
By default svgo (our SVG minifier) removes the
viewBoxattribute of the<svg>element, which breaksscaling of the SVG content when rendering.
This caused various icons on the site to render at an incorrect scale.
In some cases this was not very noticeable because the natural size of
the content was close to the scaled size. In other cases the icon failed
to appear at all.
To see the difference, compare:
Before and after this patch is applied.
I'm unclear if this issue appeared in production when svgo was last updated, which happened in 2762912, or whether it has in fact existed for a much longer period of time, since the behavior is not new in svgo.
Other projects will need to be checked for the same issue.
Fixes #5656
Background reading on how SVG scaling works: https://css-tricks.com/scale-svg/