-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
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
[Typography] Remove deprecated Typography variants and v4 changes #14562
Conversation
b190be5
to
41138a4
Compare
}; | ||
|
||
Typography.defaultProps = { | ||
align: 'inherit', | ||
color: 'default', | ||
color: 'inherit', | ||
display: 'initial', |
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.
should this be default
instead?
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.
Initial is a valid CSS value: https://developer.mozilla.org/en-US/docs/Web/CSS/display. I can't find default.
f5f3ed5
to
c8c349a
Compare
I’m going to remove the default colour change from this PR to make it easier to see if things have been affected. |
b7a441b
to
2027427
Compare
The argos changes left are because of the subheading -> subtitle1 change for the menu and the display block change affecting spacing. I've left the display block change one to get some feedback |
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 would prefer if we kept PRs isolated in the future. This does now include:
- a breaking change with a deprecation in place (typo v2)
- a breaking change without a deprecation (headlingMapping)
- a breaking change without a deprecation (no more
block
forLink
) - feature
Each of those things should be kept in separate PRs in the future. Apart from following "a commit should do one thing" the change in bundle size cannot be linked to any particular change now and review is harder. For example it wasn't obvious why we removed block
in Link
at first until I had read the PR summary again.
I have followed (at least) the PR section of the contributing guide.
Remove deprecated variants
Rename
headlineMapping
tovariantMapping
Add
Typography
display
propertyBreaking changes
display: block
default typograpghy style.You can use the new
display?: 'initial' | 'inline' | 'block';
property.headlineMapping
property to better align with its purpose.Closes #12741