Conversation
f792e7f to
cb45367
Compare
| /* ── Layout: horizontal ────────────────────── */ | ||
|
|
||
| .card-group--horizontal { | ||
| max-width: 933px; |
There was a problem hiding this comment.
@julhoang could you point me to the figma component link related to card groups, please?
I'm just trying to understand where these measures are coming from, like 933px exactly.
There was a problem hiding this comment.
@herzog0 Yes sure thing – Figma link.
Also #naming-is-hard, if you have any suggestion for a better component name for card-group please let me know 😅 I understand the current one is not the most intuitive but it's the best I could come up with
cb45367 to
70fe57c
Compare
There was a problem hiding this comment.
@julhoang sorry for only noticing this now, but can we take the opportunity to also improve our DX a little bit?
Right now, since the components are asking for flat variables instead of a dict containing all the properties, using these components can be a little verbose.
For example, taking the _card_group.html as an example:
{% if heading or items %}
<section class="card-group card-group--{{ variant|default:'list' }}{% if variant == 'card' %} card-group--{{ theme|default:'default' }}{% endif %} card-group--{{ layout|default:'vertical' }}"{% if heading %} aria-labelledby="card-group-{{ heading|slugify }}"{% endif %}>
{% if heading %}
<h2 class="card-group__heading" id="card-group-{{ heading|slugify }}">{% if heading_url %}<a href="{{ heading_url }}">{{ heading }}</a>{% else %}{{ heading }}{% endif %}</h2>
{% endif %}
{% block card_list %}
{# Consumers will fill this block with their item loop #}
{% endblock %}
{% if primary_cta_url or secondary_cta_url %}
<div class="card__cta_section">
{% if primary_cta_url %}
{% include "v3/includes/_button.html" with label=primary_cta_label url=primary_cta_url style=theme|default:"primary" only %}
{% endif %}
{% if secondary_cta_url %}
{% include "v3/includes/_button.html" with label=secondary_cta_label url=secondary_cta_url style="secondary" only %}
{% endif %}
</div>
{% endif %}
</section>
{% endif %}Right now, if we want to use a _post_card, we have to be fully explicit with the arguments being passed, like:
{% with d=posts_from_the_boost_community %}
{% include "v3/includes/_post_card.html" with heading=d.heading items=d.items variant=d.variant theme=d.theme primary_cta_label=d.primary_cta_label primary_cta_url=d.primary_cta_url %}
{% endwith %}But notice how the backend is already hydrating all the properties with the same specified names in the _card_group and _post_card definitions, so it'd be much better if we could just say:
{% include "v3/includes/_post_card.html" with data=posts_from_the_boost_community only %}For that to work you'd have to update the components to use a data. parameter prefix on every property.
Let me know your thoughts!
Changed my mind, as this verbosity is a design choice by Django
QA Validation note:All the components: Post Card, Event Card, Testimonial Card, Cards Carousel and Buttons are working as expected. |
Summary & Context
Refactor post-card and event-card components into a shared
_card_group.htmlbase template using Django template inheritance , and add a new "card" variant for Post Cards (previously only "list" existed). This PR also cleans up widespread misuse ofpost-cardsCSS classes across unrelated components (event cards, testimonial cards, carousel, examples section).Changes
_card_group.htmlbase template that provides the shared shell (heading, list slot, CTA section) via{% block card_list %}— consumers extend it to render their own items for the card body.card-group.csswith shared container styles, theme variants (default, yellow, green, teal), card vs list variants, horizontal layout, and dark mode_post_card.htmlextending_card_group.htmlwith support for bothvariant="list"andvariant="card"layouts, plus theme and layout options_event_card.htmlextending_card_group.htmlto render event itemspost-card.cssandevent-card.cssas ⭐️ focused, single-card stylesheets ⭐️_testimonial_card.htmlto use its own carousel markup instead of depending onpost-cardsclasses; improve heading semantics and aria attributes_cards_carousel_v3.html— extract carousel styles into standalonecards-carousel.css, removing dependency onpost-cards.cssbuttons.cssto use semantic tokens for secondary hover and add dark-mode overrides for colored button variantsNotable Components Affected
templates/v3/includes/_card_group.html(new)templates/v3/includes/_post_card.html(new, replaces_post_card_v3.html+_post_cards_v3.html)templates/v3/includes/_event_card.html(new, replaces_event_cards.html+_event_cards_section.html+_content_event_card_item.html)templates/v3/includes/_testimonial_card.html(refactored to include_testimonial_card_quote.html)templates/v3/includes/_cards_carousel_v3.html(refactored)static/css/v3/card-group.css(new)static/css/v3/post-card.css(new, replacespost-cards.css)static/css/v3/event-card.css(new, replacesevent-cards.css)static/css/v3/cards-carousel.css(new, extracted frompost-cards.css)This refactor would require a re-QA on all of the impacted components.
Screenshots (Staging left VS localhost right)
The best way to compare Before & After in this case is probably to put these Staging side-by-side with localhost.
Self-review Checklist
Frontend