Card Style Update

This is not yet ready from a design side

Where's the card component used? Would it be possible to get more background information about the component in the issue summary and/or link to Figma?

The first pass of the component has been updated in the Figma file, see file above ^^ for reference.

My initial thoughts:

• Will there ever be any other details on the cards or is this the full extent of options we need? Longer descriptions? Can we truncate them if they do get to wordy?
• If we are okay with the amount of content here now, I can shrink the size of the thumbnail so they don't take up so much vertical height.

@l2g2 thanks for your design proposal. I've left a comment in Figma regarding some questions. cheers :)

The second pass of the cards has been implemented in the Figma document.

There are 3 different layouts, with different spacing scales.

Within those options, they were split into a single column, two column, and four column layout.

Screenshot of 3 options in single column layout attached for reference:

Next version of feedback and adjustments has been implemented. The feedback can be found in Figma on the artboards Card 02.2 and Card 02.3.

Reminder for the variety of card layouts:

• Single Column
• Two Column
• Four Column

Adjustments in Card 02.2:

• Utilize Card Layout With No Bottom Border
• Edge-To-Edge Image
• Padding Adjustments To Match Final Components
• Border-Radius Update

Adjustments in Card 02.3:

• Variants of Body Copy Length
• Variants of Heading Length
• Variants of CTA Items (Buttons)
• With and Without Image
• Mixed Views (with and without image in one view)

There is a screen shot attached for a quick-look, but take a closer peek in Figma! I think we are at the finish line with these, next steps is to move into the Final components category pending any last words of wisdom/feedback.

@saschaeggi @ckrina

Based on the comments above, I think this needs to go back to needs review.

As discussed with @lauriii, this should be ready for implementation.
I will start working on it.

I'll continue on this.

I did a start on this to apply the styles to the theme section as I think this is the main purpose of this issue (maybe the issue title is a bit misleading).
I left some tasks because I think this first needs a review.

TODO:
1. style the action links as buttons (apply the button classes to the links)
2. apply different spacings based on card with image (spacing m) and without (spacing l) and different padding spacings for horizontal alignment and vertical alignment
3. accessibility and ltr/rtl check

For point 2: This is a bit confusing in the design. There are different padding spacings for cards with image and without and different padding spacings for horizontal view and vertical view. Check 1col vs 2col vs 4col. Can we agree on a more unified spacing?

1) from my point of view if we have totally override in template we may use .card and .card__image classes in addition to .theme-info etc.
2) also it seems me useless to have same code in 2 places (it also duplicated in css/dist/components/system-admin--appearance.css)

Reworked this patch because i'm sure that Card component can be useful in other places aswell.

1. Added styles for .card bem component. Now it is independent from themes list and can be reused by any other modules.
2. Added independent component Cards List which can be reused.
3. Cleaned system-admin--appearance.css since all styles there duplicate current. Except .incompatible class which is only theme related but not to card component.
4. Removed .toolbar-vertical related conditions, since now card is pretty flexible and this is overhead to keep it related to toolbar actions.
5. Attached button classes to themes actions using claro_preprocess_links. Maybe there is better option to do it. Better backend knowledge needed :)

TODO:

1. Define behaviour for themes in installed/uninstalled states. For now installed themes always takes fullwidth(and became vertical on mobile).
   Uninstalled themes fullwidth on mobile, 2 cols on 768-1200, 4 cols on 1200+
2. Check browsers(flexbox)
3. Check image width on different screen resolutions in themes list. I think it is not cool to stretch it to min-width: 100%, and sometimes it is smaller than wrapper.

Answering #22, the installed themes should ideally go with the 2 col horizontal, and the uninstalled with 4 col vertical.
Also, about the min-with: 100%, I agreed on that it's not a good solution: we shouldn't be scaling up images. But also images shouldn't be smaller than the card, so maybe different breakpoints should be used.

Also reviewing the patch I see all cards that are using the regular button size and only the full width one should be using a smaller button size.

About the question previously raised about making all paddings the same, I've aligned most of them to the L space (24px/1,5em) and but one of them can't be aligned: the one on in the 2 col Horizontal because we really need the space when we have images + long texts.

Thanks all for all the great work!

Hello all!

1) Added `button--small` class by default for buttons. We don't have fullwidth card here. So seems all buttons are small.

2) Added marginless `card__button--last` class since buttons rendered inside list and it is better to remove last item margin.
Actually it seems me wrong to add margin to button component by default. And it is obviously antiBEM. But it isn't problem of current component.

3) Managed layouts.

Now: installed themes
1 item on 0-587px(screenshot width) and vertical display
1 item on 588px-1365px and horizontal display
2 items on 1366px+ and horizontal display

uninstalled themes always vertical
1 item on 0-587px
2 items on 588px - 1199x
3 items on 1200px - 1365px (i had to add 3 items in list to avoid image stretch)
4 items on 1366px+

4) Managed headings used .heading-f system class. Will it be supported in future?

5) Added twig condition to know when image on the place. to make content pading smaller.

6) Tested in latest safari, opera, chrome, ff, edge(looks good)
...and IE11(looks more or less, after setting max-width to grid items).

Please test

Reviewing.

Re #25:

This is a huge step forward, thank you all!

Issues that should be resolved:

1. Coding standard:
   

   FILE: claro.theme
   ----------------------------------------------------------------------
   FOUND 2 ERRORS AFFECTING 2 LINES
   ----------------------------------------------------------------------
    844 | ERROR | [ ] If the line declaring an array spans longer than
        |       |     80 characters, each element should be broken into
        |       |     its own line
    845 | ERROR | [x] Expected 1 space after IF keyword; 0 found
   ----------------------------------------------------------------------
   PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
   ----------------------------------------------------------------------
   

2. +++ b/claro.libraries.yml
   @@ -13,6 +13,8 @@ global-styling:
   +      css/dist/components/card.css: {}
   +      css/dist/components/cards-list.css: {}
   
   +++ b/css/src/components/cards-list.css
   index 03b0f24..639b609 100644
   --- a/css/src/components/system-admin--appearance.css
   
   --- a/css/src/components/system-admin--appearance.css
   +++ b/css/src/components/system-admin--appearance.css
   
   +++ b/css/src/components/system-admin--appearance.css
   @@ -6,153 +6,7 @@
   -.theme-info__header {
   -  margin-bottom: 0;
   -  font-weight: normal;
   -}
   -.theme-default .theme-info__header {
   -  font-weight: bold;
   -}
   -.theme-info__description {
   -  margin-top: 0;
   -}
   

   We replace the whole system/admin library with our own claro/system.admin.
   The system-admin--appearance.css is one of the CSS asset in that replacement library.

   With the current implementation, since card and card list styles are provided by Claro's global styles, these are loaded unconditionally, for every page, but I don't think that this is a good practice.

   Instead of adding these to global styles, I ask you to define a standalone library for card and card-list components, and add this new library as dependency to our claro/system.admin. This way we wont bloat our global styles.

3. +++ b/claro.theme
   @@ -832,6 +832,27 @@ function claro_preprocess_links__dropbutton(&$variables) {
   +/**
   + * Implements hook_preprocess_HOOK() for links in themes list.
   + */
   +function claro_preprocess_links(&$variables) {
   +  if ($variables['attributes'] && $variables['attributes']['class'] && in_array('operations', $variables['attributes']['class'])) {
   +    $variables['attributes']['class'][] = 'card__buttons';
   +    if (!empty($variables['links'])) {
   

   I find this a little dangerous: this hook is too general.

   Please implement template_preprocess_system_themes_page() instead of this and make the needed modifications there!

4. +++ b/templates/system-themes-page.html.twig
   @@ -40,36 +40,41 @@
   +                theme.is_default ? 'theme-default',
   +                theme.is_admin ? 'theme-admin',
   ...
   +                'theme-selector',
   ...
   +              <div class="card__content {{ theme.screenshot ? 'card__content--with-image' : '' }} theme-info">
   +                <h3 class="heading-f card__header theme-info__header">
   ...
   +                <div class="card__description theme-info__description">{{ theme.description }}</div>
   
   @@ -77,37 +82,43 @@
   +                theme.is_default ? 'theme-default',
   +                theme.is_admin ? 'theme-admin',
   ...
   +                'theme-selector',
   ...
   +              <div class="card__content theme-info">
   +                <h3 class="heading-f card__header theme-info__header">
   ...
   +                <div class="card__description theme-info__description">{{ theme.description }}</div>
   

   I think that we don't use .theme-selector, .theme-default, .theme-admin, .theme-info .theme-info__header, and .theme-info__description CSS classes anymore. If I'm right, please remove these from the template!

5. +++ b/templates/system-themes-page.html.twig
   @@ -40,36 +40,41 @@
   +                'cards-list__item',
   
   @@ -77,37 +82,43 @@
   +                'cards-list__item',
   

   I would separate card component styles and card-list layout styles completely, including their usage in the template.

   So instead of adding the .card-list__item layout style to the .card component root, use a standalone .card-list__item element (<div class="card-list__item">) and wrap .card into that!

6. +++ b/css/src/components/card.css
   @@ -0,0 +1,114 @@
   +.card {
   ...
   +  overflow: hidden;
   ...
   +.card .card__button {
   ...
   +  white-space: nowrap;
   

   This is a really wrong combination.

   Why are these needed?

7. +++ b/css/src/components/card.css
   @@ -0,0 +1,114 @@
   +.card__content {
   ...
   +  flex-grow: 0;
   +  flex-shrink: 0;
   ...
   +.card--vertical .card__content {
   +  flex-basis: auto;
   +}
   ...
   +@media screen and (min-width: 588px) {
   +  .card__content {
   +    flex-basis: 65%;
   +    flex-grow: 1;
   +  }
   +  .card__description {
   +    flex-grow: 1;
   +  }
   ...
   +@media screen and (min-width: 588px) {
   +  .card__image {
   +    flex-basis: 35%;
   +  }
   +  .card--vertical .card__image {
   +    flex-basis: auto;
   +  }
   +}
   ...
   +@media screen and (min-width: 1366px) {
   +  .card__image {
   +    flex-basis: 45%;
   +  }
   +  .card--vertical .card__image {
   +    flex-basis: auto;
   +  }
   +}
   
   +++ b/css/src/components/cards-list.css
   @@ -0,0 +1,115 @@
   +.cards-list--two-cols .cards-list__item,
   +.cards-list--four-cols .cards-list__item {
   +  flex-basis: 100%;
   +}
   ...
   +@media screen and (min-width: 1200px) {
   +  .cards-list--four-cols .cards-list__item {
   +    max-width: var(--cards-three-cols-width);
   +    flex-basis: var(--cards-three-cols-width);
   +  }
   

   This is not a blocker, just my opinion: I'd use the flex shorthand instead of flex-grow, flex-shrink or flex-basis (just because it's easier to track what's happening and why) if they aren't needed because of a special concept.

8. Please use relative units for the media breakpoints! If the default browser font size is bigger than your 16px, it will be really hard to use this page!

   

9. Please cover the cases when vertical toolbar is expanded! See the screenshot above.

   A working approach can be found here (/css/src/layout/system-admin--layout.css).

10. The space between the card description and the operations does not follows the design (it equals to 1rem instead of 1.5rem)

3. I mentioned in #22 that backend should be updated.
5. It may need some additional properties and from bem side it is normal to mix elements with blocks. But yeah - why not.
6. white-space: nowrap; was needed because sometimes button became 2 lined what looked ugly.
7. Shorthand flex not works well with IE

Gonna prepare next patch tomorrow.

Hello. Thanks for review.

1) Fixed. But maybe new one appears. Please check
2) Removed. But system-admin--appearance.css still contains only themes related component, and not related to cards as is. So idk what to do with it
3) Replaced. Looks ugly:) Backenders please help.
4) "If I'm right, please remove these from the template!" i don't know how to check it. Keeped as is for now.
5) It will make things more complicated. And will not give lot of clean. Since as i said it is ok to mix Blocks and Elements because their properties are different.
So here .card-list__item->`external geometry` and .card->`inner styles` So we shouldn't have collisions.
Otherwise we will need to add additional styles to cover flexbox stretches.
6) overflow: hidden; removed. white-space: nowrap; keeped because it is required to have button in one line.
7) Keeped as is to support IE 11
8) Updated.
9) Added styles for small screen only, Cards looks good on bigger screens even with toolbar.
10) Updated.

From previous comments:

6. overflow:hidden is needed for aesthetic purposes. And flex-wrap: wrap is used to be sure the buttons jump into a new line. So I don’t see anything wrong combining them.
Related to this, Ive seen now the card has border-radius: 2px 2px 0 0;. This is wrong: if you zoom in on Figma spec you’ll see all 4 corners have the same border radius :)
So I'd suggest setting border-radius: 2px; and overflow: hidden; on the .card itself, but if you want to also avoid @huzooka's concern you could move the overflow: hidden to the image wrapper .card__image and move there the top&bottom left border radius there too.

New comments:
11. We shouldn't be using the word buttons to define the region .card__buttons. Actually, they actually shouldn’t be buttons based on #3030987: Refactor usage of button component on links. I’ll create a follow-up to change the appearance of the links/buttons because I think it’s out of the scope of this issue and it'll need design discussion.
So I propose using .card__footer or .card__actions because it’s more generic and can handle also links too.

12. We're setting the width for the .card__image to 45% from min-width: 85.375rem. So it means we should add:

@media screen and (min-width: 85.375rem) {
  .card__content {
    flex-basis: 55%;
  }
}

Thanks for all the work on this!

Fixed according #31

Thanks @finnsky!

1. Sorry for jumping on it later, but after discussing it with @lauriii we think cards should also use Action links on the Appearance page because of #3030987: Refactor usage of button component on links. So the buttons should changed to Actions links in this issue. Here’s the updated design, but probably we should get #3036732: Action link component before moving forward with this one.

2. Breakpoints: on the installed themes section, I think the card should jump to use the 2 cols sooner: probably around 940-960px. Otherwise the thumbnail is too big.

How should we deal with extra-wide monitors? The same issue where the thumbnail renders too large happens on two-column cards on screens widger than ~1920px.

I didn't thought on that. I'd say let's add a max-width to the cards, and if they don't reach the end of the page it's OK.

Sounds good. Could we describe this in the design system? We should know what's the max-width, and how the elements are positioned when they are smaller than the page width.

Discussed with @ckrina and she opened #3076820: [META] Layout redesign as a result. We decided to not worry about the large screens on this issue since it should be addressed globally.

After a discussion with @lauriii we agreed on moving the actions (buttons and Action links) to the right to avoid to modify the left padding on Action links. Here's the screenshot, but it's already changed on Figma's specs.

I'm a bit surprised why #30 removed 'details.css' from global library assets and added 'card.css' instead of that...

That's completely wrong. But we need a re-roll here.

Here's the re-roll of #30.

Runned into an IE11 flexbug: https://github.com/philipwalton/flexbugs/issues/233.

Screenshots attached.

The patch doesn't apply anymore.

Also, cards without image should not have an image. See specs:

PD. Thanks for the screenshots, really useful to review the design!

Re #47:
A card component without an image wont have the 'no-screenshot' placeholder. But for themes, core adds this image if the theme does not define a screenshot path.

Do you want Claro to remove those 'no-screenshot' images for theme cards?

(BTW we're hard-blocked here because of this appearance page lacks of spacing documentation.)

Looks like reroll is not needed against 8.x-1.x, i can apply patch from #45

Talked with @ckrina and she said that we could remove the no-screenshot image in a follow-up.

We also need another issue for fixing the heading spacings. We probably shouldn't commit this issue before that has been committed to avoid regressions on the appearances page.

Opened #3083004: Update heading spacings and #3083003: Remove default no-screenshot image on appearance page.

This looks good now. I made minor code style improvements on commit. Interdiff attached. Thank you all!