From c2c8263341892766e6c7c094485a6ad02fc90333 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Tue, 10 Jan 2023 11:41:50 +0100 Subject: [PATCH] fix(material/core): default font family not applied to custom typography levels Fixes the following regressions with the new typography config: * After #25789, the top-level font family wasn't being applied to the individual levels anymore. * Custom typography levels were defaulting to a `null` font family while the default ones were defaulting to `Roboto` which was inconsistent. Now they both default to `Roboto` like they did in the legacy config. I've also set up tests for the config since changing the logic can be annoying to debug. Fixes #26387. --- .../core/mdc-helpers/_mdc-helpers.scss | 15 +-- src/material/core/theming/tests/BUILD.bazel | 9 ++ .../tests/test-typography-font-family.scss | 107 ++++++++++++++++++ .../core/typography/_all-typography.scss | 98 ++++++++-------- 4 files changed, 171 insertions(+), 58 deletions(-) create mode 100644 src/material/core/theming/tests/test-typography-font-family.scss diff --git a/src/material/core/mdc-helpers/_mdc-helpers.scss b/src/material/core/mdc-helpers/_mdc-helpers.scss index eced4eea4d6a..3c795cecd87a 100644 --- a/src/material/core/mdc-helpers/_mdc-helpers.scss +++ b/src/material/core/mdc-helpers/_mdc-helpers.scss @@ -84,16 +84,17 @@ $mat-typography-mdc-level-mappings: ( } // Converts an MDC typography level config to an Angular Material one. -@function typography-config-level-from-mdc($mdc-level, $overrides: ()) { +@function typography-config-level-from-mdc($mdc-level) { $mdc-level-config: map.get(mdc-typography.$styles, $mdc-level); + // Explicitly set the font family to null since we'll apply it globally + // through the `define-typgraphy-config`/`define-legacy-typography-config`. @return typography.define-typography-level( - $font-size: map.get($overrides, font-size) or map.get($mdc-level-config, font-size), - $font-family: map.get($overrides, font-family) or map.get($mdc-level-config, font-family), - $line-height: map.get($overrides, line-height) or map.get($mdc-level-config, line-height), - $font-weight: map.get($overrides, font-weight) or map.get($mdc-level-config, font-weight), - $letter-spacing: - map.get($overrides, letter-spacing) or map.get($mdc-level-config, letter-spacing) + $font-family: null, + $font-size: map.get($mdc-level-config, font-size), + $line-height: map.get($mdc-level-config, line-height), + $font-weight: map.get($mdc-level-config, font-weight), + $letter-spacing: map.get($mdc-level-config, letter-spacing) ); } diff --git a/src/material/core/theming/tests/BUILD.bazel b/src/material/core/theming/tests/BUILD.bazel index dd77e9ffd842..78a3c954bc70 100644 --- a/src/material/core/theming/tests/BUILD.bazel +++ b/src/material/core/theming/tests/BUILD.bazel @@ -45,6 +45,14 @@ sass_binary( deps = ["//src/material:sass_lib"], ) +# Sass binary that asserts that setting the font family for a typography config works as expected. +sass_binary( + name = "test-typography-font-family", + testonly = True, + src = "test-typography-font-family.scss", + deps = ["//src/material:sass_lib"], +) + build_test( name = "sass_compile_tests", targets = [ @@ -52,6 +60,7 @@ build_test( ":test-theming-api", ":test-theming-bundle", ":test-legacy-theming-bundle", + ":test-typography-font-family", ], ) diff --git a/src/material/core/theming/tests/test-typography-font-family.scss b/src/material/core/theming/tests/test-typography-font-family.scss new file mode 100644 index 000000000000..2f8e94554e57 --- /dev/null +++ b/src/material/core/theming/tests/test-typography-font-family.scss @@ -0,0 +1,107 @@ +@use '@material/typography' as mdc-typography; +@use '../../typography/all-typography'; +@use '../../typography/typography'; +@use 'sass:map'; +@use 'sass:meta'; + +@function assert-font-family($test-name, $obj, $expected) { + @each $level-name, $level in $obj { + @if (meta.type-of($level) == 'map' and map.get($level, 'font-family') != $expected) { + @error '[#{$test-name}]: Incorrect font-family in level "#{$level-name}". ' + + 'Expected "#{$expected}", but received "#{map.get($level, 'font-family')}".'; + } + } + @return $obj; +} + +$no-font-family: assert-font-family( + 'should take default MDC font family if none is specified', + all-typography.define-typography-config(), + mdc-typography.$font-family); + +$only-top-level-font-family: assert-font-family( + 'should take custom font family if specified at top level', + all-typography.define-typography-config($font-family: 'custom-top'), + 'custom-top'); + +$individual-levels-without-font-families: assert-font-family( + 'should set the default MDC font family if all keys are specified, but without a font-family', + all-typography.define-typography-config( + $headline-1: typography.define-typography-level($font-size: 1px), + $headline-2: typography.define-typography-level($font-size: 1px), + $headline-3: typography.define-typography-level($font-size: 1px), + $headline-4: typography.define-typography-level($font-size: 1px), + $headline-5: typography.define-typography-level($font-size: 1px), + $headline-6: typography.define-typography-level($font-size: 1px), + $subtitle-1: typography.define-typography-level($font-size: 1px), + $subtitle-2: typography.define-typography-level($font-size: 1px), + $body-1: typography.define-typography-level($font-size: 1px), + $body-2: typography.define-typography-level($font-size: 1px), + $caption: typography.define-typography-level($font-size: 1px), + $button: typography.define-typography-level($font-size: 1px), + $overline: typography.define-typography-level($font-size: 1px), + ), + mdc-typography.$font-family +); + +$individual-levels-without-font-families-with-top-level-family: assert-font-family( + 'should set a custom top-level font family if all keys are specified, but without a font-family', + all-typography.define-typography-config( + $font-family: 'custom-top', + $headline-1: typography.define-typography-level($font-size: 1px), + $headline-2: typography.define-typography-level($font-size: 1px), + $headline-3: typography.define-typography-level($font-size: 1px), + $headline-4: typography.define-typography-level($font-size: 1px), + $headline-5: typography.define-typography-level($font-size: 1px), + $headline-6: typography.define-typography-level($font-size: 1px), + $subtitle-1: typography.define-typography-level($font-size: 1px), + $subtitle-2: typography.define-typography-level($font-size: 1px), + $body-1: typography.define-typography-level($font-size: 1px), + $body-2: typography.define-typography-level($font-size: 1px), + $caption: typography.define-typography-level($font-size: 1px), + $button: typography.define-typography-level($font-size: 1px), + $overline: typography.define-typography-level($font-size: 1px), + ), + 'custom-top' +); + +$individual-levels-with-font-families: assert-font-family( + 'should use the level font family if one is specified, but there is none at the top level', + all-typography.define-typography-config( + $headline-1: typography.define-typography-level($font-size: 1px, $font-family: 'custom-level'), + $headline-2: typography.define-typography-level($font-size: 1px, $font-family: 'custom-level'), + $headline-3: typography.define-typography-level($font-size: 1px, $font-family: 'custom-level'), + $headline-4: typography.define-typography-level($font-size: 1px, $font-family: 'custom-level'), + $headline-5: typography.define-typography-level($font-size: 1px, $font-family: 'custom-level'), + $headline-6: typography.define-typography-level($font-size: 1px, $font-family: 'custom-level'), + $subtitle-1: typography.define-typography-level($font-size: 1px, $font-family: 'custom-level'), + $subtitle-2: typography.define-typography-level($font-size: 1px, $font-family: 'custom-level'), + $body-1: typography.define-typography-level($font-size: 1px, $font-family: 'custom-level'), + $body-2: typography.define-typography-level($font-size: 1px, $font-family: 'custom-level'), + $caption: typography.define-typography-level($font-size: 1px, $font-family: 'custom-level'), + $button: typography.define-typography-level($font-size: 1px, $font-family: 'custom-level'), + $overline: typography.define-typography-level($font-size: 1px, $font-family: 'custom-level'), + ), + 'custom-level' +); + +$individual-levels-with-font-families-and-top-level-family: assert-font-family( + 'should use the level font family if a top-level one is specified together with it', + all-typography.define-typography-config( + $font-family: 'custom-top', + $headline-1: typography.define-typography-level($font-size: 1px, $font-family: 'custom-level'), + $headline-2: typography.define-typography-level($font-size: 1px, $font-family: 'custom-level'), + $headline-3: typography.define-typography-level($font-size: 1px, $font-family: 'custom-level'), + $headline-4: typography.define-typography-level($font-size: 1px, $font-family: 'custom-level'), + $headline-5: typography.define-typography-level($font-size: 1px, $font-family: 'custom-level'), + $headline-6: typography.define-typography-level($font-size: 1px, $font-family: 'custom-level'), + $subtitle-1: typography.define-typography-level($font-size: 1px, $font-family: 'custom-level'), + $subtitle-2: typography.define-typography-level($font-size: 1px, $font-family: 'custom-level'), + $body-1: typography.define-typography-level($font-size: 1px, $font-family: 'custom-level'), + $body-2: typography.define-typography-level($font-size: 1px, $font-family: 'custom-level'), + $caption: typography.define-typography-level($font-size: 1px, $font-family: 'custom-level'), + $button: typography.define-typography-level($font-size: 1px, $font-family: 'custom-level'), + $overline: typography.define-typography-level($font-size: 1px, $font-family: 'custom-level'), + ), + 'custom-level' +); diff --git a/src/material/core/typography/_all-typography.scss b/src/material/core/typography/_all-typography.scss index 8fe4d27d3968..229fd0b3f43d 100644 --- a/src/material/core/typography/_all-typography.scss +++ b/src/material/core/typography/_all-typography.scss @@ -1,3 +1,4 @@ +@use '@material/typography' as mdc-typography; @use 'sass:map'; @use 'sass:math'; @use 'sass:meta'; @@ -62,6 +63,20 @@ } } +// Applies the default font family to all levels in a typography config. +@function _apply-font-family($font-family, $initial-config) { + $config: $initial-config; + + @each $key, $level in $config { + @if map.get($level, 'font-family') == null { + // Sass maps are immutable so we have to re-assign the variable each time. + $config: map.set($config, $key, map.set($level, 'font-family', $font-family)); + } + } + + @return map.set($config, 'font-family', $font-family); +} + /// Generates an Angular Material typography config based on values from the official Material /// Design spec implementation (MDC Web). All arguments are optional, but may be passed to override /// the default values. The `mat-typography-level` function can be used to generate a custom @@ -87,7 +102,7 @@ @function define-typography-config( // TODO(mmalerba): rename this function to define-typography-config, // and create a predefined px based config for people that need it. - $font-family: null, + $font-family: mdc-typography.$font-family, $headline-1: null, $headline-2: null, $headline-3: null, @@ -102,37 +117,21 @@ $button: null, $overline: null, ) { - // Declare an initial map with all of the levels. - $overrides: if($font-family, (font-family: $font-family), ()); - - @return ( - headline-1: $headline-1 or _rem-to-px( - mdc-helpers.typography-config-level-from-mdc(headline1, $overrides)), - headline-2: $headline-2 or _rem-to-px( - mdc-helpers.typography-config-level-from-mdc(headline2, $overrides)), - headline-3: $headline-3 or _rem-to-px( - mdc-helpers.typography-config-level-from-mdc(headline3, $overrides)), - headline-4: $headline-4 or _rem-to-px( - mdc-helpers.typography-config-level-from-mdc(headline4, $overrides)), - headline-5: $headline-5 or _rem-to-px( - mdc-helpers.typography-config-level-from-mdc(headline5, $overrides)), - headline-6: $headline-6 or _rem-to-px( - mdc-helpers.typography-config-level-from-mdc(headline6, $overrides)), - subtitle-1: $subtitle-1 or _rem-to-px( - mdc-helpers.typography-config-level-from-mdc(subtitle1, $overrides)), - subtitle-2: $subtitle-2 or _rem-to-px( - mdc-helpers.typography-config-level-from-mdc(subtitle2, $overrides)), - body-1: $body-1 or _rem-to-px( - mdc-helpers.typography-config-level-from-mdc(body1, $overrides)), - body-2: $body-2 or _rem-to-px( - mdc-helpers.typography-config-level-from-mdc(body2, $overrides)), - caption: $caption or _rem-to-px( - mdc-helpers.typography-config-level-from-mdc(caption, $overrides)), - button: $button or _rem-to-px( - mdc-helpers.typography-config-level-from-mdc(button, $overrides)), - overline: $overline or _rem-to-px( - mdc-helpers.typography-config-level-from-mdc(overline, $overrides)), - ); + @return _apply-font-family($font-family, ( + headline-1: $headline-1 or _rem-to-px(mdc-helpers.typography-config-level-from-mdc(headline1)), + headline-2: $headline-2 or _rem-to-px(mdc-helpers.typography-config-level-from-mdc(headline2)), + headline-3: $headline-3 or _rem-to-px(mdc-helpers.typography-config-level-from-mdc(headline3)), + headline-4: $headline-4 or _rem-to-px(mdc-helpers.typography-config-level-from-mdc(headline4)), + headline-5: $headline-5 or _rem-to-px(mdc-helpers.typography-config-level-from-mdc(headline5)), + headline-6: $headline-6 or _rem-to-px(mdc-helpers.typography-config-level-from-mdc(headline6)), + subtitle-1: $subtitle-1 or _rem-to-px(mdc-helpers.typography-config-level-from-mdc(subtitle1)), + subtitle-2: $subtitle-2 or _rem-to-px(mdc-helpers.typography-config-level-from-mdc(subtitle2)), + body-1: $body-1 or _rem-to-px(mdc-helpers.typography-config-level-from-mdc(body1)), + body-2: $body-2 or _rem-to-px(mdc-helpers.typography-config-level-from-mdc(body2)), + caption: $caption or _rem-to-px(mdc-helpers.typography-config-level-from-mdc(caption)), + button: $button or _rem-to-px(mdc-helpers.typography-config-level-from-mdc(button)), + overline: $overline or _rem-to-px(mdc-helpers.typography-config-level-from-mdc(overline)), + )); } /// Generates an Angular Material typography config based on values from the official Material @@ -160,7 +159,7 @@ @function define-rem-typography-config( // TODO(mmalerba): rename this function to define-typography-config, // and create a predefined px based config for people that need it. - $font-family: null, + $font-family: mdc-typography.$font-family, $headline-1: null, $headline-2: null, $headline-3: null, @@ -175,24 +174,21 @@ $button: null, $overline: null, ) { - // Declare an initial map with all of the levels. - $overrides: if($font-family, (font-family: $font-family), ()); - - @return ( - headline-1: $headline-1 or mdc-helpers.typography-config-level-from-mdc(headline1, $overrides), - headline-2: $headline-2 or mdc-helpers.typography-config-level-from-mdc(headline2, $overrides), - headline-3: $headline-3 or mdc-helpers.typography-config-level-from-mdc(headline3, $overrides), - headline-4: $headline-4 or mdc-helpers.typography-config-level-from-mdc(headline4, $overrides), - headline-5: $headline-5 or mdc-helpers.typography-config-level-from-mdc(headline5, $overrides), - headline-6: $headline-6 or mdc-helpers.typography-config-level-from-mdc(headline6, $overrides), - subtitle-1: $subtitle-1 or mdc-helpers.typography-config-level-from-mdc(subtitle1, $overrides), - subtitle-2: $subtitle-2 or mdc-helpers.typography-config-level-from-mdc(subtitle2, $overrides), - body-1: $body-1 or mdc-helpers.typography-config-level-from-mdc(body1, $overrides), - body-2: $body-2 or mdc-helpers.typography-config-level-from-mdc(body2, $overrides), - caption: $caption or mdc-helpers.typography-config-level-from-mdc(caption, $overrides), - button: $button or mdc-helpers.typography-config-level-from-mdc(button, $overrides), - overline: $overline or mdc-helpers.typography-config-level-from-mdc(overline, $overrides), - ); + @return _apply-font-family($font-family, ( + headline-1: $headline-1 or mdc-helpers.typography-config-level-from-mdc(headline1), + headline-2: $headline-2 or mdc-helpers.typography-config-level-from-mdc(headline2), + headline-3: $headline-3 or mdc-helpers.typography-config-level-from-mdc(headline3), + headline-4: $headline-4 or mdc-helpers.typography-config-level-from-mdc(headline4), + headline-5: $headline-5 or mdc-helpers.typography-config-level-from-mdc(headline5), + headline-6: $headline-6 or mdc-helpers.typography-config-level-from-mdc(headline6), + subtitle-1: $subtitle-1 or mdc-helpers.typography-config-level-from-mdc(subtitle1), + subtitle-2: $subtitle-2 or mdc-helpers.typography-config-level-from-mdc(subtitle2), + body-1: $body-1 or mdc-helpers.typography-config-level-from-mdc(body1), + body-2: $body-2 or mdc-helpers.typography-config-level-from-mdc(body2), + caption: $caption or mdc-helpers.typography-config-level-from-mdc(caption), + button: $button or mdc-helpers.typography-config-level-from-mdc(button), + overline: $overline or mdc-helpers.typography-config-level-from-mdc(overline), + )); } // Includes all of the typographic styles.