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(Header): Add subheader prop #476

Merged
merged 3 commits into from
Sep 8, 2016
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
@@ -0,0 +1,8 @@
import React from 'react'
import { Header } from 'stardust'

const HeaderSubheaderPropExample = () => (
<Header as='h2' content='Account Settings' subheader='Manage your account settings and set email preferences' />
)

export default HeaderSubheaderPropExample
4 changes: 4 additions & 0 deletions docs/app/Examples/elements/Header/Content/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ const HeaderContentExamples = () => (
description='Headers may contain subheaders'
examplePath='elements/Header/Content/HeaderSubheaderExample'
/>
<ComponentExample
description='You can pass an Subheader content to the Header subheader prop'
examplePath='elements/Header/Content/HeaderSubheaderPropExample'
/>
</ExampleSection>
)

Expand Down
21 changes: 17 additions & 4 deletions src/elements/Header/Header.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import HeaderContent from './HeaderContent'
function Header(props) {
const {
color, content, dividing, block, attached, floated, inverted, disabled, sub, size, textAlign,
icon, image, children, className,
icon, image, children, className, subheader,
} = props

const classes = cx(
Expand All @@ -47,22 +47,29 @@ function Header(props) {

if (icon && typeof icon !== 'boolean') {
return (
<ElementType className={classes} {...rest}>
<ElementType {...rest} className={classes}>
{createIcon(icon)}
{content && <HeaderContent>{content}</HeaderContent>}
{subheader && <HeaderSubheader content={subheader} />}
Copy link
Member Author

Choose a reason for hiding this comment

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

Seems, I missed test-case there.

</ElementType>
)
}

if (image) {
return (
<ElementType className={classes} {...rest}>
<ElementType {...rest} className={classes}>
{createImage(image)} {content}
{subheader && <HeaderSubheader content={subheader} />}
</ElementType>
)
}

return <ElementType {...rest} className={classes}>{children || content}</ElementType>
return (
<ElementType {...rest} className={classes}>
{children || content}
{subheader && <HeaderSubheader content={subheader} />}
</ElementType>
)
Copy link
Member

Choose a reason for hiding this comment

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

Currently, the Header will render children with shorthand even though there are prop type warnings. While we're here, let's split this return with a conditional return to only render children if provided. The goal in doing this in all components is to enforce proper usage (not rendering shorthand when children are present):

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

  return (
    <ElementType {...rest} className={classes}>
      {content}
      {subheader && <HeaderSubheader content={subheader} />}
    </ElementType>
  )

}

Header._meta = {
Expand Down Expand Up @@ -156,6 +163,12 @@ Header.propTypes = {
/** Content headings are sized with em and are based on the font-size of their container. */
size: PropTypes.oneOf(Header._meta.props.size),

/** Shorthand for the Header.Subheader component. Mutually exclusive with children */
subheader: customPropTypes.every([
customPropTypes.disallow(['children']),
PropTypes.string,
]),

/** Align header content */
textAlign: PropTypes.oneOf(Header._meta.props.textAlign),
}
Expand Down
27 changes: 21 additions & 6 deletions src/elements/Header/HeaderSubheader.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,22 @@
import React, { PropTypes } from 'react'
import cx from 'classnames'
import { getElementType, getUnhandledProps, META } from '../../lib'
import React, { PropTypes } from 'react'

import {
customPropTypes,
getElementType,
getUnhandledProps,
META,
} from '../../lib'

function HeaderSubheader(props) {
const { children, className } = props
const { children, className, content } = props
const classes = cx('sub header', className)
const rest = getUnhandledProps(HeaderSubheader, props)
const ElementType = getElementType(HeaderSubheader, props)

return (
<ElementType className={classes} {...rest}>
{children}
{children || content}
Copy link
Member

Choose a reason for hiding this comment

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

In this case, having a single return is OK because either children or content will be rendered. But, never will children and shorthand be rendered. 👍

</ElementType>
)
}
Expand All @@ -28,11 +34,20 @@ HeaderSubheader.propTypes = {
PropTypes.func,
]),

/** Primary content of the HeaderSubheader */
children: PropTypes.node,
/** Primary content of the HeaderSubheader. Mutually exclusive with content */
children: customPropTypes.every([
customPropTypes.disallow(['content']),
PropTypes.node,
]),

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

/** Shorthand for primary content. Mutually exclusive with children */
content: customPropTypes.every([
customPropTypes.disallow(['children']),
PropTypes.string,
]),
}

export default HeaderSubheader
14 changes: 13 additions & 1 deletion test/specs/elements/Header/Header-test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import faker from 'faker'
import React from 'react'

import Header from 'src/elements/Header/Header'
import HeaderContent from 'src/elements/Header/HeaderContent'
import HeaderSubheader from 'src/elements/Header/HeaderSubheader'
Expand Down Expand Up @@ -47,7 +49,7 @@ describe('Header', () => {
describe('content', () => {
it('adds child text', () => {
shallow(<Header content='foo' />)
.should.have.prop('children', 'foo')
.should.contain.text('foo')
})
it('adds child text when there is an image', () => {
shallow(<Header content='foo' image='foo.png' />)
Expand All @@ -65,4 +67,14 @@ describe('Header', () => {
wrapper.should.not.have.descendants('HeaderContent')
})
})

describe('subheader', () => {
it('adds HeaderSubheader as child', () => {
const text = faker.hacker.phrase()

shallow(<Header subheader={text} />)
.find('HeaderSubheader')
.should.have.prop('content', text)
})
})
})
12 changes: 12 additions & 0 deletions test/specs/elements/Header/HeaderSubheader-test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,19 @@
import faker from 'faker'
import React from 'react'

import HeaderSubheader from 'src/elements/Header/HeaderSubheader'
import * as common from 'test/specs/commonTests'

describe('HeaderSubheader', () => {
common.isConformant(HeaderSubheader)
common.rendersChildren(HeaderSubheader)

describe('content', () => {
it('renders text', () => {
const text = faker.hacker.phrase()

shallow(<HeaderSubheader content={text} />)
.should.contain.text(text)
})
})
})