-
Notifications
You must be signed in to change notification settings - Fork 4.1k
Container Component updated to v1 API #277
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
Changes from 11 commits
27dc01c
a5441e5
7631a91
d64f2b2
84f9f53
9e2d9b4
06fde8b
724759a
bd4b8de
241a022
f420da9
a083fd5
bd628e3
77b4bb5
0d50e12
edf673a
9bb4509
f31fe72
ad81023
6330ca0
f2781cd
2366b67
8863f6d
28f6d7f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
import React, { Component } from 'react' | ||
import ContainerTypesExamples from './Types/ContainerTypesExamples' | ||
import ContainerVariationsExamples from './Variations/ContainerVariationsExamples' | ||
|
||
export default class ContainerExamples extends Component { | ||
render() { | ||
return ( | ||
<div> | ||
<ContainerTypesExamples /> | ||
<ContainerVariationsExamples /> | ||
</div> | ||
) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
/* eslint-disable max-len */ | ||
|
||
import React, { Component } from 'react' | ||
import { Container } from 'stardust' | ||
|
||
export default class ContainerContainerExample extends Component { | ||
render() { | ||
return ( | ||
<Container> | ||
<p>Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Aenean commodo ligula eget dolor. Aenean massa strong. Cum sociis natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus. Donec quam felis, ultricies nec, pellentesque eu, pretium quis, sem. Nulla consequat massa quis enim. Donec pede justo, fringilla vel, aliquet nec, vulputate eget, arcu. In enim justo, rhoncus ut, imperdiet a, venenatis vitae, justo. Nullam dictum felis eu pede link mollis pretium. Integer tincidunt. Cras dapibus. Vivamus elementum semper nisi. Aenean vulputate eleifend tellus. Aenean leo ligula, porttitor eu, consequat vitae, eleifend ac, enim. Aliquam lorem ante, dapibus in, viverra quis, feugiat a, tellus. Phasellus viverra nulla ut metus varius laoreet. Quisque rutrum. Aenean imperdiet. Etiam ultricies nisi vel augue. Curabitur ullamcorper ultricies nisi.</p> | ||
</Container> | ||
) | ||
} | ||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
/* eslint-disable max-len */ | ||
|
||
import React, { Component } from 'react' | ||
import ComponentExample from 'docs/app/Components/ComponentDoc/ComponentExample' | ||
import ExampleSection from 'docs/app/Components/ComponentDoc/ExampleSection' | ||
import { Message } from 'stardust' | ||
|
||
export default class ContainerTypesExamples extends Component { | ||
render() { | ||
return ( | ||
<ExampleSection title='Types'> | ||
<ComponentExample | ||
title='Container' | ||
description='A standard container' | ||
examplePath='elements/Container/Types/ContainerContainerExample' | ||
/> | ||
<ComponentExample | ||
title='Text Container' | ||
description='A container can reduce its maximum width to more naturally accomodate a single column of text' | ||
examplePath='elements/Container/Types/ContainerTextExample' | ||
> | ||
<Message className='info'> | ||
<p>A text container is a simpler markup alternative to using a grid with a single column, and is designed to have a reasonable maximum width for displaying flowing text</p> | ||
</Message> | ||
</ComponentExample> | ||
</ExampleSection> | ||
) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
/* eslint-disable max-len */ | ||
|
||
import React, { Component } from 'react' | ||
import { Container, Divider } from 'stardust' | ||
|
||
export default class ContainerAlignmentExample extends Component { | ||
render() { | ||
return ( | ||
<div> | ||
<Container left> | ||
Left Aligned | ||
</Container> | ||
<Container center> | ||
Center Aligned | ||
</Container> | ||
<Container right> | ||
Right Aligned | ||
</Container> | ||
<Container justified> | ||
<b>Justified</b> | ||
<Divider /> | ||
<p>Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Aenean commodo ligula eget dolor. Aenean massa strong. Cum sociis natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus. Donec quam felis, ultricies nec, pellentesque eu, pretium quis, sem. Nulla consequat massa quis enim. Donec pede justo, fringilla vel, aliquet nec, vulputate eget, arcu. In enim justo, rhoncus ut, imperdiet a, venenatis vitae, justo. Nullam dictum felis eu pede link mollis pretium. Integer tincidunt. Cras dapibus. Vivamus elementum semper nisi. Aenean vulputate eleifend tellus. Aenean leo ligula, porttitor eu, consequat vitae, eleifend ac, enim. Aliquam lorem ante, dapibus in, viverra quis, feugiat a, tellus. Phasellus viverra nulla ut metus varius laoreet. Quisque rutrum. Aenean imperdiet. Etiam ultricies nisi vel augue. Curabitur ullamcorper ultricies nisi.</p> | ||
<p>Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Aenean commodo ligula eget dolor. Aenean massa strong. Cum sociis natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus. Donec quam felis, ultricies nec, pellentesque eu, pretium quis, sem. Nulla consequat massa quis enim. Donec pede justo, fringilla vel, aliquet nec, vulputate eget, arcu. In enim justo, rhoncus ut, imperdiet a, venenatis vitae, justo. Nullam dictum felis eu pede link mollis pretium. Integer tincidunt. Cras dapibus. Vivamus elementum semper nisi. Aenean vulputate eleifend tellus. Aenean leo ligula, porttitor eu, consequat vitae, eleifend ac, enim. Aliquam lorem ante, dapibus in, viverra quis, feugiat a, tellus. Phasellus viverra nulla ut metus varius laoreet. Quisque rutrum. Aenean imperdiet. Etiam ultricies nisi vel augue. Curabitur ullamcorper ultricies nisi.</p> | ||
</Container> | ||
</div> | ||
|
||
) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
/* eslint-disable max-len */ | ||
|
||
import React, { Component } from 'react' | ||
import { Container, Message } from 'stardust' | ||
|
||
export default class ContainerFluidExample extends Component { | ||
render() { | ||
return ( | ||
<div> | ||
<Message className='info'> | ||
Fluid containers are useful for setting text alignment, or other variations on unstyled content | ||
</Message> | ||
<Container fluid> | ||
<h2>Dogs Roles with Humans</h2> | ||
<p>Domestic dogs inherited complex behaviors, such as bite inhibition, from their wolf ancestors, which would have been pack hunters with complex body language. These sophisticated forms of social cognition and communication may account for their trainability, playfulness, and ability to fit into human households and social situations, and these attributes have given dogs a relationship with humans that has enabled them to become one of the most successful species on the planet today.</p> | ||
<p>The dogs' value to early human hunter-gatherers led to them quickly becoming ubiquitous across world cultures. Dogs perform many roles for people, such as hunting, herding, pulling loads, protection, assisting police and military, companionship, and, more recently, aiding handicapped individuals. This impact on human society has given them the nickname "man's best friend" in the Western world. In some cultures, however, dogs are also a source of meat.</p> | ||
</Container> | ||
</div> | ||
|
||
) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
import React, { Component } from 'react' | ||
import ComponentExample from 'docs/app/Components/ComponentDoc/ComponentExample' | ||
import ExampleSection from 'docs/app/Components/ComponentDoc/ExampleSection' | ||
|
||
export default class ContainerVariationsExamples extends Component { | ||
render() { | ||
return ( | ||
<ExampleSection title='Variations'> | ||
<ComponentExample | ||
title='Text Alignment' | ||
description='A container can specify its text alignment' | ||
examplePath='elements/Container/Variations/ContainerAlignmentExample' | ||
/> | ||
<ComponentExample | ||
title='Fluid' | ||
description='A fluid container has no maximum width' | ||
examplePath='elements/Container/Variations/ContainerFluidExample' | ||
/> | ||
</ExampleSection> | ||
) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,33 +1,68 @@ | ||
import React, { Component, PropTypes } from 'react' | ||
import classNames from 'classnames' | ||
import META from '../../utils/Meta' | ||
import cx from 'classnames' | ||
import React, { PropTypes } from 'react' | ||
import META from '../../utils/Meta'; | ||
import { getUnhandledProps, useKeyOnly } from '../../utils/propUtils' | ||
|
||
/** | ||
* A container that gives your content some side padding. | ||
* A container limits content to a maximum width | ||
*/ | ||
export default class Container extends Component { | ||
static propTypes = { | ||
children: PropTypes.node, | ||
className: PropTypes.string, | ||
} | ||
|
||
static _meta = { | ||
library: META.library.semanticUI, | ||
name: 'Container', | ||
type: META.type.element, | ||
} | ||
|
||
render() { | ||
const classes = classNames( | ||
'sd-container', | ||
'ui', | ||
this.props.className, | ||
'container', | ||
) | ||
return ( | ||
<div {...this.props} className={classes}> | ||
{this.props.children} | ||
</div> | ||
) | ||
} | ||
function Container(props) { | ||
const { | ||
text, left, center, right, justified, fluid, | ||
children, className, | ||
} = props | ||
|
||
const classes = cx('sd-container ui', | ||
useKeyOnly(text, 'text'), | ||
useKeyOnly(left, 'left aligned'), | ||
useKeyOnly(center, 'center aligned'), | ||
useKeyOnly(right, 'right aligned'), | ||
useKeyOnly(justified, 'justified'), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are 4 types of possible class to props mappings for SUI classes. The alignment class here follows this API pattern: aligned="left" // useValueAndKey(aligned, 'aligned')
aligned="right" // useValueAndKey(aligned, 'aligned')
aligned="center" // useValueAndKey(aligned, 'aligned') One reason we use this key/value relationship is because two simultaneous alignment props are not valid: <Container right left /> Where as making those values of the <Container aligned="left" /> However, the cx(
aligned === 'justified' ? 'justified' : useValueAndKey(aligned, 'aligned'),
) Thoughts? EDIT Fixed incorrect className suggestion. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think your suggestion of just using a ternary operator is probably the best option. You could go down the route of adding unnecessary additions to propUtils but seems it's succinct enough to be used in this one isolated case. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
You are correct, I edited my typo shortly after submitting. See above.
For a prop that may be boolean only or enum, there is the <Input action /> // <input class="action" />
<Input action='left' /> // <input class="left action" />
<Input action='right' /> // <input class="right action" />
I think this is the way to go. You can see this in use in the Label for the pointing prop. A Label can be pointing alone or pointing with a direction, but never more than one. I think with this one special handling for the justified case we have a pretty good solution: cx(
aligned === 'justified' ? 'justified' : useValueAndKey(aligned, 'aligned'),
) If the <Container aligned='justified' /> // <div class="ui container justified"></div>
<Container aligned='left' /> // <div class="ui container left aligned"></div>
<Container aligned='right' /> // <div class="ui container right aligned"></div> There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note, we won't be able to use the common test for Since the alignment prop is pretty common, it may warrant it's own There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I'd digested more of your code after making that statement so Have amended the Container alignment as per your suggestion and updated test Levi Thomason wrote:
|
||
useKeyOnly(fluid, 'fluid'), | ||
'container', | ||
className | ||
) | ||
|
||
const ContainerComponent = 'div' | ||
const rest = getUnhandledProps(Container, props) | ||
|
||
return ( | ||
<ContainerComponent className={classes} {...rest}> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Absolutely. Wasn't sure how stringently to follow your initial efforts
Sent from Postbox |
||
{children} | ||
</ContainerComponent> | ||
) | ||
} | ||
|
||
Container._meta = { | ||
library: META.library.semanticUI, | ||
name: 'Container', | ||
type: META.type.element, | ||
} | ||
|
||
Container.propTypes = { | ||
/** Primary content of the Container */ | ||
children: PropTypes.node, | ||
|
||
/** Classes to add to the container className. */ | ||
className: PropTypes.string, | ||
|
||
/** Reduce maximum width to more naturally accomodate text */ | ||
text: PropTypes.bool, | ||
|
||
/** Align container content to left */ | ||
left: PropTypes.bool, | ||
|
||
/** Align container content to center */ | ||
center: PropTypes.bool, | ||
|
||
/** Align container content to right */ | ||
right: PropTypes.bool, | ||
|
||
/** Justify content to available space */ | ||
justified: PropTypes.bool, | ||
|
||
/** Container has no maximum with */ | ||
fluid: PropTypes.bool, | ||
} | ||
|
||
export default Container |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
import React from 'react' | ||
|
||
import Container from 'src/elements/Container/Container' | ||
import * as common from 'test/specs/commonTests' | ||
|
||
describe('Container', () => { | ||
common.isConformant(Container) | ||
common.rendersChildren(Container) | ||
common.hasUIClassName(Container) | ||
|
||
common.propKeyOnlyToClassName(Container, 'text') | ||
common.propKeyOnlyToClassName(Container, 'left', { className: 'aligned' }) | ||
common.propKeyOnlyToClassName(Container, 'center', { className: 'aligned' }) | ||
common.propKeyOnlyToClassName(Container, 'right', { className: 'aligned' }) | ||
common.propKeyOnlyToClassName(Container, 'justified') | ||
common.propKeyOnlyToClassName(Container, 'fluid') | ||
|
||
it('renders a <div /> element', () => { | ||
shallow(<Container />) | ||
.should.have.tagName('div') | ||
}) | ||
|
||
it('adds the Container class', () => { | ||
shallow(<Container />) | ||
.should.have.className('container') | ||
}) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test is automated as part of the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So the common.propKeyOnlyToClassName are still required, but not the 2
Sent from Postbox There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, should've been more explicit. You can keep all tests in this file, just remove this one: it('adds the Container class', ... All components must include a class that matches the component name. A The conformance tests use the component's There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. After updating this PR and getting the new tools merged in #285, remove this test: - it('does not have aligned class when justified', () => {
- shallow(<Container aligned='justified' />)
- .should.not.have.className('aligned')
- }) Then, replace line 12 above: -common.propKeyAndValueToClassName(Container, 'aligned')
+common.implementsAlignedProp(Container) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
}) |
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.
Same comment with the
<Message />
here as above. Let's hoist it up out of the example code.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.
Ok, have actioned all of those. Sorry still getting your code under my
belt!
Sent from Postbox
https://www.postbox-inc.com/?utm_source=email&utm_medium=siglink&utm_campaign=reach