-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Use more modern SVG sprite compiler #4665
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
Conversation
Tested on an ARM64 arch Here's a symbol from the sprite sheet from the old dist (as used by Tock; USWDS@2.13.0): <symbol viewBox="0 0 24 24" id="zoom_out_map" xmlns="http://www.w3.org/2000/svg">
<path fill="none" d="M0 0h24v24H0z"/>
<path d="M15 3l2.3 2.3-2.89 2.87 1.42 1.42L18.7 6.7 21 9V3h-6zM3 9l2.3-2.3 2.87 2.89 1.42-1.42L6.7 5.3 9 3H3v6zm6 12l-2.3-2.3 2.89-2.87-1.42-1.42L5.3 17.3 3 15v6h6zm12-6l-2.3 2.3-2.87-2.89-1.42 1.42 2.89 2.87L15 21h6v-6z"/>
</symbol> And here's the same symbol from the current build: <symbol id="zoom_out_map" viewBox="0 0 24 24">
<g enable-background="new 0 0 24 24">
<g>
<rect fill="none" height="24" width="24"/>
</g>
<g>
<g>
<g>
<path d="M15,3l2.3,2.3l-2.89,2.87l1.42,1.42L18.7,6.7L21,9V3H15z M3,9l2.3-2.3l2.87,2.89l1.42-1.42L6.7,5.3L9,3H3V9z M9,21 l-2.3-2.3l2.89-2.87l-1.42-1.42L5.3,17.3L3,15v6H9z M21,15l-2.3,2.3l-2.87-2.89l-1.42,1.42l2.89,2.87L15,21h6V15z"/>
</g>
</g>
</g>
</g>
</symbol> The main EDIT: I also checked it out on an M1 Mac and it works a treat there as well. 😄 Thanks @thisisdano |
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.
Everything appears to compile and render as expected
I want to hold this one for a little more testing, but I agree that it looks good so far. Let's just dig into the compiler's options, test the icons at a few sizes with our usual classes, and prep this for release after |
This updates the SVG sprite compiler from
gulp-svg-sprite
togulp-svgstore
(https://www.npmjs.com/package/gulp-svgstore). This does not have a dependency onsvg-sprite
so it avoids issues withphantomjs
which should fix compatibility issues with arm64 processors as outlined in uswds/uswds-compile#25.I've also taken the opportunity to update our Sass compiler to
sass-embedded
(https://www.npmjs.com/package/sass-embedded)This also removes some low and minor vulnerabilities in our devDependencies.
It should get a bit more thorough testing, but I have not noticed any icon regressions.