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.

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

Optimize SVGs using svgo #2330

Merged
merged 2 commits into from Jul 15, 2018
Merged

Optimize SVGs using svgo #2330

merged 2 commits into from Jul 15, 2018

Conversation

lencioni
Copy link
Contributor

@lencioni lencioni commented Jul 3, 2018

I was able to reduce the size of most of the SVGs in this repo, which
should give visitors a little bit better performance.

@montogeek
Copy link
Member

Unfortunately we have no visual regression tests (we will, soon). Did you notice any weird change on sizes or something?

@lencioni
Copy link
Contributor Author

lencioni commented Jul 4, 2018

Unfortunately we have no visual regression tests (we will, soon).

Cool! You should consider happo.io. I bet @trotzig would help get you set up.

Did you notice any weird change on sizes or something?

I did all of this visually in SVGOMG and it all looked very very similar to me, almost identical. You should be able to see these visually in GitHub by clicking on "View" when looking at the code changes.

@trotzig
Copy link

trotzig commented Jul 4, 2018

I ran a quick check with happo (only looking at icons, in Chrome) and it looks like most diffs are subtle:
https://happo.io/a/23/compare/dev-76d8f9e7e371e59ba789/dev-876b368611dcedef48a6

I worry a little about the sizing change for src/assets/by.svg and src/assets/cc.svg. Depending on how they're used, we might want to address that.

If you'd be interested, I could look at a more thorough happo integration.

@lencioni
Copy link
Contributor Author

lencioni commented Jul 5, 2018

I worry a little about the sizing change for src/assets/by.svg and src/assets/cc.svg. Depending on how they're used, we might want to address that.

It looks like they are used in the footer. I grabbed their URLs from this PR and pasted them into the footer in my browser's inspector tool, and they look like they are still the same size. I can add height and width attributes to those images to be safe, and to also avoid jitter as the images come in. It seems that these images have their width and height specified in Footer.scss

img {
width: auto;
height: 100%;
margin-left: 1px;
}

I was able to reduce the size of most of the SVGs in this repo, which
should give visitors a little bit better performance.
@lencioni
Copy link
Contributor Author

@montogeek do you want me to make any changes to this PR?

@montogeek montogeek merged commit 2ec5c53 into webpack:master Jul 15, 2018
@montogeek
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants