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

fix(MenuItem): prop types#1197

Merged
kuzhelov merged 6 commits intomasterfrom
fix/menu-item-prop-types
Apr 10, 2019
Merged

fix(MenuItem): prop types#1197
kuzhelov merged 6 commits intomasterfrom
fix/menu-item-prop-types

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.


/** 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.

3 participants