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

Update MenuItem label propType to node #373

Merged
merged 2 commits into from
Aug 5, 2016

Conversation

Johnsel
Copy link
Contributor

@Johnsel Johnsel commented Aug 5, 2016

Hi everyone,

I have made this change to allow the label to be filled with some React component as well, in my case I needed to put in a "FormattedMessage" from the react-intl library to show a potentially translated label.

@codecov-io
Copy link

codecov-io commented Aug 5, 2016

Current coverage is 92.87% (diff: 100%)

Merging #373 into master will not change coverage

@@             master       #373   diff @@
==========================================
  Files            63         63          
  Lines           828        828          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits            769        769          
  Misses           59         59          
  Partials          0          0          

Powered by Codecov. Last update 6306be2...84bda0f

@levithomason
Copy link
Member

Heads up, @awei01 is working on revamping the Menu API entirely (#319) to meet our v1 component API pattern (#269).

We can merge this for now though, but we should probably go ahead and use:

label: PropTypes.node,

Which is valid for string, number, React component, or regular DOM node.

@Johnsel
Copy link
Contributor Author

Johnsel commented Aug 5, 2016

Cool, I did not know that. Will you make the change?

@levithomason
Copy link
Member

I'll merge it, but I don't have rights to your fork. Easy win contribution if you like to push the update :)

@Johnsel
Copy link
Contributor Author

Johnsel commented Aug 5, 2016

Sure, thanks. One minute please...

@levithomason levithomason changed the title MenuItem should accept an object for it's 'label' prop Update MenuItem label propType to node Aug 5, 2016
@levithomason levithomason merged commit 8b440f1 into Semantic-Org:master Aug 5, 2016
@levithomason
Copy link
Member

Thanks much!

@awei01
Copy link

awei01 commented Aug 5, 2016

@levithomason Hi. Sorry, but I won't be able to take on Menu. I've been sidetracked onto other projects.

@levithomason
Copy link
Member

It happens, thanks for the update @awei01.

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.

4 participants