Skip to content

Commit

Permalink
[Typography] Only define letter spacing if default family is used
Browse files Browse the repository at this point in the history
  • Loading branch information
eps1lon committed Sep 23, 2018
1 parent 3350ba8 commit ab46bd3
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 3 deletions.
13 changes: 11 additions & 2 deletions packages/material-ui/src/styles/createTypography.js
Expand Up @@ -16,8 +16,9 @@ function round(value) {
* @see @link{https://material.io/design/typography/understanding-typography.html}
*/
export default function createTypography(palette, typography) {
const defaultFontFamiliy = '"Roboto", "Helvetica", "Arial", sans-serif';
const {
fontFamily = '"Roboto", "Helvetica", "Arial", sans-serif',
fontFamily: fontFamilyOption,
// The default font size of the Material Specification.
fontSize = 14, // px
fontWeightLight = 300,
Expand All @@ -41,6 +42,8 @@ export default function createTypography(palette, typography) {
...other
} = typeof typography === 'function' ? typography(palette) : typography;

const fontFamily = fontFamilyOption || defaultFontFamiliy;

warning(
!Object.keys(other).some(variant => deprecatedVariants.includes(variant)),
'Deprecation Warning: Material-UI: You are passing a deprecated variant to ' +
Expand Down Expand Up @@ -90,12 +93,18 @@ export default function createTypography(palette, typography) {
const utils = { getVariant, letterSpacingToEm, pxToRem };

const propertiesForCategory = (weight, size, casing, letterSpacing) => {
// The letter spacing was designed for the Roboto font-family. Using the same letter-spacing
// across font-families can cause issues with the kerning.
const robotoStyles = !fontFamilyOption
? { letterSpacing: letterSpacingToEm(letterSpacing, size) }
: {};

return {
color: palette.text.primary,
fontFamily,
fontSize: pxToRem(size),
fontWeight: weight,
letterSpacing: letterSpacingToEm(letterSpacing, size),
...robotoStyles,
...casing,
...allVariants,
};
Expand Down
7 changes: 6 additions & 1 deletion packages/material-ui/src/styles/createTypography.test.js
Expand Up @@ -3,7 +3,7 @@ import { mock } from 'sinon';
import createPalette from './createPalette';
import createTypography from './createTypography';

describe('createTypography', () => {
describe.only('createTypography', () => {
let palette;

before(() => {
Expand Down Expand Up @@ -66,6 +66,11 @@ describe('createTypography', () => {
});
});

it('only defines letter-spacing if the font-family is not overwritten', () => {
assert.isDefined(createTypography(palette, {}).headline1.letterSpacing);
assert.isUndefined(createTypography(palette, { fontFamily: 'Gotham' }).headline1.letterSpacing);
});

describe('typography v2 migration', () => {
let warning;

Expand Down

0 comments on commit ab46bd3

Please sign in to comment.