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

style(Checkbox|Flag|Image|Radio): propTypes cleanups and typings update #1155

Merged
merged 5 commits into from
Jan 17, 2017

Conversation

layershifter
Copy link
Member

@layershifter layershifter commented Jan 13, 2017

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

@layershifter layershifter force-pushed the style/proptypes-cleanups branch from 217aa84 to 5652ee5 Compare January 13, 2017 09:56
@@ -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.
**/
Copy link
Member Author

Choose a reason for hiding this comment

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

Added description 😄

Copy link
Member

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:

/**
 * ...
 */

Copy link
Member Author

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)
Copy link
Member Author

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. */
Copy link
Member Author

Choose a reason for hiding this comment

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

Dots 😄

Copy link
Member

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>;
Copy link
Member Author

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'}`)
})
})
Copy link
Member Author

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)
})
Copy link
Member Author

Choose a reason for hiding this comment

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

^

Copy link
Member

Choose a reason for hiding this comment

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

^

@codecov-io
Copy link

codecov-io commented Jan 13, 2017

Current coverage is 95.88% (diff: 100%)

No coverage report found for master at b7ffa6a.

Powered by Codecov. Last update b7ffa6a...358cadc

@layershifter layershifter changed the title style(Checkbox|Flag|Image|Radio): propTypes cleanups style(Checkbox|Flag|Image|Radio): propTypes cleanups and typings update Jan 14, 2017
}

Radio.propTypes = {
/** Format to emphasize the current selection state */
/** Format to emphasize the current selection state. */
Copy link
Member

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.
**/
Copy link
Member

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) => {
Copy link
Member

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)

Copy link
Member Author

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)
})
Copy link
Member

Choose a reason for hiding this comment

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

^

@layershifter
Copy link
Member Author

TODO: Check for #1169.

@layershifter
Copy link
Member Author

I've updated commonTests and all usages of propKeyOrValueAndKeyToClassName.

@levithomason levithomason merged commit ff68268 into master Jan 17, 2017
@levithomason levithomason deleted the style/proptypes-cleanups branch January 17, 2017 19:27
@levithomason
Copy link
Member

Released in semantic-ui-react@0.64.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants