feat: add Card component #28
Conversation
✅ Deploy Preview for storybook-ui-seeds ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
|
@slowbot Just want to run a couple things by you regarding the props…so I ended up creating a I also decided to try out t-shirt sizing for the |
|
@jaredcwhite The spacing looks great. Is there an option to have "none" Is there a reason that the card header doesn't consume "heading" or "heading group" as a subcomponent? |
|
|
src/blocks/Card.tsx
Outdated
| return ( | ||
| <header id={props.id} className={classNames.join(" ")} data-divider={props.divider} data-flex-children={!!props.suffix}> | ||
| {props.children} | ||
| {props.suffix} |
There was a problem hiding this comment.
This suffix feature is kind of weird to me. It could just be a personal preference, but I'd prefer all of the content to be in the children. In fact with your current setup when I do the following it looks the same as if I put it in the suffix.
<Card.Header divider="flush" className="test-card-header">
<>
<h3>Wildflower</h3>
<div>Second part of it</div>
</>
</Card.Header>
That's because when you have no suffix prop passed it outputs as data-flex-children="false" so the css is applied.
I'll lean on the rest of the team on what syntax people prefer, but at minimum we should probably fix the issue where the css is only getting applied when we want it to.
There was a problem hiding this comment.
Yeah, I believe that was a holdover from a past attempt to work on the idea of adding an icon or close button or something else to the opposite end of the header, but we might just instead want a flex/grid child to get added rather than bake it in the Header. I'm fine with just taking that prop out.
| --card-header-padding-inline: var(--card-header-padding-inline-desktop); | ||
| --card-content-padding-inline: var(--card-content-padding-inline-desktop); | ||
|
|
||
| @media (max-width: 640px) { /* small screen */ |
There was a problem hiding this comment.
Do we / could we potentially have 640 as a var somewhere?
There was a problem hiding this comment.
Yeah, we do have screen values as tokens in the CSS, but unfortunately media queries are one area where native CSS variables aren't supposed, so we'll have to fallback to something like Sass variables (which is what happens in ui-components, but it's not currently in ui-seeds). I'll file a separate ticket to investigate a good solution.
|
🎉 This PR is included in version 1.5.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This PR addresses #22
Description
Adds a Card component to the Seeds design system.
How Can This Be Tested/Reviewed?
Review the Card component in Storybook: https://deploy-preview-28--storybook-ui-seeds.netlify.app/?path=/story/blocks-card--text-content
Checklist: