-
Notifications
You must be signed in to change notification settings - Fork 55
[READY FOR REVIEW] feat(Button): adding disabled prop to Button #14
Conversation
Codecov Report
@@ Coverage Diff @@
## master #14 +/- ##
==========================================
- Coverage 82.77% 82.73% -0.04%
==========================================
Files 59 59
Lines 830 840 +10
Branches 186 180 -6
==========================================
+ Hits 687 695 +8
- Misses 139 141 +2
Partials 4 4
Continue to review full report at Codecov.
|
The icon should not need to be styled individually here, I don't think... |
52db469
to
e6e2629
Compare
27c19ef
to
c097d5a
Compare
src/components/Button/Button.tsx
Outdated
key="btn-icon" | ||
name={icon} | ||
xSpacing={!content ? 'none' : iconIsAfterButton ? 'before' : 'after'} | ||
color={primary ? 'white' : 'black'} |
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.
If we want the icon color to be the same as the text color, we can use 'currentColor' instead of the hard-coded value.
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.
@notandrew
currentColor
would be a hex number right now and we're not supporting that for Icon
component yet. I think @kuzhelov has an ongoing PR for that; we'll address this afterwards
fyi @mnajdova
c097d5a
to
49b0924
Compare
49b0924
to
975e3ce
Compare
975e3ce
to
86f23b2
Compare
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.
Good to be merged here.
Button(
disabled
prop)This PR will introduce possibility to specify
disabled
buttonsTODO
API Proposal
disabled
disabled
property will disable buttons by manipulatingopacity
CSS style, borders, colors, hover styles and preventing clickingKnow issue: Icons are not disabled because
disabled
property was not added to icons at the time of creating this PR. I created a PR for that #113Icon buttons will become disabled out of the box after merging that one
will render