Skip to content
This repository has been archived by the owner on Mar 4, 2020. It is now read-only.

fix(MenuItem): prop types #1197

Merged
merged 6 commits into from
Apr 10, 2019
Merged

fix(MenuItem): prop types #1197

merged 6 commits into from
Apr 10, 2019

Conversation

kuzhelov
Copy link
Contributor

@kuzhelov kuzhelov commented Apr 9, 2019

Fixes #1112

@codecov
Copy link

codecov bot commented Apr 9, 2019

Codecov Report

Merging #1197 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1197   +/-   ##
=======================================
  Coverage   82.48%   82.48%           
=======================================
  Files         742      742           
  Lines        8758     8758           
  Branches     1236     1236           
=======================================
  Hits         7224     7224           
  Misses       1519     1519           
  Partials       15       15
Impacted Files Coverage Δ
packages/react/src/components/Menu/MenuItem.tsx 59.48% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1d8c578...1067c96. Read the comment docs.

@@ -105,7 +105,7 @@ export interface MenuItemProps
wrapper?: ShorthandValue

/** Shorthand for the submenu. */
menu?: ShorthandValue
menu?: ShorthandValue | ShorthandValue[]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
menu?: ShorthandValue | ShorthandValue[]
menu?: ShorthandValue | ShorthandCollection

menu: customPropTypes.itemShorthand,
menu: PropTypes.oneOfType([
customPropTypes.itemShorthand,
PropTypes.arrayOf(customPropTypes.itemShorthand),
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
PropTypes.arrayOf(customPropTypes.itemShorthand),
customPropTypes.collectionShorthand,

Copy link
Member

@layershifter layershifter left a comment

Choose a reason for hiding this comment

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

Please use existing types for collections 🐰

@layershifter layershifter added the needs author changes Author needs to implement changes before merge label Apr 10, 2019
@kuzhelov kuzhelov added 🚀 ready for review and removed needs author changes Author needs to implement changes before merge labels Apr 10, 2019
@kuzhelov kuzhelov merged commit 5bf8061 into master Apr 10, 2019
@delete-merged-branch delete-merged-branch bot deleted the fix/menu-item-prop-types branch April 10, 2019 12:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants