fix: Button icon part color #102
Conversation
levithomason
requested review from
dvdzkwsk,
kuzhelov and
mnajdova
as code owners
levithomason
force-pushed
the
fix/button-icon-color
branch
from
ccfd498
to
b234a65
Compare
Codecov Report
@@ Coverage Diff @@
## master #102 +/- ##
==========================================
+ Coverage 87.51% 88.13% +0.62%
==========================================
Files 43 43
Lines 729 725 -4
Branches 98 96 -2
==========================================
+ Hits 638 639 +1
+ Misses 88 83 -5
Partials 3 3
Continue to review full report at Codecov.
|
src/components/Button/Button.tsx
Outdated
| @@ -105,7 +105,7 @@ class Button extends UIComponent<any, any> { | |||
| key="btn-icon" | |||
| name={icon} | |||
| xSpacing={!content ? 'none' : iconIsAfterButton ? 'before' : 'after'} | |||
| color={primary ? 'white' : 'black'} | |||
| variables={{ color: primary ? 'white' : 'black' }} | |||
There was a problem hiding this comment.
actually, this still will result in Icon having color different from the button's one (obviously, Icon here could be either black or white) - while, as a consumer, I would rather expect that it would take foreground color of Button
levithomason
force-pushed
the
fix/button-icon-color
branch
from
b234a65
to
eab2fd5
Compare
There was a problem hiding this comment.
as we've agreed upon, will share cases that I asee will be problematic in case
- of using
propsinstead of variables, with the limited set of values and much limited set of things available for tweaks. To be clear - limit amount of customization options is absolutely necessary if we would like components to look consistent, but this move to props will eliminate some customization mechanisms that I see being critical - introduce cases where 'inherit' fallback won't be an option - there are quite a few of them
levithomason
force-pushed
the
fix/button-icon-color
branch
from
f09feae
to
a2464c1
Compare
levithomason
force-pushed
the
fix/button-icon-color
branch
from
a2464c1
to
181cf51
Compare
levithomason
force-pushed
the
fix/button-icon-color
branch
from
181cf51
to
628cd19
Compare
|
This PR touched a few concepts in fixing the color. I have updated it to only fix the issue with passing the correct Icon variable. Broader changes concerning our props vs variables API can be hashed out and handled separately. |
|
All tests now pass. @kuzhelov, merging since I've reverted the changes you've objected to. |
This PR fixes the icon color inside of Buttons.