chore: uniform icons across list views #10363
Conversation
mistercrunch
commented
Jul 19, 2020
mistercrunch
commented
Codecov Report
@@ Coverage Diff @@
## master #10363 +/- ##
==========================================
- Coverage 70.48% 69.75% -0.73%
==========================================
Files 603 196 -407
Lines 32427 19045 -13382
Branches 3294 0 -3294
==========================================
- Hits 22856 13285 -9571
+ Misses 9467 5760 -3707
+ Partials 104 0 -104
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
mistercrunch
force-pushed
the
icons
branch
2 times, most recently
from
1c0cfda to
7318589
Compare
| <svg width="24" height="24" viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg"> | ||
| <g id="Icon / Share"> | ||
| <g id="upload"> | ||
| <path id="Shape" fill-rule="evenodd" clip-rule="evenodd" d="M8.71 7.71002L11 5.41002V15C11 15.5523 11.4477 16 12 16C12.5523 16 13 15.5523 13 15V5.41002L15.29 7.71002C15.4778 7.89933 15.7334 8.00582 16 8.00582C16.2666 8.00582 16.5222 7.89933 16.71 7.71002C16.8993 7.52225 17.0058 7.26666 17.0058 7.00002C17.0058 6.73338 16.8993 6.47779 16.71 6.29002L12.71 2.29002C12.6149 2.19898 12.5028 2.12761 12.38 2.08002C12.1365 1.98 11.8635 1.98 11.62 2.08002C11.4972 2.12761 11.3851 2.19898 11.29 2.29002L7.29 6.29002C7.03634 6.54368 6.93728 6.91339 7.03012 7.2599C7.12297 7.6064 7.39362 7.87705 7.74012 7.9699C8.08663 8.06274 8.45634 7.96368 8.71 7.71002ZM21 12C20.4477 12 20 12.4477 20 13V19C20 19.5523 19.5523 20 19 20H5C4.44772 20 4 19.5523 4 19V13C4 12.4477 3.55228 12 3 12C2.44772 12 2 12.4477 2 13V19C2 20.6569 3.34315 22 5 22H19C20.6569 22 22 20.6569 22 19V13C22 12.4477 21.5523 12 21 12Z" fill="#666666"/> |
There was a problem hiding this comment.
Can you change this fill color to currentColor which should make it easy to set the color property in these icons.
| | 'medium' | ||
| | 'lg' | ||
| | 'large' | ||
| | undefined; |
There was a problem hiding this comment.
making a prop optional, via the ?, automatically adds the undefined value.
| @@ -173,7 +174,7 @@ class DashboardList extends React.PureComponent<Props, State> { | |||
| }, | |||
| }: any) => ( | |||
| <span className="no-wrap"> | |||
| {published ? <i className="fa fa-check" /> : ''} | |||
| {published ? <i className="fa fa-check text-muted" /> : ''} | |||
There was a problem hiding this comment.
is this color supposed to match the other Icons? it doesn't currently. Maybe replace this with <Icon name="check' />
There was a problem hiding this comment.
+1 this. I think we should move to use our own svg/Icon component and finally remove Font Awesome. otherwise, i always feel confused which icon to use :(
mistercrunch
force-pushed
the
icons
branch
3 times, most recently
from
c808262 to
4a66b90
Compare
| @@ -62,13 +62,15 @@ export default function DeleteModal({ | |||
| primaryButtonType="danger" | |||
| show={open} | |||
| title={title} | |||
| bsSize="medium" | |||
There was a problem hiding this comment.
bycatch
after
before
|
Addressed comments! |
* chore: uniform icons across list views * smaller modals * integrating comments
mistercrunch
added
🏷️ bot
