feat(DescriptionList): add new DescriptionList component - #4586
Conversation
|
PF4 preview: https://patternfly-react-pr-4586.surge.sh |
jenny-s51
changed the title
There was a problem hiding this comment.
This looks good to me @jenny-s51 . @maryshak1996 can you also give this a look?
|
@mcarrano @jenny-s51 I think that this is looking good! The only thing that I think that we should update would be the default spacing in between columns from 16px to 24. The tightness of the columns makes it a little difficult to distinguish between where one description pair starts and another ends (screenshot of what I'm referring to below) |
jschuler
left a comment
jschuler
left a comment
There was a problem hiding this comment.
LGTM, just comment on class string
| ...props | ||
| }: DescriptionListDescriptionProps) => ( | ||
| <dd className={css(styles.descriptionListDescription, className)} {...props}> | ||
| <div className={'pf-c-description-list__text'}>{children}</div> |
There was a problem hiding this comment.
is this class defined on styles.descriptionListText?
There was a problem hiding this comment.
No it isn't @jschuler - there is no .pf-c-description-list__text class or corresponding styles in https://github.com/patternfly/patternfly/blob/master/src/patternfly/components/DescriptionList/description-list.scss, but this class is still applied in the core examples for some reason
| ...props | ||
| }: DescriptionListTermProps) => ( | ||
| <dt className={css(styles.descriptionListTerm, className)} {...props}> | ||
| <span className={'pf-c-description-list__text'}>{children}</span> |
|
Your changes have been released in:
Thanks for your contribution! 🎉 |
9 participants
What: Closes #4523