Modular Card component

[chrismcv]

I made a PR based on @tomesch 's very nice card component. #672

I've added basic docs and fixed a few lint issues.

I'm wondering about the avatar in the header, and whether this should be restructured to use the new avatar component?

[hai-cea]

@chrismcv Cool! :)

Yes, I think we should use the new Avatar component if possible. Thanks!

[mairh]

@hai-cea Using Avatar component in Card header will inject too much dependency on using Avatar component from material-ui. Shouldn't we keep it optional and let the users decide what they want to place in Card header component.

I have my own custom Avatar component in my app which I can easily inject if it is not dependent on Avatar component from material-ui

[hai-cea]

@mairh That's a very good point - the way that I solved this was to just pass the avatar in as an element prop. You can see this in action in the list component. In your case, you'd just pass in your own Avatar component as the prop.

[chrismcv]

How does this work for you both? Uses the build in avatar if providing a source, otherwise accepts whatever element is passed in. It will automatically add 16px margin-right to the avatar element whatever that is.

Commit unsquashed so you can see the changes clearly....

[mairh]

Looks good

[tomesch]

👍

[chrismcv]

@hai-cea i've made the requested changes to tidy up... line-height does need px wondering if that is a sensible default?

[hai-cea]

@chrismcv Yes, I've been using px for line-heights.

[chrismcv]

oops, i did but didn't do it.... 1 min

[chrismcv]

that's you now - sorry about that

[chrismcv]

(and your other changes are in now too)

[hai-cea]

@chrismcv I think it looks good. Is this ready to be merged? The only other thing we might want to consider is adding spread attributes to at least the root nodes of the new components and also merging styles as well. What do you think? This would allow users to pass down additional props to the component and custom styles.

[chrismcv]

@hai-cea I think it's pretty much merge ready....
The card component has spread attributes already. The other components all have div's as their root, so i'm not sure what spreading adds here? As for merging styles, I think this is a good idea - I'll start putting this together now.

The only other feature(s) that might be useful for the component are 1) expand and collapse, and 2) responsive expanding and collapsing. e.g. <CardActions xs={false} sm={true}>...</...>

https://github.com/wearefractal/react-responsive seems to work well for this, but it is another dependency.

[hai-cea]

@chrismcv Sounds good to me - I think we can hold off on expand / collapse for a future PR.

The spread attribute would allow developers to attach events to those elements if they needed - but not a big issue for me either way and can always be added later.

[chrismcv]

@hai-cea that's the style propable commit - have a skim and i'll squash when you're happy.

fair point about events - i've added spreads in.

[hai-cea]

@chrismcv Looks great. Thanks so much! 👍

[chrismcv]

my pleasure :)

[hai-cea]

Just merged - Thanks @chrismcv 👍