Adds CardDescription component #12105
Conversation
|
Preview: https://pf-react-pr-12105.surge.sh A11y report: https://pf-react-pr-12105-a11y.surge.sh |
dlabaj
changed the title
dlabaj
force-pushed
the
cardDescription
branch
from
104a371 to
d741b63
Compare
| ### Basic cards | ||
|
|
||
| Basic cards typically have a `<CardTitle>`, `<CardBody>` and `<CardFooter>`. You may omit these components as needed, but it is recommended to at least include a `<CardBody>` to provide details about the card item. | ||
| Basic cards typically have a `<CardTitle>`, `<CardSubtitle>`, `<CardBody>` and `<CardFooter>`. You may omit these components as needed, but it is recommended to at least include a `<CardBody>` to provide details about the card item. |
There was a problem hiding this comment.
Would we want to say that cards will typically have a subtitle, or should the subtitles be just somsthing that can be added but aren't usually there?
There was a problem hiding this comment.
this has been removed and we have a seperate demo
There was a problem hiding this comment.
To match the core structure, CardTitle needs a subtitle prop which passes the content and puts the cardSubtitle at a sibling level to the CardTitle's internal cardTitleText div. The current implementation is putting the cardSubtitle as a child of cardTitleText.
I think I would prefer making the subtitle prop represent only the ReactNode content and conditionally rendering the cardSubtitle div in CardTitle based on the prop, but I'd also be fine with keeping CardSubtitle as a component.
| /** Sets the base component to render. defaults to div */ | ||
| component?: keyof React.JSX.IntrinsicElements; | ||
| /** @beta Subtitle of the card title */ | ||
| subtitle?: string; |
There was a problem hiding this comment.
I would probably make this a ReactNode like the ModalHeader description
| expect(asFragment()).toMatchSnapshot(); | ||
| }); | ||
|
|
||
| test('className is added to the root element', () => { |
There was a problem hiding this comment.
CardSubtitle doesn't pass className anymore, and is only used internally so this test can be removed to fix the build.
There was a problem hiding this comment.
Discussed offline, but just to document it - we have 2 examples in core to show different uses of the subtitle element
- One is with a traditional card that has multiple actions to the right. This shows how the subtitle goes under the title, but if the title text wraps due to not having enough space from the actions to the right, the subtitle text will wrap with the title - which looks a little odd. It's ideal for shorter descriptions.
- The other is with an image + actions at the top, and the title is shown below both. This is a really common layout used in quickstarts and catalog view I believe. In this case, the title and subtitle span the full width of the card so you can have a super long subtitle and it looks fine
Could you update the example you have now to add actions? You don't need to make the text "really really really... long" - it can just be a normal title.
Then add the card with an action below it in the same example? There should already be an example card that looks like that you can copy/paste
| ```ts file='./CardSubtitle.tsx' | ||
|
|
||
| ``` | ||
| ### Card with subtitle and Actions |
There was a problem hiding this comment.
@mcoker Added subtitle and actions that is similar to what is in core.
|
Your changes have been released in:
Thanks for your contribution! 🎉 |
5 participants
Adds tests to ensure the CardDescription component renders correctly with PatternFly Core styles, handles className, and spreads extra props to the root element.