-
Notifications
You must be signed in to change notification settings - Fork 55
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1716 +/- ##
==========================================
- Coverage 70.97% 70.95% -0.02%
==========================================
Files 860 860
Lines 7142 7144 +2
Branches 2072 2056 -16
==========================================
Hits 5069 5069
- Misses 2067 2069 +2
Partials 6 6
Continue to review full report at Codecov.
|
@codepretty can you please check these changes? 🙏 |
fontWeight: v.contentFontWeight, | ||
|
||
...(p.size === 'small' && { | ||
fontSize: v.sizeSmallFontSize, |
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.
Adding the medium line-height as a default will also fix this - #1376
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.
Cool, applied 👍
…at/button-size # Conflicts: # CHANGELOG.md
@@ -24,6 +24,11 @@ const Variations = () => ( | |||
description="A button can be shown in form of a text to indicate some less-pronounced actions." | |||
examplePath="components/Button/Variations/ButtonExampleText" | |||
/> | |||
<ComponentExample | |||
title="Size" | |||
description="A button can have assorted sizes." |
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.
It would be nice to consolidate the description of the size in the components' examples. We have different for Avatar, Text, Button.. :\
minWidth: height, | ||
...(p.circular && | ||
!p.text && { | ||
minWidth: v.height, |
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.
Hmm, shouldn't this be updated depending on the size? It seems like a special case..
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.
Fixes #1376.
Fixes #1671.
BREAKING CHANGES
paddingLeftRightValue
was renamed topadding
.Changes
This PR adds a new
size
prop and styles forsize=small
.I also made a small refactor to use
p
&v
pattern.