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

[MenuItem] Long secondary text covers next menu item #3531

Closed
ShadowMen opened this issue Feb 29, 2016 · 5 comments · Fixed by #3928
Closed

[MenuItem] Long secondary text covers next menu item #3531

ShadowMen opened this issue Feb 29, 2016 · 5 comments · Fixed by #3928
Labels
bug 🐛 Something doesn't work

Comments

@ShadowMen
Copy link
Contributor

Problem Description

When I create a menu item, within an icon menu, with a large text and a large secondary text, the secondary text will break into a new line, but still floats on the right side and cover the next menu item text.

Versions

  • Material-UI: 0.14.4 & 0.15.0-alpha.1
  • React: 0.14.7
  • Browser: Google Chrome 48
@mbrookes
Copy link
Member

@heetvachhani I think this is your speciality! 😆 Care to take a look?

@mbrookes mbrookes added the bug 🐛 Something doesn't work label Feb 29, 2016
@heetvachhani
Copy link
Contributor

Thanks @mbrookes! but it's @tintin1343 speciality. 😁
Sure we will take a look 👍

@tintin1343
Copy link
Contributor

@ShadowMen : Do you need something like this?

Check the secondary text on Superscript

screen shot 2016-02-29 at 12 29 11 pm

Also how long is the secondary text? As per Google's Material design spec, every menu item should account for one line only. So if you have a long secondary text which doesn't fit in one line means you might have to make an effort to rephrase it to keep it to one line and not let it overflow.

We can still make it better using ellipsis. But it won't be as informative as it should be and won't serve the purpose.

@mbrookes : What do you think? I don't think it is a bug.

tintin1343 added a commit to tintin1343/material-ui that referenced this issue Mar 4, 2016
Add flex property to menuItem

Resolves mui#3531
tintin1343 added a commit to tintin1343/material-ui that referenced this issue Mar 4, 2016
tintin1343 added a commit to tintin1343/material-ui that referenced this issue Mar 4, 2016
tintin1343 added a commit to tintin1343/material-ui that referenced this issue Mar 4, 2016
tintin1343 added a commit to tintin1343/material-ui that referenced this issue Mar 5, 2016
mbradleyis pushed a commit to staticinstance/material-ui that referenced this issue Mar 7, 2016
und3fined pushed a commit to und3fined/material-ui that referenced this issue Mar 22, 2016
nathanmarks added a commit to nathanmarks/material-ui that referenced this issue Apr 10, 2016
mbrookes added a commit that referenced this issue Apr 10, 2016
[MenuItem] Revert flex props from #3597, fixes #3845, reopens #3531
@nathanmarks nathanmarks reopened this Apr 10, 2016
@nathanmarks
Copy link
Member

@ShadowMen @tintin1343 This has been re-opened due to a regression the fix caused. The fix needs to be implemented in a different way (ie, not use flex while list item doesn't use flex) as to not cause regressions in the current setup.

und3fined pushed a commit to und3fined/material-ui that referenced this issue Apr 20, 2016
@oliviertassinari
Copy link
Member

We have been porting the component on the v1-beta branch. We reimplemented it from the ground-up. While we haven't tested it, I think that the issue is most likely fixed on that branch. Hence, I'm closing it.
Still, we will accept PR fixes until v1-beta takes over the master branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants