-
Notifications
You must be signed in to change notification settings - Fork 81
Update cf-component-label to use Fela #103
Changes from all commits
0e69a20
8cea988
c9be72a
5d7780b
44ea947
99e1d66
cdb0464
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 |
---|---|---|
@@ -1,13 +1,35 @@ | ||
import React, { PropTypes } from 'react'; | ||
import { createComponent } from 'cf-style-container'; | ||
|
||
const styles = props => { | ||
const theme = props.theme; | ||
return { | ||
borderRadius: theme.borderRadius, | ||
color: theme.color, | ||
display: theme.display, | ||
fontSize: theme.fontSize, | ||
fontWeight: theme.fontWeight, | ||
lineHeight: theme.lineHeight, | ||
paddingTop: theme.paddingTop, | ||
paddingRight: theme.paddingRight, | ||
paddingBottom: theme.paddingBottom, | ||
paddingLeft: theme.paddingLeft, | ||
textTransform: theme.textTransform, | ||
userSelect: theme.userSelect, | ||
verticalAlign: theme.verticalAlign, | ||
'-webkit-text-stroke': theme.webkitTextStroke, | ||
whiteSpace: theme.whiteSpace, | ||
backgroundColor: theme[`${props.type}BackgroundColor`] | ||
}; | ||
}; | ||
|
||
class Label extends React.Component { | ||
render() { | ||
return React.createElement( | ||
this.props.tagName, | ||
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. Why are we changing the API? We use 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. The reason is because I checked our codebase and the only implementation is using span. This also breaks our pattern of having a standardized component, a user could technically render anything using the tagName. 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. in the same way 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. ok |
||
{ | ||
className: 'cf-label cf-label--' + this.props.type | ||
}, | ||
this.props.children | ||
const { children, className } = this.props; | ||
return ( | ||
<span className={className}> | ||
{children} | ||
</span> | ||
); | ||
} | ||
} | ||
|
@@ -20,12 +42,7 @@ Label.propTypes = { | |
'warning', | ||
'error' | ||
]).isRequired, | ||
tagName: PropTypes.string, | ||
children: PropTypes.node | ||
}; | ||
|
||
Label.defaultProps = { | ||
tagName: 'span' | ||
}; | ||
|
||
export default Label; | ||
export default createComponent(styles, Label); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
import { rem } from 'polished'; | ||
|
||
export default baseTheme => { | ||
return { | ||
borderRadius: baseTheme.borderRadius, | ||
color: baseTheme.colorWhite, | ||
display: 'inline-block', | ||
fontSize: `${(parseFloat(baseTheme.fontSize) - 2) / parseFloat(baseTheme.fontSize)}rem`, | ||
fontWeight: baseTheme.weightSemiBold, | ||
lineHeight: baseTheme.rem, | ||
paddingTop: '0.26667em', | ||
paddingRight: '0.4em', | ||
paddingBottom: '0.26667em', | ||
paddingLeft: '0.4em', | ||
textTransform: 'uppercase', | ||
userSelect: 'none', | ||
verticalAlign: 'baseline', | ||
webkitTextStroke: 0, | ||
whiteSpace: 'nowrap', | ||
|
||
defaultBackgroundColor: baseTheme.color.cement, | ||
infoBackgroundColor: baseTheme.color.marine, | ||
successBackgroundColor: baseTheme.color.grass, | ||
warningBackgroundColor: baseTheme.color.tangerine, | ||
errorBackgroundColor: baseTheme.color.apple | ||
}; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,8 @@ | ||
import Label from './Label'; | ||
import LabelUnstyled from './Label'; | ||
import LabelTheme from './LabelTheme'; | ||
|
||
export default Label; | ||
import { applyTheme } from 'cf-style-container'; | ||
|
||
const Label = applyTheme(LabelUnstyled, LabelTheme); | ||
|
||
export { Label, LabelTheme, LabelUnstyled }; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,15 +1,11 @@ | ||
import React from 'react'; | ||
import renderer from 'react-test-renderer'; | ||
import Label from '../../cf-component-label/src/index'; | ||
import felaTestContext from '../../../felaTestContext'; | ||
import { Label } from '../../cf-component-label/src/index'; | ||
|
||
test('should render', () => { | ||
const component = renderer.create(<Label type="default">A Label</Label>); | ||
expect(component.toJSON()).toMatchSnapshot(); | ||
}); | ||
|
||
test('should render with custom tagName', () => { | ||
const component = renderer.create( | ||
<Label type="default" tagName="small">A Label</Label> | ||
felaTestContext(<Label type="default">A Label</Label>) | ||
); | ||
expect(component.toJSON()).toMatchSnapshot(); | ||
}); |
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.
You don't need to change these manually. Lerna would handle this during the release.
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.
Should I remove these? Or let them be?
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.
You can leave it.