Skip to content

Conversation

@xs9627
Copy link
Contributor

@xs9627 xs9627 commented Jan 26, 2021

This PR migrates the MenuItem component to the new emotion format as a part of #24405.

@mui-pr-bot
Copy link

mui-pr-bot commented Jan 26, 2021

@material-ui/core: parsed: +0.16% , gzip: +0.12%

Details of bundle changes

Generated by 🚫 dangerJS against 354ff22

@oliviertassinari oliviertassinari added the scope: menu Changes related to the menu. label Jan 26, 2021
Copy link
Member

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

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

I believe we should implement this by adding components and componentsProps in the ListItem component, similar to how we done it in #24634 cc @oliviertassinari

@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 27, 2021

@mnajdova On a side note, we think that we should remove MenuList > ListItem in v5 because the abstraction makes too little sense.

@mnajdova
Copy link
Member

@mnajdova On a side note, we think that we should remove MenuList > ListItem in v5 because the abstraction makes too little sense.

In that case should we hold off with these changes until the component is refactored?

@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 27, 2021

In that case should we hold off with these changes until the component is refactored?

@mnajdova I have repurposed #21587 to keep track of this effort. I don't know when it will happen. I think that it would be more valuable to first remove JSS from the list of dependencies entirely. Having to implement the components/componentsProps makes us a step further in the unstyled direction we are pursuing

@mnajdova
Copy link
Member

@mnajdova I have repurposed #21587 to keep track of this effort. I don't know when it will happen. I think that it would be more valuable to first remove JSS from the list of dependencies entirely. Having to implement the components/componentsProps makes us a step further in the unstyled direction we are pursuing

Makes sense! @xs9627 would you like to proceed with this? #24634 should be a good template for introducing components and componentsProps and using them in a component.

@xs9627
Copy link
Contributor Author

xs9627 commented Jan 27, 2021

Not quite sure about the changes, just saw all tests passed for MenuItem and ListItem :)

@xs9627 xs9627 requested a review from mnajdova January 31, 2021 10:37
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Feb 11, 2021
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged. label Feb 18, 2021
@oliviertassinari oliviertassinari merged commit 22491f2 into mui:next Feb 18, 2021
@oliviertassinari
Copy link
Member

@xs9627 Thanks :)

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

Labels

scope: menu Changes related to the menu.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants