demos(Card): added horizontal split and details demos #6193
Conversation
|
PF4 preview: https://patternfly-react-pr-6193.surge.sh |
There was a problem hiding this comment.
Looks good to me. @mattnolting can you also take a look? The only question I had was why this URL on the right is not linked:
It's that way in the core example, too. I wasn't sure if this was by intent or just an oversight.
| Cluster API Address | ||
| </DescriptionListTerm> | ||
| <DescriptionListDescription> | ||
| https://api2.devcluster.openshift.com |
There was a problem hiding this comment.
This feels a bit odd to see a link as plain text. Could we change this to a link, or change the description/descriptionListTerm to use less link-looking text?
There was a problem hiding this comment.
Yeah this seems like an oversight. I think we should link this text like the other card - @mcarrano do you agree?
There was a problem hiding this comment.
Yup. I agree. it looks odd. This is how the core implementation looks. @mnolting can you see @mcarrano's question above
#6193 (review)
There was a problem hiding this comment.
It looks like neither of the URLs are linked in the original design - https://marvelapp.com/prototype/eb14d81/screen/70405126
There was a problem hiding this comment.
Yes, Let's just make that a link for consistency.
There was a problem hiding this comment.
@mcoker Do you want to open a core issue for the update.
| CardHorizontalSplitDemo = () => { | ||
| return ( | ||
| <Card id="card-demo-horizontal-split-example" isFlat> | ||
| <Grid md={6} hasGutter> |
There was a problem hiding this comment.
| <Grid md={6} hasGutter> | |
| <Grid md={6}> |
| <Grid md={6} hasGutter> | ||
| <GridItem style={{minHeight: '200px', backgroundPosition: 'center', backgroundSize: 'cover', backgroundImage: 'url(/assets/images/pfbg_992@2x.jpg)' }}/> | ||
| <GridItem> | ||
| <CardTitle>Headline</CardTitle> |
There was a problem hiding this comment.
The <Card*> elements should be wrapped in <Card>. We're missing this on the core side, too - patternfly/patternfly#4310
There was a problem hiding this comment.
@mcoker I am confused about the structure. I assumed the <Card> on line 273 was the wrapping element here.
There was a problem hiding this comment.
Ah hah! I missed that, ignore my comment 🤦♂️
|
Your changes have been released in:
Thanks for your contribution! 🎉 |
What: Closes #6122 and closes #6121