feat(Card): add textAlign prop to Card subcomponents #2038
Conversation
itamar244
changed the title
itamar244
changed the title
Codecov Report
@@ Coverage Diff @@
## master #2038 +/- ##
=======================================
Coverage 99.76% 99.76%
=======================================
Files 148 148
Lines 2561 2561
=======================================
Hits 2555 2555
Misses 6 6
Continue to review full report at Codecov.
|
itamar244
changed the title
|
ready for a merge |
| @@ -24,5 +24,6 @@ describe('CardContent', () => { | |||
| mapValueToProps: val => ({ content: val }), | |||
| }) | |||
|
|
|||
| common.implementsTextAlignProp(CardContent, ['left', 'center', 'right']) | |||
There was a problem hiding this comment.
Lets use _.without(SUI.TEXT_ALIGNMENTS, 'justified') in tests.
There was a problem hiding this comment.
Maybe a new constant equals to ['left', 'center', 'right'] is needed?
It is smaller when bundling and faster.
There was a problem hiding this comment.
I don't think that there is real necessity. This code doesn't work in production builds.
src/views/Card/CardGroup.js
Outdated
| @@ -26,11 +27,13 @@ function CardGroup(props) { | |||
| items, | |||
| itemsPerRow, | |||
| stackable, | |||
| textAlign, | |||
| } = props | |||
|
|
|||
| const classes = cx('ui', | |||
There was a problem hiding this comment.
Lets add newline there:
-const classes = cx('ui',
+const classes = cx(
+ 'ui',
src/views/Card/CardMeta.js
Outdated
| const { children, className, content } = props | ||
| const classes = cx(className, 'meta') | ||
| const { children, className, content, textAlign } = props | ||
| const classes = cx(className, useTextAlignProp(textAlign), 'meta') |
There was a problem hiding this comment.
Lets make this multiline:
const classes = cx(
className,
useTextAlignProp(textAlign),
'meta',
)
src/views/Card/CardContent.js
Outdated
| } = props | ||
|
|
||
| const classes = cx( | ||
| className, | ||
| useKeyOnly(extra, 'extra'), | ||
| useTextAlignProp(textAlign), | ||
| 'content', | ||
| ) |
There was a problem hiding this comment.
As we update almost all Card components, lets also move className to end, it's one of our style rules. I overlooked this on initial review.
cx(
useKeyOnly(extra, 'extra'),
useTextAlignProp(textAlign),
'content',
className,
) There was a problem hiding this comment.
Should i first write 'ui', then functions, then other literals and then className?
| @@ -1,3 +1,6 @@ | |||
| import _ from 'lodash'; | |||
There was a problem hiding this comment.
Heads up, tests are failing due to lint:
/home/ubuntu/Semantic-UI-React/test/specs/views/Card/CardContent-test.js
1:23 error Extra semicolon semi
You can run npm run lint:fix to fix most errors and then push. Note that you can ignore warnings.
|
You guys are great! Thanks again @layershifter for always filling in for me ❤️ |
|
Released in |
* feat(Card): add textAlign prop to CardContent * feat(Card): add textAlign to all Card subcomponents * styles(Card): fix code style inconsistencies * refactor(Card): change array literals functions calls * fix(Card): fix lint errors * style(Card): fix classes order
Fixes #2027