-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
style(Checkbox|Flag|Image|Radio): propTypes cleanups and typings update #1155
Conversation
0d7d1cf
to
217aa84
Compare
217aa84
to
5652ee5
Compare
@@ -53,9 +53,12 @@ const names = [ | |||
'ye', 'yemen', 'yt', 'mayotte', 'za', 'south africa', 'zm', 'zambia', 'zw', 'zimbabwe', | |||
] | |||
|
|||
/** | |||
* A flag is is used to represent a political state. | |||
**/ |
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.
Added description 😄
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.
The closing tag for a doc block only needs one asterisk:
/**
* ...
*/
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.
Also updated other doc blocks.
function Flag(props) { | ||
const { className, name } = props | ||
const classes = cx(name, className, 'flag') | ||
const classes = cx(name, 'flag', className) |
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.
className
is last
} | ||
|
||
Radio.propTypes = { | ||
/** Format to emphasize the current selection state */ | ||
/** Format to emphasize the current selection state. */ |
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.
Dots 😄
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.
😄
type?: 'checkbox' | 'radio'; | ||
} | ||
|
||
export const Radio: React.StatelessComponent<RadioProps>; |
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.
New typings for Radio
.
shallow(<Component { ...requiredProps } verticalAlign={propVal} />) | ||
.should.have.className(`${propVal} ${'aligned'}`) | ||
}) | ||
}) |
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.
We don't have _meta.props
now.
|
||
_.each(_.get(Component, `_meta.props[${propKey}]`), propVal => { | ||
wrapper.should.not.have.className(propVal) | ||
}) |
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.
^
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.
^
Current coverage is 95.88% (diff: 100%)
|
} | ||
|
||
Radio.propTypes = { | ||
/** Format to emphasize the current selection state */ | ||
/** Format to emphasize the current selection state. */ |
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.
😄
@@ -53,9 +53,12 @@ const names = [ | |||
'ye', 'yemen', 'yt', 'mayotte', 'za', 'south africa', 'zm', 'zambia', 'zw', 'zimbabwe', | |||
] | |||
|
|||
/** | |||
* A flag is is used to represent a political state. | |||
**/ |
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.
The closing tag for a doc block only needs one asterisk:
/**
* ...
*/
const { assertRequired } = commonTestHelpers('implementsVerticalAlignProp', Component) | ||
|
||
describe('verticalAlign (common)', () => { | ||
assertRequired(Component, 'a `Component`') | ||
|
||
_noDefaultClassNameFromProp(Component, 'verticalAlign', options) | ||
_noClassNameFromBoolProps(Component, 'verticalAlign', options) | ||
|
||
_.each(Component._meta.props.verticalAlign, (propVal) => { |
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.
Let's implement a similar solution as we will do in #1165 (comment)
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.
Agree with this, will work on it.
|
||
_.each(_.get(Component, `_meta.props[${propKey}]`), propVal => { | ||
wrapper.should.not.have.className(propVal) | ||
}) |
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.
^
TODO: Check for #1169. |
I've updated |
Released in |
This PR is part of work for removing propTypes in production bundle (#524, #731).
Also, cleanups and updates typings for #1072.
Affects following components:
Checkbox
Flag
Image
Radio