fix(card): introduce large card variant #3793

DaoDaoNoCode · 2021-01-12T17:11:19Z

patternfly-build · 2021-01-12T17:14:09Z

Preview: https://patternfly-pr-3793.surge.sh

A11y report: https://patternfly-pr-3793-coverage.surge.sh

CSS Size Report

Name	Current	Previous	Diff %
components/Card/card.css	9.1 kB	7.6 kB	16.79
patternfly-no-reset.css	979.6 kB	978.1 kB	0.16
patternfly.css	981.5 kB	980.0 kB	0.16
patternfly.min.css	872.3 kB	870.9 kB	0.16

mcoker · 2021-01-12T19:37:58Z

@mcarrano @mceledonia should this follow the variation naming we introduced for the tile and CTA button (.pf-m-display-lg)?

mcoker · 2021-01-12T19:43:01Z

src/patternfly/components/Card/examples/Card.md

 | `.pf-m-no-fill` | `.pf-c-card__body` | Sets a `.pf-c-card__body` to not fill the available space in `.pf-c-card`. `.pf-m-no-fill` can be added to multiple card bodies. |
 | `.pf-m-hoverable` | `.pf-c-card` | Modifies the card to include hover styles on `:hover`. |
 | `.pf-m-selectable` | `.pf-c-card` | Modifies a selectable card so that it is selectable. |
 | `.pf-m-selected` | `.pf-c-card.pf-m-selectable` | Modifies a selectable card for the selected state. |
-| `.pf-m-flat` | `.pf-c-card` | Modifies the card to have a border instead of a shadow. `.pf-m-flat` is for use in layouts where cards are against a white background.
+| `.pf-m-flat` | `.pf-c-card` | Modifies the card to have a border instead of a shadow. `.pf-m-flat` is for use in layouts where cards are against a white background. |
+| `.pf-m-rounded` | `.pf-c-card` | Modifies the card to for the rounded corner. |


mcoker · 2021-01-12T20:14:10Z

src/patternfly/components/Card/card.scss

+  --pf-c-card--m-large--child--PaddingBottom: var(--pf-global--spacer--xl);
+  --pf-c-card--m-large--child--PaddingLeft: var(--pf-global--spacer--xl);
+  --pf-c-card--m-large--c-divider--child--PaddingTop: var(--pf-global--spacer--xl);
+  --pf-c-card--m-large__title--not--last-child--PaddingBottom: var(--pf-global--spacer--lg);


Looking at these designs, it looks like the header/title should have the --xl spacer.

In the card, the title and header are somewhat synonymous as far as the spacing is concerned. The header is just used to wrap the title if you have other things you want to put adjacent to the title. In the examples in the design, there is a "title" but that looks to just be a generic title, not necessarily the card's title element. Do you see it differently?

@mcoker Sorry for not noticing that! Thank you, will update.

mcarrano · 2021-01-12T21:39:10Z

@mcoker I'm OK to call this "Large" for lack of a better name unless @mceledonia or @ajacobs21e have a better idea.

ajacobs21e · 2021-01-12T22:21:43Z

Large sounds good to me. The only other thing I could think of is something like 'web' or 'marketing' but they're too specific.

mcoker · 2021-01-12T22:36:29Z

@mcarrano @ajacobs21e thanks! Just to clarify, the variation is currently called pf-m-large. This is inline with other components, like empty state or modal, with basic size variations.

However, when we introduced the CTA button, we decided to introduce a pf-m-display-[name] variation (#3214 (comment)) - https://pf4.patternfly.org/components/button#call-to-action, and to use that for variants of a different presentational use, like web/marketing. In theory, we could have a pf-m-large variant of a card that is just a general "large" card, and still have a pf-m-display-lg as a separate variant that is for web/marketing use.

We also used pf-m-display-lg as a variation for the tile (#3229) - https://pf4.patternfly.org/components/tile#stacked-tiles-large

Following that, I'm assuming it makes sense to call this variation pf-m-display-lg since it's specifically for web/marketing, and not just another size variant for the card? Since naming things is always hard, I just want to make sure we're all on the same page and in agreement.

mceledonia · 2021-01-13T14:13:51Z

@mcoker That makes sense to me in terms of being consistent. I do think it would make sense to just have more generic large variations without implying a use case with the name, that seems like something better suited for documentation. I would say given that the display naming convention already exists, we should probably be consistent with it for now.

mcoker

LGTM!

mattnolting

Perfect! Well done @DaoDaoNoCode! 🚢 it!

mcarrano

Looks good to me, but @mceledonia can you also take a look?

As an aside, I don't understand why the card variants/examples don't show in the right-hand POC for this page as they do for other components.

mcoker · 2021-01-16T01:28:46Z

As an aside, I don't understand why the card variants/examples don't show in the right-hand POC for this page as they do for other components.

@redallen do you know why that is? Seems like if you have more than 23 h3's, all of them disappear.

redallen · 2021-01-18T14:04:47Z

@mcoker Bump theme-patternfly-org and this should be fixed. I think the limit used to be 25 items total per design.

mceledonia · 2021-01-18T15:33:03Z

@mcarrano @gdoyle1 @ajacobs21e I am definitely late to the party here, but reviewing this I got confused by the usage of the title section. In the design examples above, there are various examples where the card header is in the body section, when I would expect it to be in the title section. I was always under the impression that the title/header section of the card was primarily for this purpose. I see the use case for an icon/image there, shown in another example, but I can see this getting confusing for users when trying to understand what to use the title section for (why not the actual title/header?).

In other words, I would expect the title section to be used for titles/headers/images, and the font size of the text inside that area to reflect that (larger than body font as seen in designs)

I first noticed this when I noticed the mismatch of font sizes in the implementation of header/body. The header section has a 16px font size and the body has 18px, which doesn't match the designs at first glance.

Any insight on the decision to use headers in the body section as seen above?

--- quick edit
Another point of confusion here is the button appearing in the footer section (example above) versus the body section here (example below)

I think we should try to provide some standardization for these, some consistency within our examples and documentation even though we don't actually limit what can live in those sections from a technical perspective.

gdoyle1 · 2021-01-19T14:41:10Z

@mceledonia I think the problem was that the spacing between the header and body was changing throughout the examples that the web team had. There was a new-ish part of the card, which was optional, which is the image/video. And then sometimes a title was placed in but didn't span across the whole card, and the spacing between the title and body often changed.

I'm good with going with whatever you suggest to standardize them all. I think it would just require us to make sure the spacer used btwn title and body is consistent

mcoker · 2021-01-19T21:42:14Z

src/patternfly/components/Card/card.scss

+  --pf-c-card--m-display-lg--child--PaddingBottom: var(--pf-global--spacer--xl);
+  --pf-c-card--m-display-lg--child--PaddingLeft: var(--pf-global--spacer--xl);
+  --pf-c-card--m-display-lg--c-divider--child--PaddingTop: var(--pf-global--spacer--xl);
+  --pf-c-card--m-display-lg__title--not--last-child--PaddingBottom: var(--pf-global--spacer--xl);


Suggested change

--pf-c-card--m-display-lg__title--not--last-child--PaddingBottom: var(--pf-global--spacer--xl);

--pf-c-card--m-display-lg__title--not--last-child--PaddingBottom: var(--pf-global--spacer--lg);

@DaoDaoNoCode confirmed with @mceledonia that everything should have an xl spacer except the space below the title, that should be lg

mattnolting

LPTM🎉

redallen · 2021-01-19T22:53:41Z

🎉 This PR is included in version 4.80.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

fix(card): introduce large card variant

4156233

DaoDaoNoCode requested review from mattnolting and mcoker January 12, 2021 17:11

DaoDaoNoCode changed the title ~~fix(card): introduce large card variant~~ [WIP] fix(card): introduce large card variant Jan 12, 2021

chore(card): add support for rounded corner

e1b290e

DaoDaoNoCode changed the title ~~[WIP] fix(card): introduce large card variant~~ fix(card): introduce large card variant Jan 12, 2021

mcoker reviewed Jan 12, 2021

View reviewed changes

chore(card): fix small issues

ccab3ae

chore(card): rename .pf-m-large to .pf-m-display-lg

529d431

mcoker approved these changes Jan 13, 2021

View reviewed changes

mattnolting approved these changes Jan 15, 2021

View reviewed changes

mcoker requested a review from mcarrano January 15, 2021 20:14

mcarrano mentioned this pull request Jan 15, 2021

Large card variant patternfly/patternfly-react#5334

Closed

mcarrano reviewed Jan 15, 2021

View reviewed changes

mcarrano requested a review from mceledonia January 15, 2021 21:25

mcoker reviewed Jan 19, 2021

View reviewed changes

DaoDaoNoCode added 2 commits January 19, 2021 16:46

chore(card): change padding bottom for card title

748380e

chore(card): change font size in large card

6e3f330

mcoker approved these changes Jan 19, 2021

View reviewed changes

mattnolting approved these changes Jan 19, 2021

View reviewed changes

mcoker merged commit 3f7c12f into patternfly:master Jan 19, 2021

redallen added the released label Jan 19, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(card): introduce large card variant #3793

fix(card): introduce large card variant #3793

DaoDaoNoCode commented Jan 12, 2021 •

edited

Loading

patternfly-build commented Jan 12, 2021 •

edited

Loading

mcoker commented Jan 12, 2021

mcoker Jan 12, 2021

mcoker Jan 12, 2021

DaoDaoNoCode Jan 12, 2021

mcarrano commented Jan 12, 2021

ajacobs21e commented Jan 12, 2021

mcoker commented Jan 12, 2021

mceledonia commented Jan 13, 2021

mcoker left a comment

mattnolting left a comment

mcarrano left a comment

mcoker commented Jan 16, 2021

redallen commented Jan 18, 2021

mceledonia commented Jan 18, 2021 •

edited

Loading

gdoyle1 commented Jan 19, 2021

mcoker Jan 19, 2021

mcoker Jan 19, 2021

mattnolting left a comment

redallen commented Jan 19, 2021

	\| `.pf-m-rounded` \| `.pf-c-card` \| Modifies the card to for the rounded corner. \|
	\| `.pf-m-rounded` \| `.pf-c-card` \| Modifies the card to have rounded corners. \|

	--pf-c-card--m-display-lg__title--not--last-child--PaddingBottom: var(--pf-global--spacer--xl);
	--pf-c-card--m-display-lg__title--not--last-child--PaddingBottom: var(--pf-global--spacer--lg);

fix(card): introduce large card variant #3793

fix(card): introduce large card variant #3793

Conversation

DaoDaoNoCode commented Jan 12, 2021 • edited Loading

patternfly-build commented Jan 12, 2021 • edited Loading

mcoker commented Jan 12, 2021

mcoker Jan 12, 2021

Choose a reason for hiding this comment

mcoker Jan 12, 2021

Choose a reason for hiding this comment

DaoDaoNoCode Jan 12, 2021

Choose a reason for hiding this comment

mcarrano commented Jan 12, 2021

ajacobs21e commented Jan 12, 2021

mcoker commented Jan 12, 2021

mceledonia commented Jan 13, 2021

mcoker left a comment

Choose a reason for hiding this comment

mattnolting left a comment

Choose a reason for hiding this comment

mcarrano left a comment

Choose a reason for hiding this comment

mcoker commented Jan 16, 2021

redallen commented Jan 18, 2021

mceledonia commented Jan 18, 2021 • edited Loading

gdoyle1 commented Jan 19, 2021

mcoker Jan 19, 2021

Choose a reason for hiding this comment

mcoker Jan 19, 2021

Choose a reason for hiding this comment

mattnolting left a comment

Choose a reason for hiding this comment

redallen commented Jan 19, 2021

DaoDaoNoCode commented Jan 12, 2021 •

edited

Loading

patternfly-build commented Jan 12, 2021 •

edited

Loading

mceledonia commented Jan 18, 2021 •

edited

Loading