Skip to content

Conversation

thisisdano
Copy link
Contributor

This updates the SVG sprite compiler from gulp-svg-sprite to gulp-svgstore (https://www.npmjs.com/package/gulp-svgstore). This does not have a dependency on svg-sprite so it avoids issues with phantomjs 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.

@thisisdano thisisdano requested a review from mejiaj May 4, 2022 18:19
@mejiaj mejiaj requested a review from amyleadem May 6, 2022 14:18
@mgwalker
Copy link

mgwalker commented May 6, 2022

Tested on an ARM64 arch node:16 docker container; the install and npm run build both worked. When I diff the files in dist/img between the ARM64 container and an x86_64 container, there are no differences, but I have to admit I'm not sure how to compare the sprite sheet to previous builds because it is different, but maybe not in meaningful ways.

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 path elements are the same (just using different formatting, but the result should be the same). Most of the rest looks like it accomplishes the same set of things, but I'm not familiar enough with SVG to say for sure if the <path fill="none" d="M0 0h24v24H0z" /> and the <rect fill="none" height="24" width="24" /> accomplish the same thing.

EDIT: I also checked it out on an M1 Mac and it works a treat there as well. 😄 Thanks @thisisdano

Copy link
Contributor

@amyleadem amyleadem left a 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

@thisisdano thisisdano added the Hold label May 6, 2022
@thisisdano
Copy link
Contributor Author

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 3.0.1

@brunerae brunerae removed the Hold label May 6, 2022
@brunerae brunerae added this to the uswds 3.2.0 milestone Sep 30, 2022
@thisisdano thisisdano merged commit c2cbf14 into develop Oct 6, 2022
@thisisdano thisisdano deleted the dw-update-svg-sprite-compile branch October 6, 2022 17:39
This was referenced Oct 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants