-
Notifications
You must be signed in to change notification settings - Fork 4.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(Comment): add size prop to Group #1327
feat(Comment): add size prop to Group #1327
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1327 +/- ##
=======================================
Coverage 99.75% 99.75%
=======================================
Files 140 140
Lines 2402 2402
=======================================
Hits 2396 2396
Misses 6 6
Continue to review full report at Codecov.
|
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.
And we need add size
to typings.
src/views/Comment/CommentGroup.js
Outdated
@@ -57,6 +59,9 @@ CommentGroup.propTypes = { | |||
/** Comments can hide extra information unless a user shows intent to interact with a comment. */ | |||
minimal: PropTypes.bool, | |||
|
|||
/** Comments can have different sizes*/ | |||
size: PropTypes.oneOf(['mini', 'tiny', 'small', 'large', 'big', 'huge', 'massive']), |
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.
We use SIZES
const for definitions, example.
@@ -8,4 +8,7 @@ describe('CommentGroup', () => { | |||
common.propKeyOnlyToClassName(CommentGroup, 'collapsed') | |||
common.propKeyOnlyToClassName(CommentGroup, 'minimal') | |||
common.propKeyOnlyToClassName(CommentGroup, 'threaded') | |||
common.propValueOnlyToClassName(CommentGroup, 'size', [ | |||
'mini', 'tiny', 'small', 'large', 'big', 'huge', 'massive', |
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.
^
@layershifter alright, fixed up. let me know. |
Fantastic, we just need a doc example for "Group Size" now. Here are some existing group size doc examples for comparison: Examples/elements/Button/GroupVariations/ButtonExampleGroupSize.js The comment group size should be listed in |
@levithomason I've added doc site examples as per your request. Only thing I noticed was that the props for size was listed this way: |
@levithomason I think this should wrap up this fix! |
size
prop
Thanks again guys! |
Released in |
* fix(Comment.Group): added size prop * fix(Comment.Group): appending size prop value to className, added tests * fix(Comment.Group): refactor to use SUI sizes * fix(Comment.Group): fix typings * Update index.d.ts * Update CommentGroup-test.js * Update CommentGroup.js * Update index.d.ts * docs(Comment.Group): added comment group size examples to doc site
Fixed #1311