chore(docs): clean up cards examples and fix layout issues #6415
Conversation
🦋 Changeset detectedLatest commit: 1e80ddf The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
hugomslv
changed the title
alizedebray
left a comment
alizedebray
left a comment
There was a problem hiding this comment.
Can you update the PR title? It should be like:
chore({package}): {description}
| return html` | ||
| ${['Link Text', 'More Link'].map(label => html` <a class="card-link" href="#">${label}</a> `)} | ||
| <div class="card-links"> | ||
| ${['Link Text', 'More Link'].map(label => html` <a href="#">${label}</a> `)} |
There was a problem hiding this comment.
Can you update this to have only one link in the card to fit what's in Figma?
Also the links must have tertiary button classes (you can check the button documentation to see which one to use).
There was a problem hiding this comment.
Classes for the link are not correct, please check the documentation: https://preview-6415--swisspost-design-system-next.netlify.app/?path=/docs/eb78afcb-ce92-4990-94b6-6536d5ec6af4--docs&story=Default&args=tag:a;variant:btn-tertiary%20px-0;iconPosition:iconAfterText#eb78afcb-ce92-4990-94b6-6536d5ec6af4--default
packages/documentation/src/stories/components/card/card.stories.ts
Outdated
Show resolved
Hide resolved
| function getCardButton() { | ||
| return html` | ||
| <button class="btn btn-primary"> | ||
| <button class="btn btn-primary "> |
There was a problem hiding this comment.
To match Figma, it would be good to have 3 buttons here: a primary, a secondary and a tertiary.
There was a problem hiding this comment.
I see you've added a "Variant" control for the button, what I meant was have the 3 buttons in the card as designed here: https://www.figma.com/design/JIT5AdGYqv6bDRpfBPV8XR/Foundations---Components-Next-Level?node-id=9908-19099&m=dev
| decorators: [gridContainer], | ||
| render: (args: Args) => | ||
| html`${args.action === 'button' ? renderInteractiveCard(args) : renderNoninteractiveCard(args)}`, | ||
| html`${args.action === 'button' |
There was a problem hiding this comment.
Can you add a new "interactive" argument to control this independently of the selected action?
Ideally we would have the following controls:
- Title (string)
- Body (string)
- Interactive (boolean)
- Action (buttons, links, or none) - only if Interactive is false
- Interactive Action (button or link) - only if Interactive is true
There was a problem hiding this comment.
Please update the "Interactive" control and move it to the "Card Content" section.
The goal of this control is to always have this behavior:
- The card is interactive (i.e. is is a
post-linkarea) if there is one and only one link or button - The card is not interactive (i.e. it is a
div) if there is no or multiple links or buttons
Therefore depending on whether user select true or false for the "Interactive" control, you should show 0, 1 or multiple links or buttons:
| Interactive | (Interactive) Action | Container | Content |
|---|---|---|---|
| true | button | post-linkarea | text + 1 full-width button (primary) |
| true | link | post-linkarea | text + 1 link (tertiary no padding) |
| false | buttons | div | text + 3 full-width buttons in a column (primary, secondary, tertiary) |
| false | links | div | text + 2 links in a row (tertiary no padding) |
| false | none | div | text only |
|
There was a problem hiding this comment.
Please check my comments above.
Also the PR title is still incorrect.
hugomslv
changed the title
📄 Description
clean up card documentation
🚀 Demo
If applicable, please add a screenshot or video to illustrate the changes.
🔮 Design review
📝 Checklist