Skip to content
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

Card: add component #387

Merged
merged 15 commits into from
Aug 18, 2016
Merged

Card: add component #387

merged 15 commits into from
Aug 18, 2016

Conversation

athurman
Copy link
Contributor

@athurman athurman commented Aug 11, 2016

Adds Card component to v1 API

http://semantic-ui.com/views/card.html

Types

Fluid Card

<div class="ui fluid card">
  ...
</div>
<Card fluid />

Centered Card

<div class="ui centered card">
  ...
</div>
<Card centered />

Raised Card

<div class="ui raised card">
  ...
</div>
<Card raised />

Link Card

<a class="ui card" href="http://www.dog.com">
  ...
</a>
<Card href='http://www.dog.com' />

<Card onClick={(e) => (this.onClick(e))} />

Colored Card

<div class="ui red card">
  ...
</div>
<Card color='red' />

Cards

A group of cards

<div class="ui cards">
  <div class="card">
    ...
  </div>
  <div class="card">
    ...
  </div>
</div
<Card.Group cards={[{}]} />

<Card.Group>
  <Card color='red' />
  <Card color='orange' />
</Card.Group>

Column Count

defines how many cards should exist in a row

<div class="ui four cards">
  <div class="card">
    ...
  </div>
  <div class="card">
    ...
  </div>
</div
<Card.Group itemsPerRow={4} />

Stackable

<div class="ui three stackable cards">
  <div class="card">
    ...
  </div>
  <div class="card">
    ...
  </div>
</div
<Card.Group itemsPerRow={3} stackable />

Doubling

<div class="ui six doubling cards">
  <div class="card">
    ...
  </div>
  <div class="card">
    ...
  </div>
</div
<Card.Group itemsPerRow={6} doubling />

@athurman athurman mentioned this pull request Aug 11, 2016
function Card(props) {
const { children, className, ...rest } = props
const classes = cx(
className,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add ui class as the first class. Then, put the user's className as the last class.

@levithomason
Copy link
Member

Great, glad to have this started again. Let's get the subcomponents in and a doc example going. Reference the Rating docs for the most recent doc site pattern.

@levithomason
Copy link
Member

API looks like a good start. Linking to the docs is helpful but you don't have to go through that if you don't want!

Still thinking about the absence of the ui class on the card when in cards. Though, as we tested the markup is OK with it included.

We just need to add the subcomponents now:

Content

<div class="content"></div>
<Card.Content />

extra

<div class="extra content"></div>
<Card.Content extra />

Header

<div class="header"></div>
<Card.Header />

Description

<div class="description"></div>
<Card.Description />

Meta

<div class="meta"></div>
<Card.Meta />

@codecov-io
Copy link

codecov-io commented Aug 11, 2016

Current coverage is 95.10% (diff: 100%)

Merging #387 into master will increase coverage by 0.20%

@@             master       #387   diff @@
==========================================
  Files            85         91     +6   
  Lines          1119       1165    +46   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           1062       1108    +46   
  Misses           57         57          
  Partials          0          0          

Powered by Codecov. Last update 8ffd7e0...05718d6

color,
fluid,
raised,
...rest,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have another propUtil to get the rest:

import { getUnhandledProps } from '../../utils/propUtils'

const rest = getUnhandledProps(Card, props)

This is needed since we might not always destructure all the props when using ...rest. Especially when there are multiple render methods. The util creates an object of all the props that your component does not handle (i.e. the rest of the user's props).

@levithomason
Copy link
Member

I've updated this PR to use the newly refactored utils, see #388. I also updated the shorthand props to comply with our latest RFC on this, #391.

@levithomason levithomason changed the title Add Card Component to v1 API Add Card Component Aug 13, 2016
@athurman athurman force-pushed the feature/card-view branch 4 times, most recently from a099621 to b4945d2 Compare August 15, 2016 22:18
'in',
'upstate',
'New',
'York.'].join(' ')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This strikes me as strange, can we use a regular string here? If we're fighting max column issues, we can put more content on a single line:

const description = [
  'Amy is a violinist with 2 years experience in the wedding industry.',
  'She enjoys the outdoors and currently resides in upstate New York.',
].join(' ')

@levithomason
Copy link
Member

Before I also forget, here's a reminder to mark the "Card" off in the README.md support table.

@athurman athurman force-pushed the feature/card-view branch 3 times, most recently from b0ed621 to 951e6e1 Compare August 16, 2016 18:54
@levithomason
Copy link
Member

@athurman bump:

Before I also forget, here's a reminder to mark the "Card" off in the README.md support table.

header='Elliot Baker'
meta='Friend'
description='Elliot is a sound engineer living in Nashville who enjoys playing guitar and hanging with his cat.'
/>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For shorthand examples where there are no nodes between the Card and Content, let's just put the props on the Card:

<Card 
  href='#link'
  header='Elliot Baker'
  meta='Friend'
  description='Elliot is a sound engineer living in Nashville who enjoys playing guitar and hanging with his cat.'
/>

meta='Friend'
description='Elliot is a sound engineer living in Nashville who enjoys playing guitar and hanging with his cat.'
/>
</Card>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To assist with the docs, let's separate the shorthand props examples into separate files. This way there are two descriptions and two code blocks. It also removes the need for the extra div wrapper.

See this thread for more context and an example: https://github.com/TechnologyAdvice/stardust/pull/344#discussion_r74117525


const LinkCard = () => (
<Card
href='#link'
Copy link
Member

@levithomason levithomason Aug 17, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops, no # when defining an href.

Oh, I see, this is going to link to the example of the same name. In the future, prefer empty # to linking to actual anchors. No need to update. 👍

@levithomason levithomason changed the title Add Card Component Card: add component Aug 18, 2016
@levithomason levithomason merged commit e8f0147 into master Aug 18, 2016
@levithomason levithomason deleted the feature/card-view branch August 18, 2016 17:48
@levithomason
Copy link
Member

Released in v0.35.0

@layershifter layershifter mentioned this pull request Aug 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants