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

feat(List): update to v1 API #311

Closed
wants to merge 21 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
6197943
Convert List to v1 API
joshuazmiller Jun 28, 2016
6c7a3d6
Merge branch 'master' of https://github.com/TechnologyAdvice/stardust
joshuazmiller Jun 28, 2016
959aff4
Merge branch 'master' into master
joshuazmiller Jun 28, 2016
e1f7042
Merge branch 'master' of https://github.com/TechnologyAdvice/stardust
joshuazmiller Jun 28, 2016
0906e78
Finished refactoring List.js for v1. It works without errors on my ma…
joshuazmiller Jun 28, 2016
5d071df
Merge branch 'master' of https://github.com/joshuazmiller/stardust
joshuazmiller Jun 28, 2016
fdd3113
Renamed _List to List
joshuazmiller Jun 28, 2016
268c3c8
Got rid of ListComponent var and inlined 'div'
joshuazmiller Jun 28, 2016
1e86005
Updated List.js to get rid of 3 Linting errors.
joshuazmiller Jun 28, 2016
7e24d28
List: replaced classNames with declarative props
joshuazmiller Jun 29, 2016
1bd5965
Merge branch 'master' into master
joshuazmiller Jun 29, 2016
681fa30
List: added tests
joshuazmiller Jun 29, 2016
057be3a
Merge remote-tracking branch 'origin/master'
joshuazmiller Jun 29, 2016
d83a3c0
List: refined tests
joshuazmiller Jun 29, 2016
202a8b9
List: corrected lint
joshuazmiller Jun 29, 2016
cd7171d
Supporting env var assignments on Windows
joshuazmiller Jun 29, 2016
bf48ba3
Revert "Supporting env var assignments on Windows"
joshuazmiller Jun 29, 2016
f9ddcd0
Merge branch 'master' of https://github.com/TechnologyAdvice/stardust
joshuazmiller Jun 30, 2016
8245887
Fixing small things based on pull-request #311
joshuazmiller Jun 30, 2016
5b6519f
Merge branch 'master' of https://github.com/TechnologyAdvice/stardust
joshuazmiller Jul 4, 2016
4e7f6c9
feat: add ability to create list items via subcomponents
joshuazmiller Jul 4, 2016
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,15 @@ export default class ListDescriptionExample extends Component {
header='Nashville'
description='This city is located in the state of Tennessee'
/>
<List.Item>
<List.ItemIcon className='user' />
<List.ItemContent>
<List.ItemHeader>Daniel Louise</List.ItemHeader>
<List.ItemDescription>
This is an example of using sub-components.
</List.ItemDescription>
</List.ItemContent>
</List.Item>
</List>
)
}
Expand Down
10 changes: 10 additions & 0 deletions docs/app/Examples/elements/List/Content/ListHeaderExample.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,16 @@ export default class ListHeaderExample extends Component {
<List.Item header='Chapter 1' description='The chapter in which we meet the characters' />
<List.Item header='Chapter 2' description='The chapter in which the bad guy is introduced' />
<List.Item header='Chapter 3' description='Spoiler alert: The chapter in which the good guy wins!' />
<List.Item>
<List.ItemContent>
Copy link
Contributor Author

@joshuazmiller joshuazmiller Jul 4, 2016

Choose a reason for hiding this comment

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

When there is no image/icon do we want to require ItemContent? The current ListItem file suggests no.
ListItem.js: if (media) content = <div className='content'>{content}</div>

Copy link
Member

Choose a reason for hiding this comment

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

Correct, no content when there is no icon/image. Reference the SUI docs for the exact markup.

http://semantic-ui.com/elements/list

In the case of the Item view, adding content with no icon/image will actually break the vertically aligned variations.

<List.ItemHeader>
Chapter 4
</List.ItemHeader>
<List.ItemDescription>
This is an example of a sub-component.
</List.ItemDescription>
</List.ItemContent>
</List.Item>
</List>
)
}
Expand Down
6 changes: 6 additions & 0 deletions docs/app/Examples/elements/List/Content/ListIconExample.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@ export default class ListIconExample extends Component {
<List.Item icon={helpIcon}>
This item uses <code>child</code> text, check the code.
</List.Item>
<List.Item>
<List.ItemIcon className='right triangle' />
<List.ItemContent>
This is an example of using sub-components
</List.ItemContent>
</List.Item>
</List>
)
}
Expand Down
33 changes: 33 additions & 0 deletions src/elements/Content/Content.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import React, { PropTypes } from 'react'
import cx from 'classnames'
import META from '../../utils/Meta'

function Content(props) {
const {
children, className, ...rest,
} = props

const classes = cx(
'content',
className
)

return (
<div className={classes} {...rest}>{children}</div>
)
}

Content._meta = {
name: 'Content',
type: META.type.element,
}

Content.propTypes = {
/** Primary content of the List */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I realize my comment says List while it should say Content. I've noticed the typos in the Content and Description files and will fix.

children: PropTypes.node,

/** Classes to add to the list className. */
className: PropTypes.string,
}

export default Content
34 changes: 34 additions & 0 deletions src/elements/Description/Description.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import React, { PropTypes } from 'react'
import cx from 'classnames'
import META from '../../utils/Meta'

function Description(props) {
const {
children, className, ...rest,
} = props

const classes = cx(
'description',
className
)

return (
<div className={classes} {...rest}>{children}</div>
)
}

Description._meta = {
library: META.library.semanticUI,
name: 'Description',
type: META.type.element,
}

Description.propTypes = {
/** Primary content of the List */
children: PropTypes.node,

/** Classes to add to the list className. */
className: PropTypes.string,
}

export default Description
2 changes: 1 addition & 1 deletion src/elements/Icon/Icon.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ export default class Icon extends Component {
render() {
const { className } = this.props
const classes = cx(
className,
'icon',
className
)
return (
<i {...this.props} className={classes} />
Expand Down
38 changes: 38 additions & 0 deletions src/elements/List/ItemHeader.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import React, { PropTypes } from 'react'
import cx from 'classnames'
import META from '../../utils/Meta'

/**
* I'm not sure a seperate file is necessary or if there's a better way.
* Also not sure this is where this file would go.
*/

function ItemHeader(props) {
const {
children, className, ...rest,
} = props

const classes = cx(
'header',
className
)

return (
<div className={classes} {...rest}>{children}</div>
)
}

ItemHeader._meta = {
name: 'ItemHeader',
type: META.type.element,
}

ItemHeader.propTypes = {
/** Primary content of the List */
children: PropTypes.node,

/** Classes to add to the list className. */
className: PropTypes.string,
}

export default ItemHeader
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we want this file? Is this where it should be placed or perhaps under views/Item?

Copy link
Member

@levithomason levithomason Jul 7, 2016

Choose a reason for hiding this comment

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

I've left an analysis and proposal for "component parts" on this PR itself: https://github.com/TechnologyAdvice/stardust/pull/311#issuecomment-231006785

16 changes: 15 additions & 1 deletion src/elements/List/List.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ import {
useKeyOnly,
} from '../../utils/propUtils'
import ListItem from './ListItem'
import Icon from '../Icon/Icon'
import Image from '../Image/Image'
import ItemHeader from './ItemHeader'
import Content from '../Content/Content'
import Description from '../Description/Description'

function List(props) {
const {
Expand Down Expand Up @@ -44,6 +49,11 @@ function List(props) {
}

List.Item = ListItem
List.ItemIcon = Icon
List.ItemImage = Image
List.ItemHeader = ItemHeader
List.ItemContent = Content
List.ItemDescription = Description

List._meta = {
library: META.library.semanticUI,
Expand All @@ -52,6 +62,7 @@ List._meta = {
props: {
size: sui.sizes,
aligned: sui.verticalAlignments,
relaxed: 'very',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just a fix from an unrelated previous omission. It can either be a string (i.e. very) or a plain Boolean.

Copy link
Member

Choose a reason for hiding this comment

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

Correct, this should be an array though.

relaxed: ['very']

},
}

Expand Down Expand Up @@ -93,7 +104,10 @@ List.propTypes = {
inverted: PropTypes.bool,

/** List can relax its padding to provide more negative space */
relaxed: PropTypes.bool,
relaxed: PropTypes.oneOf(
List._meta.props.relaxed,
PropTypes.bool
),

/** A selection list formats list items as possible choices */
selection: PropTypes.bool,
Expand Down