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

ListItemText with primary, secondary, secondaryTypographyProps does not display correctly #19943

Closed
2 tasks done
protzman opened this issue Mar 2, 2020 · 3 comments · Fixed by #20039
Closed
2 tasks done
Labels
bug 🐛 Something doesn't work component: list This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process.

Comments

@protzman
Copy link

protzman commented Mar 2, 2020

When using a ListItemText component with the fields primary, secondary, and specifically secondaryTypographyProps the text is not displayed properly inside of the ListItemText. The secondary prop once given secondaryTypographyProps is displayed at the end of the primary text instead of on its own row below.

  • The issue is present in the latest release.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

If a ListItemText component is given secondaryTypographyProps it will join the primary text on the first line.

Expected Behavior 🤔

It seems as you would want the secondary text to be allowed its secondaryTypographyProps and retain its position on the second row as it does without secondaryTypographyProps.

Steps to Reproduce 🕹

Here is a codesandbox of the recreation using the MUI template - but for clarity's sake, here is the snippet:

<List>
  <ListItem>
    <ListItemText
      primary="non working example"
      primaryTypographyProps={{ variant: "overline" }}
      secondary="improper secondary text"
      secondaryTypographyProps={{ variant: "overline" }}
    />
  </ListItem>
  <ListItem>
    <ListItemText
      primary="working example"
      primaryTypographyProps={{ variant: "overline" }}
      secondary="proper secondary text but cannot apply props"
    />
  </ListItem>
</List>

Here is a picture of the above code in action:
image

Steps:

  1. Add a ListItemText component to a List
  2. Add primary, secondary, and most importantly secondaryTypographyProps to the ListItemText

Not a step to reproduce, but could get the desired effect by applying
display: gird to .MuiListItemText-multiline. Not an expert though so not 100% sure that is the desired solution.

Context 🔦

I need to be able to style the secondary text in the ListItemText to achieve my desired ux.

Your Environment 🌎

Tech Version
Material-UI v4.8.0 (code sandbox using 4.9.5)
React ^16.12.0
Browser Chrome, (reproducible in Edge)
TypeScript no
@joshwooding joshwooding added the bug 🐛 Something doesn't work label Mar 4, 2020
@joshwooding
Copy link
Member

This is due to the display changes we made in v4. Seems like a simple fix.

  if (primary != null && primary.type !== Typography && !disableTypography) {
    primary = (
      <Typography
        variant={dense ? 'body2' : 'body1'}
        className={classes.primary}
        component="span"
+       display="block"
        {...primaryTypographyProps}
      >
        {primary}
      </Typography>
    );
  }
if (secondary != null && secondary.type !== Typography && !disableTypography) {
    secondary = (
      <Typography
        variant="body2"
        className={classes.secondary}
        color="textSecondary"
+       display="block"
        {...secondaryTypographyProps}
      >
        {secondary}
      </Typography>
    );
  }

https://github.com/mui-org/material-ui/blob/89c0ed4468b2a703728f2d6279c8252fc2b7ef1b/packages/material-ui/src/ListItemText/ListItemText.js

@protzman Do you want to work on a pull-request?

@oliviertassinari oliviertassinari added the good first issue Great for first contributions. Enable to learn the contribution process. label Mar 6, 2020
@psdr03
Copy link
Contributor

psdr03 commented Mar 7, 2020

Can I take this? Would be my first issue.

@joshwooding
Copy link
Member

@psdr03 Sure :)

@oliviertassinari oliviertassinari added component: list This is the name of the generic UI component, not the React module! and removed component: ListItem labels Mar 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: list This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants