ref(svg): convert jsx svgs to raw svgs #6843
Conversation
Chrissy
changed the title
Chrissy
changed the title
Generated by 🚫 danger |
Chrissy
changed the title
|
@ckj I think you will be able to review this in full, as there aren't any substantive changes. just:
I reviewed all of these visually and they should look identical, minus a few alignment bugs I tweaked to fix. This looks like a lot of code but it's all pretty fluffy! |
| @@ -32,6 +32,17 @@ module.exports = { | |||
| test: /\.css$/, | |||
| use: ['style-loader', 'css-loader'], | |||
| }, | |||
| { | |||
There was a problem hiding this comment.
these changes are just a duplication of what we do in the primary webpack config file, since we need the same support in storybook land.
| @@ -330,7 +329,7 @@ class FormField extends React.Component { | |||
| } else if (isSaved) { | |||
| return ( | |||
| <SettingsIsSaved> | |||
| <IconCheckmarkSm size="18" /> | |||
| <InlineSvg src="icon-checkmark-sm" size="18px" /> | |||
There was a problem hiding this comment.
Most browsers use a common sense approach when it comes to unit-less values as attributes, which is why all these naked numbers get converted to pixels and not ems, percentages, vms, etc.
But that kind of ambiguity is somewhat confusing and unpredictable. So I converted them all to pixel values.
Chrissy
force-pushed
the
react_svg_to_svg
branch
from
cea48f1
to
2eb7764
Compare
| @@ -32,6 +32,17 @@ module.exports = { | |||
| test: /\.css$/, | |||
| use: ['style-loader', 'css-loader'], | |||
| }, | |||
| { | |||
| strokeWidth="1" | ||
| fill="none" | ||
| strokeLinecap="round" | ||
| strokeLinejoin="round" |
There was a problem hiding this comment.
I had to manually camelCase all of these attributes to js-ify them for React. In the SVG spec, they're hyphenated e.g. stroke-linejoin="round". Is this something we should revert back to or does it matter?
| @@ -330,7 +329,7 @@ class FormField extends React.Component { | |||
| } else if (isSaved) { | |||
| return ( | |||
| <SettingsIsSaved> | |||
| <IconCheckmarkSm size="18" /> | |||
| <InlineSvg src="icon-checkmark-sm" size="18px" /> | |||
|
Unrelated, but it would be awesome if we had a section in storybook for all of our icons |
Not gonna include
icon-file-genericandicon-openin this one because I need to get mock data working for release overviews.Everything else has been tested and looks identical.