-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
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
RFC: material typography spec v2 #12741
Comments
@eps1lon Thanks for setting that up. We might not need a codemod for such change, the search&replace can potentially be simple to perform. I have no objection otherwise. ping @mui-org/core-contributors |
Maybe we could provide a new Or maybe (even better) allow selecting the typography style in the theme? Thus making the change almost non-breaking by setting all typography properties in the theme and allow users to bring back the old typography config. |
@leMaik I would rather avoid this type of operational overhead. What's wrong with adding a depreciation in a minor and removing the old variant in a major?
I love the idea. It's way more flexibility. We need to find a way to have dynamic prop-types, doesn't sounds good for TypeScript users. We also need a way to identify the theme.typography keys that are variants. |
Nothing. |
Maybe this seems overly pedantic but we do implement the material design and if this design changes so should we. This is currently implemented with a simple feature flag in |
I say we just include the deprecation warnings and remove them in the next major. |
@Abbo44 I'm aware of the spec. The new variants are almost finished. I just need to polish the deprecation warnings so that they don't interfere with our own test suite. |
Implemented typography v2 with deprecation warnings and docs in typography v2 in #12916. |
The deprecation warning lists Also the Stepper component is still using Kind regards, |
@elmeerr |
@eps1lon Indeed, my bad! Just saw one warning and assume that was all the same. But the |
I am working through the demo component ClippedDrawer and noticed these warnings below. Being a newbie on this I am not sure if I am doing something wrong or need to update something on the demo in GitHub to prepare the ClippedDrawer demo for the next version? Warning: Material-UI: You are using the deprecated typography variant subheading that will be removed in the next major release. Warning: Material-UI: You are using the typography variant body1 which will be restyled in the next major release. |
Just in case any other newbies like me run across my question above with the default examples and getting warnings. I was able to solve it with the content on this page: https://material-ui.com/customization/themes/ In particular, it seems the default theme was the culprit and the above discussion makes more sense now about the find/replace. I suspect that after that is done then the examples, like ClippedDrawer, that uses variants that trigger the warnings will be resolved. In the meantime, I did the following to create a custom theme with the only option being the useNextVariants set to true and the warnings went away now. Here is my create-react-app code that I am using to test which seems to have resolved it for the ClippedDrawer example.
|
Hi @eps1lon , I think you forgot to implement the Actual TypographyClassKey :
|
@oliviertassinari I was wondering why this was changed to I think it is a lot more confusing to use |
@novascreen Because of https://twitter.com/olivtassinari/status/1044347621425520642 We are using the following rewrite at work: <Typography headlineMapping={{
h1: 'h1',
h2: 'h1',
title: 'h1',
}} /> |
Fair enough |
@novascreen Yet, you are definitely right, |
@3rwww1 const theme = createMuiTheme({
typography: {
allVariants: {
letterSpacing: 0
},
}
}); |
@eps1lon thanks for the tip! Please disregard my comment and sorry for the noise. Background: I'm aware of the spec change and actually checked out https://material.io/design/typography/the-type-system.html#type-scale before posting. Initially thought you might have misinterpreted the letter-spacing conversion to web units - sorry for doubting you folks 🙏 Then checked with different browsers and OSes, and AFAIK this kerning issue only happens on Firefox Linux (OK on Chrome/Linux, Chrome/OSX, Firefox/OSX). The difference is quite striking though (at least for me): I do not have a Windows setup install (or different Linux flavor) handy to run further tests. |
This comment has been minimized.
This comment has been minimized.
So, in case someone else also notices this weird font rendering after migrating, this is officially a Firefox kerning issue: Summary:
|
@3rwww1 We considered removing the letter-spacing entirely because @oliviertassinari had concerns with kerning issues but I'm pretty adamant when it comes to spec compliance by default. The only difference I can spot is when using |
As suggested by @eps1lon I have the following in my createMuiTheme to suppress the odd looking extra letter spacing (particularly in FF) after upgrade to v2 typography: typography: {
useNextVariants: true,
allVariants: {
letterSpacing: 0
},
} However, that has not removed any letter spacing from any of my themed/styled components. Am I missing something? |
@wijwoj Something is wrong with your setup: https://codesandbox.io/s/5xy7q2ykxx. |
Ah sorry. Thanks for checking. Will dig in and work out what am doing wrong. Thanks. EDIT: I am a dufus. That typography config section has somehow found its way into the palette section. All working now. Thanks @oliviertassinari . |
Google seems to have updated their color specs for Typography (?). Unlike MUI defines "primary", "secondary", "disabled" and "hint", google only defines "high emphasis", "medium emphasis" and "disabled". Should MUI take over these changes accordingly? see https://material.io/design/color/text-legibility.html# |
@FranBran Could you open a separate issue and provide links to the spec? You can also just go ahead and open a PR to change it if it's indeed defined that way in the spec. |
please add useNextVariants type |
@rlarnfktkd1 This was removed in v4. |
This is a proposal for a migration path from the v1 typography spec to v2.
Breaking changes
Implementation
Color
The spec does not mention color. Use primary by default and apply color manually on every usage.
Variant Name
Use names from spec but use
headlineX
instead ofhX
which matches mwc implementationRoadmap
Next
setUsers should be warned if an internal component uses a deprecated variant. Recommend settingsuppressDeprecationWarning
on every internal Typography usage to suppress warnings. They are for the user not the dev.useNextVariants
per component.useNextVariants
to Typography. This will use the style of variants in the next major (only concerns variants that will be restyledfor all variants so that usinguseNextVariants
has no breaking change when switching to new major)useNextVariants
,suppressDeprecationWarning
will become noop propsuseNextVariants
,suppressDeprecationWarning
propsTypography
, use codemodIf we don't consider style changes a breaking change we can skip the hole
suppressDeprecationWarning
step which reduces the workload but is probably annoying for users that aim at pixel perfect apps.Misc
Spec:
Existing Implementations:
Related:
@oliviertassinari Could you ping core contributors?
The text was updated successfully, but these errors were encountered: