-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
New Components - Cards (alternative version) #9215
Conversation
This file has all the base styles for cards.
Add the card.md file to the docs to explain and showcase the new component.
Add the card component to the main foundation.scss file for inclusion.
Make sure cards appear in the docs navigation
The most important feature why I originally created the feature component is missing. Imagine cards with the following content.
So now I want that all cards in one row have the header and the footer at the same height. So the content section to fill the space. With my solution cards can be seamlessly integrated into the existing grid adding the vertical layout that has to be added with javascript in a non flexbox environment. I have the impression, that both pull requests go into completely different directions. |
@DaSchTour I get what you are saying, and I'm happy to work together to get a solution that everyone is happy with and is useful for a wide range of people. What do you think the best approach would be to get this into 1 coherent PR that everyone's happy with? Also would solving #9208 go a long way to help with your vertical card requirement? |
Well maybe solving #9208 would create the foundation for vertical layouting in cards. Maybe only a few additions to the grid are needed to create vertical layouting. I'll have to figure this out in the next days. |
Hey guys @kball asked me to hop in and check this out. First off let me add that when we first started looking at Foundation 6 we wanted to strip components out that we found our users used rarely which led us to removing a few components that had been in Foundation for years. The beauty of that is it also made Foundation lighter by default which became a key feature of the new release and drove some awesome initiatives on dropping as much code as possible. We experimented with cards early on, but choose to remove them on this mantra, but after 8+ months of using the framework on tons of client projects we're totally finding the original need we had for them. They're a component I use on about 50% of projects which definitely meets the qualifications of the framework base. I'll take a look at this PR with the lens that a very base use of a card would be helpful and see how tiny we can keep it. @DaSchTour I love the idea of using Flexbox for this height reason, but wouldn't want it in the normal implementation of the component. We have however been working on "Flexbox Modes" of every Foundation component which gives you added features with the same mark-up. I'd love to see your implementation added in as a Flexbox mode similar to how we did the top-bar. I'll check this out today and give any feedback. Thanks, |
Checked it out, and it feels nice and simple and looks like you even had some flexbox mode in there which is nice. @kball I'm ok with this, but lets get some more eyes on it. Great work @brettsmason! |
border: $border; | ||
margin-bottom: $margin; | ||
background: $background; | ||
overflow: hidden; |
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.
What the purpose of this, i fear it will confuse people trying to position labels that overflow outside the bounds of the box?
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.
@zurbrandon so it was in the initial Zurb version of cards too. I think the reasoning is if you wanted to add a border radius to the container (which I guess is quite common) you dont have to apply radius to the top/bottom section and can just apply to the whole container.
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.
Ah yeah, ok. I may have been the one to do that even ha.
Sounds good to me :)
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.
@zurbrandon Cool thanks for the feedback. One more thing - what are your thoughts on border radius and box shadow as options? They were part of the initial Zurb version but I removed them to tidy it all up. Now I'm thinking they should be options (border radius defaults to global radius, box shadow off by default). That would give the mixin a bit more power.
Or... do you think thats over complicating things?
@DaSchTour Is there anything else that needs adding to this from your point of view for flexbox support? |
…adow. Added `flex-direction: column` to the container.
This is rad, @brettsmason. Are you sure that |
@andycochran So its based on the initial card version the Zurb guys put together - I've also noticed its the same as in Foundation for Apps: http://foundation.zurb.com/apps/docs/#!/card I guess it will mainly be used for a header/footer to a card. I'm not sure divider is the right name, but I'm not sure what else would fit - I dont feel like I was also thinking of changing the |
I like I agree, |
… Foundation for Apps version and is more semantic.
@andycochran @DaSchTour @brettsmason @ncoden What is your sense on the state of this PR? @DaSchTour I think we're going to move forward with this implementation, rather than #8722, but thank you for getting the ball rolling, and I do want to make sure it handles the flexbox cases you have in mind. Can you take a look and verify? Thanks! |
@@ -53,6 +53,7 @@ | |||
<li class="docs-nav-title">Containers</li> | |||
<li{{#ifpage 'accordion'}} class="current"{{/ifpage}}><a href="accordion.html">Accordion <span class="label">JS</span></a></li> | |||
<li{{#ifpage 'callout'}} class="current"{{/ifpage}}><a href="callout.html">Callout</a></li> | |||
<li{{#ifpage 'callout'}} class="current"{{/ifpage}}><a href="card.html">Card</a></li> |
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.
Looks like this was copied from the previous line. Shouldn't #ifpage 'callout'
be #ifpage '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.
@andycochran Thanks for pointing that out, had missed that! 😄
I've just corrected it.
@kball from my point of view it should be about ready, but others testing it too would be good. |
Looks god to me. @brettsmason is a champion! |
There are travis issues that are related to the version this was branched off of, so I checked it out to run with updated tests, and ran into this issue building:
|
@kball Sorry I missed this. I'll try and sort this tonight. I'll also run some tests just to make sure all is working properly before we merge. |
@kball no it doesn't handle the flexbox cases I had in mind. My implementation based on the idea that a card might have a header and footer see .card-header and .card-footer in my PR and a .card-block with flex-grow in the middle. flex-grow in this cases makes the block and only the block take all space that is available. I also made a slight different implementation here: https://github.com/DaSchTour/foundation-components/blob/master/scss/_cards.scss |
OK @DaSchTour I think I understand the case you're tackling enough to take a pass on it. @brettsmason so that I understand... what was the reason for making dividers have the same flex properties as sections? It seems like what might behave a bit better (and possibly address @DaSchTour 's concern) is to have something like
So that dividers shrink down to minimum necessary height by default and the sections take up the remainder |
Great work @brettsmason! |
:standing-ovation: |
Well, I checked 6.3.0-rc.1 to verify that what I created with my PR works and it doesn't. Well it does but not the way that it looks good and as it is documented. |
@DaSchTour I'm sorry you feel that way - the intention was never to disregard your PR or your suggestions, I just think a simpler card implementation was desired, and add more utility outside of cards that enabled what you wanted to achieve. @kball added a PR which contains a bunch of flexbox utility mixins/classes. The idea was to provide this sort of functionality to everything, rather than just cards. You can see the PR here: #9324 I had a look at your Codepen and modified it, take a look here: http://codepen.io/brettsmason/pen/GNyPOw Is that what you were trying to do? |
@brettsmason yes that's exactly what I achieved with my card implementation. I just don't really see the point why it now needs that many additional classes. I hoped there could be a more simple solution. |
@DaSchTour There's always mixins if you wanted to create something simpler to implement? Create your own class that has all relevant mixins assigned and you should be good to go. |
@brettsmason well sure. I'm already heavily make use of foundations flexbox mixins. Although naming get's really confusion if changing the direction. Right equals bottom and Left equals top and so on. If you work with flexbox a lot it's simple to layout what ever you want from scratch. I thought I could provide a simple solution for a common problem that can be used by everybody without diving deep into flexbox. |
@DaSchTour So looking at this some more, there's just a few pieces of the puzzle missing from what I understand about flexbox. With all the classes @kball added, we could use one that sets all the children as
Then all that needs adding to Alternatively you add this as a I'd like to get this usable for you so you are happy with it, so if you can work with me on a solution I'd appreciate it. Codepen: http://codepen.io/brettsmason/pen/GNyPOw |
@brettsmason Loving this new pattern! I started making a card library with them and so far so good! You are right that one way to handle this is with flex utility classes. Another way, what Daniel is suggesting is to use the flex grow property to automatically do this. With flexbox, the columns in a row are the same height, so getting the card divider to sit on the bottom totally work. @DaSchTour Love your idea! I think that it makes sense that a .card-divider at the bottom of the card should be aligned: bottom. The difference: vs There is probably another way to handle this where we set the @DaSchTour What do you think? Can we get a flex implementation that blends the your technique in with the current version where the card is inside the column rather than on it? |
Ok so played around with it more - I think we can achieve both easily. Columns are not display: flex by default (various reasons) but if we add a class to Foundation that makes them display flex, the card (or any content inside) will be full height. How about suggesting a |
As @ncoden might say… 😉 |
@andycochran yep that sounds good. What about a class that applies it to all children too? In this case you would apply it to the row. Or even an option with block grid? |
ooo interesting! @andycochran right on @brettsmason Hmm, yeah it would make sense, what to name it thought? |
@rafibomb not sure, would need to look through the new flex helper classes. I wonder if a lot of those new classes could also have a version that applies the effect to all children instead? |
@brettsmason This seems to be the same concept as in: #9098 (comment). You could use According to me, the card is a graphical component and the grid is a structural component. Columns that take the full height should be the grid job, so I don't like the flex properties applied to Maybe you could use row and column modifiers to apply flex properties inside a card ? So you can create non-card component with the "grow" behavior, or card components without it, simply by using or not @rafibomb @DaSchTour With that, you could can have cards with or without margins by applying using the card component inside columns ( What do you think ? |
I think this is a really important discussion to have and the answer, I believe, hinges on design goals with presently undecided levels of priority. These are decided:
These are not settled (at least not consistently):
Here's a relevant example: If I'm a noobie checking out Foundation for the first time, I might check out this card example and, perhaps, conclude that card heights will match no matter the content provided I use the row-grid structure. This, isn't the case. While chaining The linked approach makes it easier to maintain common heights; the second approach places content one level deeper to make it easier to control column widths. The correct choice, naturally, depends on the design philosophy of Zurb. Personally, I like the idea of keeping markup as simple as possible. @brettsmason made the markup for Off-Canvas so much simpler -- it's elegant. IMHO, pushing for greater markup elegance will make Foundation more accessible for users who rely on pre-built Foundation. Leveraging @zurbrandon 's work on frame-grid would help a lot. ¢¢ |
What if the "chained cards" approach had an optional |
@andycochran We have the grids to create complete and almost fully customizable rows and columns. Why re-create everything, the entirely structural logic, instead of using the grid's one ? |
Why do we have button group when we could just put expanded buttons in a collapsed grid? |
Good question. 😄 More seriously, because -for the specific case of button groups- :
I think this is the reasons why we do not use the collapsed grid for buttons. But maybe we should, I don't know the collapsed buttons behaviour to be sure about that. Is there others reasons ? However, I know the reasons I given does not apply to Cards. Cards are not simple components, we expect from them a complex and customizable behavior that is very close to the grid one, with a similar markup. |
This is rough, but totally works. $card-group-max: 6 !default;
.card-group {
@include clearfix;
}
@include -zf-each-breakpoint {
@for $i from 1 through $card-group-max {
.card-group-#{$-zf-size}-up-#{$i} {
@include grid-layout($i, '.card', 0);
margin-right: ($card-margin * ($i - 1));
.card:not(:nth-child(#{$i}n+#{$i})) {
margin-right: $card-margin;
}
.card:nth-child(#{$i}n+#{$i}) {
margin-right: -$card-margin * $i;
}
}
}
} <div class="card-group card-group-medium-up-4">
<div class="card">
<div class="card-section">
<p>A card in a grid.</p>
</div>
</div>
<div class="card">
<div class="card-section">
<p>A card in a grid.</p>
</div>
</div>
<div class="card">
<div class="card-section">
<p>A card in a grid.</p>
</div>
</div>
<div class="card">
<div class="card-section">
<p>A card in a grid.</p>
</div>
</div>
</div> |
You are right. How many CSS lines does it generate ? |
Could we not set a negative margin on either side of the container and then apply margin both sides to the card? Might be a bit simpler possibly? |
@andycochran Might I point out something? Semantics aside, cards can fit into current (flex) grid system in the following way: <div class="row">
<div class="card small-12 medium-4">
<div class="card-section">
<p>Card Uno</p>
</div>
</div>
<div class="card small-12 medium-4">
<div class="card-section">
<p>Card Dos</p>
</div>
</div>
<div class="card small-12 medium-4">
<div class="card-section">
<p>Card Tres</p>
</div>
</div> You just have to be comfortable with dropping ".column" from the markup. |
Then all we'd need is… .card-group {
@include grid-row;
} …which could add zero new lines of code. |
@andycochran Righ, so EDIT: EDIT 2: |
Can you talk about it in a new issue ? @andycochran Do you have others remarks about #9215 (comment) ? Thank you. 😓 |
Re: cards without spacing or margins - Re: matching height cards - @DaSchTour I opened a new issue where we can focus features of cards on. If I'm missing something there, please add a comment on issue #9533 |
This is my version of cards for Foundation following #8722 as I felt it needed to be less opinionated and more of a foundation.
Its based on the original version that was in the alpha version of Foundation 6 but some with changes make it cleaner and hopefully more flexible.
Would love some feedback on this!
@kball sorry I ran out of time to do this yesterday like I promised.