Skip to content

Conversation

@austingreendev
Copy link
Contributor

@austingreendev austingreendev commented Dec 4, 2018

Description

This PR removes the babel-plugin-inline-react-svg (and svg-react-loader in styleguidist) package in favor of the @svgr/webpack webpack loader.

Detail

This move is to align ourselves more closely with create-react-app and use a more widely support inline-SVG solution. SVGR also allows us some more advanced customization than the babel plugin.

This PR includes:

  • Updates to all inline-SVG usages in our markdown documentation (bulk of changes)
  • Update webpack configs for build and styleguidist
  • Include new SVG mock for Jest
  • Remove the default SVGO preference of removing the viewBox attribute
svgoConfig: {
  plugins: {
    removeViewBox: false
  }
}

This change will allow us to have more flexibility in how we embed SVG's in the future.

Checklist

  • 👌 design updates are Garden Designer approved (add the
    designer as a reviewer)
  • 💅 view component styling is based on a Garden CSS
    component
  • 🌐 Styleguidist demo is up-to-date (yarn start)
  • ⬅️ renders as expected with reversed (RTL) direction
  • 💂‍♂️ includes new unit and snapshot tests
  • 📒 any new files are included in the packages src/index.js export
  • 📝 tested in Chrome, Firefox, Safari, Edge, and IE11

@austingreendev
Copy link
Contributor Author

Just added some updated SVG mocking in Jest and updated affected snapshots.

@coveralls
Copy link

coveralls commented Dec 4, 2018

Coverage Status

Coverage remained the same at 94.751% when pulling babd9dd on agreen/svgr-webpack into 4bdc25d on master.

Copy link
Contributor

@ryanseddon ryanseddon left a comment

Choose a reason for hiding this comment

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

Nice 💯

@austingreendev austingreendev merged commit 310d1aa into master Dec 5, 2018
@austingreendev austingreendev deleted the agreen/svgr-webpack branch December 5, 2018 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

6 participants