-
Notifications
You must be signed in to change notification settings - Fork 55
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1002 +/- ##
=======================================
Coverage 81.15% 81.15%
=======================================
Files 673 673
Lines 8653 8653
Branches 1528 1463 -65
=======================================
Hits 7022 7022
Misses 1616 1616
Partials 15 15
Continue to review full report at Codecov.
|
@@ -94,7 +94,7 @@ class IconViewerExample extends React.Component<any, {}> { | |||
.sort() | |||
.map(name => ( | |||
<div key={`${name}-outline`} style={cellStyles}> | |||
<Icon name={name} variables={{ outline: true }} /> | |||
<Icon name={name} outline /> | |||
<br /> | |||
<code>{name.replace(processedIconsNamePrefix, '')} outline</code> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like that here we are kind a suggesting that the outline is part of the name :\ Not related to your changes, but would rather see something like <Icon name=
${name.replace(processedIconsNamePrefix, '')} outline />
or some object {name:
${name.replace(processedIconsNamePrefix, '')}, outline: true}
, because I was asked multiple times by people about what the name like outline
for example for the Icon's doesn't work. It is not obvious that the outline is something else...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
{active ? '✔' : 'Copy'} | ||
</button> | ||
)} | ||
value={`<Icon name="${maybeExportedAs}" ${isOutline ? 'outline' : ''} />`} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Fixes #793.
BREAKING CHANGES
This PR removes the
iconVariables.outline
variable and introduces theoutline
prop.Before
After