Skip to content
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

Merged
merged 11 commits into from
Feb 18, 2019

Conversation

joshwooding
Copy link
Member

@joshwooding joshwooding commented Feb 16, 2019

Breaking changes

  • Remove the deprecated typography variants. You can upgrade by performing the following replacements:
    • display4 => h1
    • display3 => h2
    • display2 => h3
    • display1 => h4
    • headline => h5
    • title => h6
    • subheading => subtitle1
    • body2 => body1
    • body1 (default) => body2 (default)
  • Remove the opinionated display: block default typograpghy style.
    You can use the new display?: 'initial' | 'inline' | 'block'; property.
  • Rename the headlineMapping property to better align with its purpose.
-<MuiTypography headlineMapping={headlineMapping}>
+<MuiTypography variantMapping={variantMapping}>

Closes #12741

};

Typography.defaultProps = {
align: 'inherit',
color: 'default',
color: 'inherit',
display: 'initial',
Copy link
Member Author

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?

Copy link
Member

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.

@joshwooding
Copy link
Member Author

I’m going to remove the default colour change from this PR to make it easier to see if things have been affected.

@joshwooding
Copy link
Member Author

joshwooding commented Feb 17, 2019

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

Copy link
Member

@eps1lon eps1lon left a 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 for Link)
  • 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.

@oliviertassinari oliviertassinari merged commit 6a92083 into mui:next Feb 18, 2019
@joshwooding joshwooding deleted the v4-typography branch March 24, 2019 22:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants