Can we put ListItemText and table body cell text in a font size category of their own or use body2/body1?

[jedwards1211]

• This is a v1.x issue.
• I have searched the issues of this repository and believe that this is not a duplicate.

Shouldn't <Typography variant="subheading"> only be used for places in the app that are conceptually a subheading (i.e. smaller text under the title of a section)? I think ListItemText is abusing it. The primary text of a list item has nothing to do conceptually with a subheading in my mind and I want to be able to control the default list item text and subheading text size independently.

Not to mention that ListItemText uses <Typography variant="body1"> for its secondary text. (What is the concept behind body1 either?)

Personally I think list item primary text is in a conceptual category of its own, a category in which the default text size in table body cells should belong as well. If anything the size should probably just be the same as the body text (consider the fact that you guys are using body2/body1 in the list items of the sidebar of your own docs, or something that looks very similar to it? 😉)

I'm finding it pretty awkward to stick to a few conceptually clear categories of text size while satisfying the requirements from the UX team.

[oliviertassinari]

I'm finding it pretty awkward to stick to a few conceptually clear categories of text size while satisfying the requirements from the UX team.

@jedwards1211 It's a design decision. The point is to minimize entropy, the smaller set of variations we have, the easier people can quickly iterate. If your use case requires new typography variations, you can easily add them with the theme.overrides.MuiX feature.

[eps1lon]

You have to remember that these components are an implementation of the material design spec. We can make decisions about implementation details but not how the end product looks. While I not necessarily disagree with you I think this discussion should be targeted at the spec designers.

However the list spec is not very clear about the type styles since it only mentions sizes. It looks like either subtitle1 and body2 or body1 and body2. The material web components use subtitle1 and body2 if your compare their size and letter-spacing with the spec.

The Typography implementation currently lacks the letter-spacing and uses subheadingX instead of subtitleX.

[jedwards1211]

Well even if the list items have the same text size as subheadings by default, should their font sizes be linked? The spec doesn't say they should be linked does it?

[jedwards1211]

There's not a way to add a new variant like subtitle to Typojjgraphy using theme overrides is there? I thought I could only override existing JSS classes.

[eps1lon]

The spec doesn't say they should be linked does it?

No it doesn't but the example themes in the spec do it and material web components which is another implementation does it as well.

There's not a way to add a new variant like subtitle to Typojjgraphy using theme overrides is there?

I don't think so.

[jedwards1211]

@eps1lon so basically if a dev wants a different font size for subheadings and ListItemText right now, their only option is to never use <Typography variant="subheading"> in their own code and instead create their own component for subheadings.

[jedwards1211]

Also, this project doesn't have a subtitle1 variant like the spec does. Was the typography spec changed recently?

[mbrookes]

Yes, for Material 2.

[jedwards1211]

So here's another pain point that arises from this: MenuItem renders a ListItem internally. So even if I fork ListItem to tweak something like the font sizes (so that I don't have to go to the trouble to disableTyprography in a hundred places), those changes don't apply to MenuItem. Now I have to fork MenuItem as well.
And just imagine how many components I would have to fork if there were a problem I could only work around by forking ButtonBase!

I'm feeling more and more convinced that it would be extremely valuable for Material UI to provide a way to override the component implementation, not just the JSS, so that I can override ListItem, and the changes will also apply to MenuItem. One way it could work is for withStyles to look for an override component with the given name in the theme, and if found, use that implementation instead of the default.

Ability to override component implementations would come with additional benefits:

• New developers don't have to know whether they should import a given component straight from Material UI, or from a local fork intended to be used project-wide instead.
• Components could be forked more quickly, without needing to change imports anywhere they're used

I mean, I guess I could use custom Babel/Webpack resolvers as a sort of hacky workaround. But I think being able to override components in the theme would be much wiser in the long run.

[jedwards1211]

@oliviertassinari I agree with the goal of helping everyone iterate quickly, but this particular case - list item text size being linked to subheading size - cost me quite a bit of time. I had to dig through your source code to know which typography variants List item text even uses. And then I had to replace all subheadings in my project with a different component.

I have a dream...I may not believe I can fly, but I believe I could iterate more quickly than this with more API capabilities. So please don't stop believin' that improvements can be made. There is still plenty of room for fresh, original thinking, even though this is an established project.

Imagine how much faster people could iterate if they could override a component implementation with their customizations without having to change any code that imports it...

I have even done just this in past non-material UI projects using a little react-skin library I made years ago.

[eps1lon]

What do you think about using the MuiThemeProvider to set component wide primaryTypographyProps and then use a more fitting variant?

[eps1lon]

@jedwards1211 Could you take a look at https://codesandbox.io/s/18p091n8pj and tell me if this matches your vision? Important parth is in withRoot.js where the theme is created.

This applies body1 to the primary text of ListItemText and body2 to the secondary text of ListItemText.

You could also create another class for MuiTypography (lets say listItemTextPrimary) in the overrides property of createTheme and then pass the same name to the variant prop of primaryTypographyProps. Typescript might throw an error and I'm not sure if this intended or possibly considered hacky.

[jedwards1211]

@eps1lon oh, thanks for putting that example together, I didn't realize that I can pass props with the theme, passing props must be a new feature. That should do it for me.
I do still wish I could override entire components though...because I always want more ability to control the tools I use :)

[oliviertassinari]

For the sack of the posterity:

const theme = createMuiTheme({
  props: {
    MuiListItemText: {
      primaryTypographyProps: {
        variant: "body1"
      },
      secondaryTypographyProps: {
        variant: "body2"
      }
    }
  }
});

I do still wish I could override entire components though...because I always want more ability to control the tools I use :)

@jedwards1211 I would encourage people to wrap the Material-UI components. It's what we are doing at the office.

[eps1lon]

Many components have some prop like `someComponent` so this is still possible for (hopefully) most use cases.

…

On Sat, Sep 1, 2018, 18:24 Andy Edwards ***@***.***> wrote: @eps1lon <https://github.com/eps1lon> oh, thanks for putting that example together, I didn't realize that I can pass props with the theme. That should do it for me. I do still wish I could override entire components though...because I always want more ability to control the tools I use :) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#12726 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ALuPz1loSGM90W3Wp7N-q255oRXmE3cNks5uWrSfgaJpZM4WUP7M> .

[jedwards1211]

@oliviertassinari still, I'm making a PR right now to show how overriding the entire component via withStyles is quite possible.
As I mentioned, wrapping can become problematic because (for example) the default MenuItem is unable to use the wrapped ListItem.

[oliviertassinari]

Yes, it was designed to be possible.

[jedwards1211]

PR here: #12742

[oliviertassinari]

The discussion continues in #13900.