-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
More card variants with different media options - image icon, video, embed #451
Conversation
…tons except for icon variants
…t takes either custom file path or iframe and emit different markup
@sherakama @JBCSU , I still need to merge master (will have some conflicts since there were so many card branches) but please feel free to leave any comments. |
* master: (28 commits) Updated packages and configuration files. (#527) Use rem instead of em for big button font size so it doesn't blow up in sections if we set a large base font size; finetune big button padding for smaller breakpoints (#522) Change sizes of link icons to use px instead of em to prevent occasional clipping (#523) D8CORE-628: Add caption option to hero banner. (#517) Use percentage for html root font size instead of px so people can scale up using browser font size preferences (#526) Mixins documention update — @centered-column (#448) Adds Tugboat and fixes npm dependencies. (#518) Update _hero.scss (#513) Create Aspect Ratio Mixin (#445) Adding global footer variants and link tweaks for legibility, change layout at MD breakpoint to match Figma (#500) Updated package and lock files. Add parameter to @link-icon to adjust vertical position of icon and different options for $animate parameter (#504) Delete card-2019-08-28.twig (#506) Update _input.scss (#507) Scale down type-a and type-b font size at xs and sm breakpoints, repurpose splash font and minor typography tweaks (#498) Add 4 new @modular-spacing steps (#6 to #9) and modify 2 existing steps (#3 and #4) (#503) add su- prefix to Decanter core and component variables, deprecate old names (#452) Includevar (#496) Put appearance back for select dropdowns (#479) Split input.scss into several files and add new input-base placeholder (#476) ... # Conflicts (resolved manually) # core/dist/css/decanter.css # core/src/scss/components/card/_card.scss # core/src/scss/components/card/index.scss # core/src/scss/utilities/placeholders/components/_card.scss # core/src/templates/components/card/card.twig
…f card when button is last element in card body for visual balance
@sherakama , do you see any problems with me pulling in your FontAwesome changes from the v6 local footer into this branch? I could use that for the .su-card--icon-font variant. |
…su-card--link variant
…con-card * '435-icon-card' of github.com:SU-SWS/decanter: Update core/src/scss/components/card/_card--icon.scss
…for image links in mixin @card
… link/button in block_card_cta so the whole section can be replaced
…l and change max-width of icon; make placeholder %card-base call mixin @card
@sherakama , I think I made all the changes you requested. I created the mixin @card, and have the placeholder call the mixin with a deprecation notice. Please take another look when you have time. |
// | ||
// Markup: ../templates/components/card/card.twig | ||
// | ||
// Style guide: Components.Card | ||
// | ||
|
||
.su-card { | ||
@extend %card-base; | ||
@include card; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❤️
@@ -1,98 +1,14 @@ | |||
@charset "UTF-8"; | |||
|
|||
// | |||
// %card-base | |||
// <del>%card-base</del> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❤️
@include modular-spacing('padding', 0 null null); | ||
} | ||
|
||
&.su-card--minimal { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see you are allowing for multiple variants on this card. With our current variant approach, we do not allow for this as these combinators create exponential complexity. I have found myself wanting these combinators in work that I have done as well but it will be a change to our approach. As we are changing to the v6 branch soon and we have a palooza sprint we should take this chance to talk about the variant vs decorator models.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I did write the code to allow for combinations of e.g. su-card-minimal and su-card-horizontal with the media variants. I can see how this might make things harder to maintain. Let's talk more about this next week.
READY FOR REVIEW
Summary
@small-paragraph
for the card body content.Needed By (Date)
Urgency
Steps to Test
su-card--icon, su-card--video, su-card--embed
and see if they behave as intended.--minimal, --horizontal, --link
card variant classes.Note: The icon variants shows the icon at xs breakpoints since they have small file size.
Affected Projects or Products
Associated Issues and/or People