-
-
Notifications
You must be signed in to change notification settings - Fork 31.7k
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
Can we put ListItemText and table body cell text in a font size category of their own or use body2/body1? #12726
Comments
@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 |
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 The |
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? |
There's not a way to add a new variant like |
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.
I don't think so. |
@eps1lon so basically if a dev wants a different font size for subheadings and |
Also, this project doesn't have a |
Yes, for Material 2. |
So here's another pain point that arises from this: 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 Ability to override component implementations would come with additional benefits:
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. |
@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 |
What do you think about using the |
@jedwards1211 Could you take a look at https://codesandbox.io/s/18p091n8pj and tell me if this matches your vision? Important parth is in This applies You could also create another class for |
@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. |
For the sack of the posterity: const theme = createMuiTheme({
props: {
MuiListItemText: {
primaryTypographyProps: {
variant: "body1"
},
secondaryTypographyProps: {
variant: "body2"
}
}
}
});
@jedwards1211 I would encourage people to wrap the Material-UI components. It's what we are doing at the office. |
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>
.
|
@oliviertassinari still, I'm making a PR right now to show how overriding the entire component via |
Yes, it was designed to be possible. |
PR here: #12742 |
The discussion continues in #13900. |
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 thinkListItemText
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 behindbody1
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.
The text was updated successfully, but these errors were encountered: