Conversation
e50f9f7 to
8cfa65e
Compare
scss/_patterns_content-card.scss
Outdated
| border: 1px solid var(--color-border-low-contrast, rgba(0, 0, 0, 0.1)); | ||
| display: flex; | ||
| flex-direction: column; | ||
| height: 376px; |
There was a problem hiding this comment.
The values for height: and width: here, and elsewhere, should be set to variables, and should also be in rem
| align-items: flex-start; | ||
| align-self: stretch; | ||
| flex-direction: row; | ||
| gap: 16px; |
There was a problem hiding this comment.
gap should also use rem, and probably a variable if it will be reused
|
Thank youu @Skazitron ! General:
Footer
4-Column Card Without Image:
Could we see this as part of a mockup, so e.g. like in the Figma here: Just to see how it would look in practice, especially important for the animation (here I mean the border, if not the desc.)or with varying content length – and for better understanding as to how it'll look on mobile and tablet |
| flex-direction: row; | ||
| gap: 16px; | ||
| height: 187.25px; | ||
| padding: 16px 16px 0; |
There was a problem hiding this comment.
should use default padding e.g $spv--large
scss/_patterns_content-card.scss
Outdated
|
|
||
| &:not(.p-content-card__has-image) .p-content-card__hr { | ||
| margin-left: -16px; | ||
| margin-right: -16px; |
There was a problem hiding this comment.
should use default margin
@samhotep I've spoken to design about the rule element that it's being applied to and they seem to be okay with it.
There was a problem hiding this comment.
@Skazitron I see, okay then maybe worth adding variables for these values
templates/_macros/vf_card.jinja
Outdated
| {%- endif -%} | ||
| </div> | ||
| <div> | ||
| {%- if footer -%} |
There was a problem hiding this comment.
this should include the div on line 57
| padding-bottom: var(--spacing-vertical-large, 16px); | ||
| width: 284px; | ||
|
|
||
| @media screen and (min-width: 768px) { |
There was a problem hiding this comment.
should use default breakpoint variables
|
Thanks @Skazitron! Noting that tooltips on hover and the description on hover are pending. Other than that -
Researched keyboard navigation a bit more and found we'll need to make the following changes:
|
|
@eliman11 Thanks for your suggestions. One thing to note: there is no way to make the footer clickable without losing scrollability due to limitations with HTML and CSS. I've implemented the other changes though. |
|
Thanks @Skazitron! Not sure I understand - I've recorded this Loom to explain what I meant https://www.loom.com/share/c85d39673dcb4b48bc3a59b937f7941e |
|
Thank you @Skazitron ! Animation looks super cool :D This is prob a dumb q, but say for tablet, would we be able to use the 4 col cards, or switch variants from e.g. 2 col card on desktop -> 4 col card on tablet, as tablet uses the 4 col grid Just a quick question for the 4 col card here: Is there a reason that "MLOps..." seems to wrap early? As there seems to be space following the previous word |
As for turning the 2-col cards into 4 col cards we may able to do it but we'll have to hide the author and the title. |
|
Thanks @Skazitron for the quick changes! Another question – when we specify e..g 2 col, 4 col, etc. – those are technically 25% and 50%, respectively (for tablet). But, when adjusting the window size, they remain a fixed width. Could you make sure they comply with our grid, so are responding to the containers there. E.g. equal heights |





Done
.p-content-card).vf_card) supporting configurable elements: headings, images, author/date metadata, and a footer with Vanilla icons and status chips.< 768px).< 1024px).<a>tags) and preserving the clickability of nested elements (like the author link).mask-imagegradient to the footer to gracefully fade out overflowing chips/text.QA
768px, and the 6-column card snaps to vertical below1024px.docs/patterns/content-card/index.md(Check that the macro API table and examples are accurate).Fixes
https://warthogs.atlassian.net/browse/WD-31301