Skip to content

Add "all" custom prop type and fix issues#323

Merged
levithomason merged 6 commits intomasterfrom
feature/mutually-exclusive-type
Jul 5, 2016
Merged

Add "all" custom prop type and fix issues#323
levithomason merged 6 commits intomasterfrom
feature/mutually-exclusive-type

Conversation

@levithomason
Copy link
Member

@levithomason levithomason commented Jul 5, 2016

This PR adds the all() custom prop type. This allows passing multiple prop type checkers. It is needed in #321 and elsewhere to validate mutually exclusive props.

I also fixed a number of prop type issues and updated components using mutuallyExclusive.

@levithomason levithomason force-pushed the feature/mutually-exclusive-type branch from 032cb3c to 1bf6080 Compare July 5, 2016 17:48
<Header.H4 color='blue'>Blue</Header.H4>
<Header.H4 color='purple'>Purple</Header.H4>
<Header.H4 color='violent'>Violent</Header.H4>
<Header.H4 color='violet'>Violet</Header.H4>
Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed typo.

@levithomason levithomason force-pushed the feature/mutually-exclusive-type branch from 1bf6080 to 96bfb89 Compare July 5, 2016 17:52
'Invalid argument supplied to ofComponentTypes, expected an instance of array.'
` See ${componentName} prop \`${propName}\`.`,
].join(''))
}
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 validation to all custom prop types arguments. Some of these were being improperly used, so there was no validation happening at all.

@levithomason levithomason changed the title Extend custom prop types Add "all" custom prop type and fix issues Jul 5, 2016
@codecov-io
Copy link

codecov-io commented Jul 5, 2016

Current coverage is 88.71%

Merging #323 into master will decrease coverage by 0.23%

@@             master       #323   diff @@
==========================================
  Files            65         65          
  Lines           869        886    +17   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits            773        786    +13   
- Misses           96        100     +4   
  Partials          0          0          

Powered by Codecov. Last updated by da0017d...96bfb89

// mutually exclusive
const disallowed = _.transform(exclusives, (acc, exclusive) => {
if (_.has(props, propName) && _.has(props, exclusive)) {
return acc.push(exclusive)
Copy link
Member

Choose a reason for hiding this comment

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

.push returns what you pushed. Should it be return acc.concat(exclusive)?

Sidenote: feel like reduce or filter is more declarative, but is tangential to the above 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.

ack! thanks, had a spread operator here earlier.

@dvdzkwsk
Copy link
Member

dvdzkwsk commented Jul 5, 2016

👻

@kyleturco
Copy link
Contributor

🐧 looks good

@levithomason levithomason merged commit 9b91ff2 into master Jul 5, 2016
@levithomason levithomason deleted the feature/mutually-exclusive-type branch July 5, 2016 18:14
jhchill666 pushed a commit to jhchill666/stardust that referenced this pull request Jul 19, 2016
* fix(Header): fix color typos

* feat(customPropTypes): add "all", clean up, add validation

* refactor(Accordion): use "all" custom prop type

* refactor(Item): use "all" custom prop type

* fix(propUtils): mutuallyExclusive reduce

* fix(customPropTypes): grammar and array join
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants