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

feat(icons): re-enable SVGO optimizations for icons #5814

Conversation

joshblack
Copy link
Contributor

We turned off this setting back in #5603 but have since transitioned to using the original SVG assets instead of the generated JS for rendering icons in a Sketch plugin. As a result, this PR re-enables the optimizations that were causing issues in Sketch but still render fine in a browser.

Changelog

New

Changed

Removed

@joshblack joshblack requested a review from a team as a code owner April 6, 2020 19:19
@ghost ghost requested review from asudoh and tw15egan April 6, 2020 19:19
@joshblack joshblack requested a review from emyarod April 6, 2020 19:20
@netlify
Copy link

netlify bot commented Apr 6, 2020

Deploy preview for carbon-elements ready!

Built with commit 67d7e7e

https://deploy-preview-5814--carbon-elements.netlify.com

@netlify
Copy link

netlify bot commented Apr 6, 2020

Deploy preview for carbon-components-react ready!

Built with commit 67d7e7e

https://deploy-preview-5814--carbon-components-react.netlify.com

@tw15egan
Copy link
Collaborator

tw15egan commented Apr 6, 2020

Would it be possible to also run this on the SVG's in the @carbon/icons package so we can close #5568 ?

Copy link
Member

@emyarod emyarod left a comment

Choose a reason for hiding this comment

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

looks like some more snapshots need updating to pass CI. this doesn't affect Sketch symbol generation now right? (all of the Sketch symbols look good to me)

@joshblack
Copy link
Contributor Author

@emyarod the plugin should be good! And updated 👍

@tw15egan yeah! That's the plan, I think we'll need to move svg to src/svg and then output the optimized svg directory. The reason being is that SVGO-optimized assets don't really render well when going back into Sketch 😞

Copy link
Member

@emyarod emyarod left a comment

Choose a reason for hiding this comment

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

looks good to me

@joshblack joshblack merged commit 09348e6 into carbon-design-system:master Apr 8, 2020
@joshblack joshblack deleted the fix/re-enable-optimization-icons branch April 8, 2020 19:07
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.

4 participants