-
Notifications
You must be signed in to change notification settings - Fork 16
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
New typography toolkit for MUI carto-theme #498
Conversation
lineHeight: 1.428, | ||
letterSpacing: 0 | ||
}, | ||
codeMediumStrong: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're gonna go after several 'nested' concepts here, should we create a proper object (to use keyed names)?
I'm thinking about something similar to (pseudo-code):
code : {
default: { /* style definition */ }
large: {
default: { /*whatever*/}
strong: { ... }
}
}
So we would consume externally something like: code.large.strong
Also I'm feeling that this set of custom elements could be maybe separated from official typography in the tree, having like a first level with custom : { /* previous keys here */ }
Would it be too verbose? or complex? What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. I like what you're proposing about nesting, I'll give it a try.
...baseTypography, | ||
...customTypography |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about including both blocks at the same level... but probably we can move on and have a longer discussion
@VictorVelarde I added some comments and cleaned up the number of variants a bit, based on our discussion and the meeting I had with the design team earlier. We have an open issue in shortcut to improve the props/variants management, that we can address after we do the MUI update. Once this is done, we will be able to remove most of the new custom variants added. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
Description
New typography toolkit for MUI carto-theme and added several custom variants.
Type of change