refactor: fixes for theming api refactor and introduction of a theming API test

With the initial commits for the density system, a function for
retrieving the color configuration from the theme object has been
introduced.

This function is currently flawed since it always uses the theme object
as color configuration if the theme does not have a `color` property.
This is incorrect because the `color` config can be explicitly set to
`null`, or omitted if no color styles are desired.

Author: devversion

src/material/core/BUILD.bazel

@@ -138,11 +138,3 @@ filegroup(
     name = "source-files",
     srcs = glob(["**/*.ts"]),
 )
-
-# Test theme used to ensure that our themes will compile using CSS variables in the palettes.
-sass_binary(
-    name = "test-css-variables-theme",
-    testonly = True,
-    src = "theming/test-css-variables-theme.scss",
-    deps = [":theming_scss_lib"],
-)

src/material/core/_core.scss

@@ -19,38 +19,44 @@
   @include cdk-text-field();
 }
 
-// Mixin that renders all of the core styles that depend on the theme.
-@mixin mat-core-theme($theme) {
-  $color: mat-get-color-config($theme);
-  @include mat-ripple-theme($theme);
-  @include mat-option-theme($theme);
-  @include mat-optgroup-theme($theme);
-  @include mat-pseudo-checkbox-theme($theme);
-
-  // Provides external CSS classes for each elevation value. Each CSS class is formatted as
-  // `mat-elevation-z$zValue` where `$zValue` corresponds to the z-space to which the element is
-  // elevated.
-  @for $zValue from 0 through 24 {
-    .#{$_mat-elevation-prefix}#{$zValue} {
-      @include _mat-theme-elevation($zValue, $color);
-    }
-  }
-
+@mixin mat-core-color($config) {
   // Wrapper element that provides the theme background when the user's content isn't
   // inside of a `mat-sidenav-container`. Note that we need to exclude the ampersand
   // selector in case the mixin is included at the top level.
   .mat-app-background#{if(&, ', &.mat-app-background', '')} {
-    $background: map-get($color, background);
-    $foreground: map-get($color, foreground);
+    $background: map-get($config, background);
+    $foreground: map-get($config, foreground);
 
     background-color: mat-color($background, background);
     color: mat-color($foreground, text);
   }
 
+  // Provides external CSS classes for each elevation value. Each CSS class is formatted as
+  // `mat-elevation-z$zValue` where `$zValue` corresponds to the z-space to which the element is
+  // elevated.
+  @for $zValue from 0 through 24 {
+    .#{$_mat-elevation-prefix}#{$zValue} {
+      @include _mat-theme-elevation($zValue, $config);
+    }
+  }
+
   // Marker that is used to determine whether the user has added a theme to their page.
   @at-root {
     .mat-theme-loaded-marker {
       display: none;
     }
   }
 }
+
+// Mixin that renders all of the core styles that depend on the theme.
+@mixin mat-core-theme($theme) {
+  @include mat-ripple-theme($theme);
+  @include mat-option-theme($theme);
+  @include mat-optgroup-theme($theme);
+  @include mat-pseudo-checkbox-theme($theme);
+
+  $color: mat-get-color-config($theme);
+  @if $color != null {
+    @include mat-core-color($color);
+  }
+}

src/material/core/theming/_theming.scss

@@ -67,7 +67,7 @@
 // Validates the specified theme by ensuring that the optional color config defines
 // a primary, accent and warn palette. Returns the theme if no failures were found.
 @function _mat-validate-theme($theme) {
-  @if map_has_key($theme, color) {
+  @if map_get($theme, color) {
     $color: map_get($theme, color);
     @if not map_get($color, primary) {
       @error 'Theme does not define a valid "primary" palette.';
@@ -82,6 +82,16 @@
   @return $theme;
 }
 
+// Whether the specified object is a color configuration. This function is needed for backwards
+// compatibility of individual component theme mixins, because developers could pass in a color
+// configuration instead of a theme container object to a theme mixin.
+@function _mat-is-color-config($config) {
+  @return map_has_key($config, primary) and
+          map_has_key($config, accent) and map_has_key($config, warn) and
+          map_has_key($config, is-dark) and map_has_key($config, foreground) and
+          map_has_key($config, background);
+}
+
 // Creates a light-themed color configuration from the specified
 // primary, accent and warn palettes.
 @function _mat-create-light-color-config($primary, $accent, $warn: null) {
@@ -176,12 +186,16 @@
   @return _mat-validate-theme($result);
 }
 
-// Needed for backwards compatibility. If the passed `$theme` variable is not a
-// overall theme configuration, we use it as color config.
-@function mat-get-color-config($theme, $default: $theme) {
+@function mat-get-color-config($theme, $default: null) {
   @if map_has_key($theme, color) {
     @return map_get($theme, color);
   }
+  // Before we introduced the new pattern for construcing a theme, developers passed the color
+  // configuration to the theme mixins. This can be still the case if developers construct a
+  // theme manually and pass it to a theme. We support this for backwards compatibility.
+  @if _mat-is-color-config($theme) {
+    @return $theme;
+  }
   @return $default;
 }
 

src/material/core/theming/tests/BUILD.bazel

@@ -0,0 +1,22 @@
+package(default_visibility = ["//visibility:public"])
+
+load("//tools:defaults.bzl", "sass_binary")
+
+# Test theme used to ensure that our themes will compile using CSS variables in
+# the palettes.
+sass_binary(
+    name = "test-css-variables-theme",
+    testonly = True,
+    src = "test-css-variables-theme.scss",
+    deps = ["//src/material/core:theming_scss_lib"],
+)
+
+# Sass binary which is used to ensure that our themes will compile with the legacy and new
+# pattern for configuring themes. For more information on the legacy and new pattern, inspect
+# the `mat-light-theme` and `mat-dark-theme` theming functions.
+sass_binary(
+    name = "test-theming-api",
+    testonly = True,
+    src = "test-theming-api.scss",
+    deps = ["//src/material/core:theming_scss_lib"],
+)

src/material/core/theming/test-css-variables-theme.scss renamed to src/material/core/theming/tests/test-css-variables-theme.scss

@@ -1,4 +1,4 @@
-@import './all-theme';
+@import '../all-theme';
 
 // Recursively replaces all of the values inside a Sass map with a different value.
 @function replace-all-values($palette, $replacement) {

src/material/core/theming/tests/test-theming-api.scss

@@ -0,0 +1,179 @@
+@import '../all-theme';
+
+// A new way to configure themes has been introduced. This Sass file ensures that the theming
+// API is backwards compatible, so that the old pattern for configuring themes can be still
+// used. The Sass compilation of this file will fail if one of the tests/assertions fails.
+
+$primary: mat-palette($mat-blue);
+$accent: mat-palette($mat-grey);
+$warn: mat-palette($mat-indigo);
+$default-warn: mat-palette($mat-red);
+
+// Asserts that a given value matches the expected value. If not, an error is
+// thrown with the given error message.
+@mixin assert($value, $expected, $error-msg) {
+  @if $value != $expected {
+    @error 'Expected "#{$value}" to be "#{$expected}". #{$error-msg}';
+  }
+}
+
+// Asserts that the specified color configuration is configured properly.
+@mixin assert-color-config($config, $primary, $accent, $warn, $is-dark) {
+  $color: map_get($config, color);
+  @include assert(map_get($color, primary), $primary,
+      'Primary palette does not match expected palette.');
+  @include assert(map_get($color, accent), $accent,
+      'Accent palette does not match expected palette.');
+  @include assert(map_get($color, warn), $warn,
+      'Warn palette does not match expected palette.');
+  @include assert(map_get($color, is-dark), $is-dark,
+      'Expected color config `is-dark` to be: #{$is-dark}');
+}
+
+$legacy-light-theme: mat-light-theme($primary, $accent, $warn);
+$legacy-dark-theme: mat-dark-theme($primary, $accent, $warn);
+$legacy-light-theme-default-warn: mat-light-theme($primary, $accent);
+$legacy-dark-theme-default-warn: mat-dark-theme($primary, $accent);
+
+$new-api-light-theme: mat-light-theme((
+  color: (primary: $primary, accent: $accent, warn: $warn)
+));
+$new-api-dark-theme: mat-dark-theme((
+  color: (primary: $primary, accent: $accent, warn: $warn)
+));
+$new-api-light-theme-default-warn: mat-light-theme((
+  color: (primary: $primary, accent: $accent)
+));
+$new-api-dark-theme-default-warn: mat-dark-theme((
+  color: (primary: $primary, accent: $accent)
+));
+
+$light-theme-only-density: mat-light-theme((density: -3));
+$dark-theme-only-density: mat-dark-theme((density: -3));
+
+$typography-config: mat-typography-config();
+$light-theme-only-typography: mat-light-theme((typography: $typography-config));
+$dark-theme-only-typography: mat-dark-theme((typography: $typography-config));
+
+// Test which ensures that constructed themes by default do not set configurations
+// for the individual parts of the theming system (color, density, typography).
+@mixin test-default-theming-system-configs() {
+  $themes: ($legacy-light-theme, $legacy-dark-theme, $new-api-light-theme, $new-api-dark-theme);
+  @each $theme in $themes {
+    @include assert(map_has_key($theme, color), true,
+      'Expected color to be explicitly configured.');
+    @include assert(map_has_key($theme, typography), false,
+      'Expected typography to be not explicitly configured.');
+    @include assert(map_has_key($theme, density), false,
+      'Expected density to be not explicitly configured.');
+  }
+  @include assert(map_has_key($light-theme-only-density, color), false,
+      'Expected color to be not explicitly configured.');
+  @include assert(map_has_key($dark-theme-only-density, color), false,
+      'Expected color to be not explicitly configured.');
+}
+
+// Test which ensures that constructed themes have the proper color configurations
+// as specified in the `mat-light-theme` and `mat-dark-theme` functions.
+@mixin test-create-color-config() {
+  @include assert-color-config($legacy-light-theme, $primary, $accent, $warn, false);
+  @include assert-color-config($legacy-dark-theme, $primary, $accent, $warn, true);
+  @include assert-color-config($legacy-light-theme-default-warn, $primary, $accent,
+    $default-warn, false);
+  @include assert-color-config($legacy-dark-theme-default-warn, $primary, $accent,
+    $default-warn, true);
+  @include assert-color-config($new-api-light-theme, $primary, $accent, $warn, false);
+  @include assert-color-config($new-api-dark-theme, $primary, $accent, $warn, true);
+  @include assert-color-config($new-api-light-theme-default-warn, $primary, $accent,
+    $default-warn, false);
+  @include assert-color-config($new-api-dark-theme-default-warn, $primary, $accent,
+    $default-warn, true);
+}
+
+// Test which ensures that constructed themes pass through the specified
+// density and typography configurations.
+@mixin test-density-typography-config-pass-through() {
+  @include assert(map_get($light-theme-only-density, density), -3,
+    'Expected density config to be included in theme.');
+  @include assert(map_get($dark-theme-only-density, density), -3,
+    'Expected density config to be included in theme.');
+  @include assert(map_get($light-theme-only-typography, typography), $typography-config,
+    'Expected typography config to be included in theme.');
+  @include assert(map_get($dark-theme-only-typography, typography), $typography-config,
+    'Expected typography config to be included in theme.');
+}
+
+// Test which ensures that color configurations can be properly read from
+// theme objects passed to individual component theme mixins.
+@mixin test-get-color-config() {
+  $color-config: map_get($legacy-light-theme, color);
+  $no-color-light-theme: mat-light-theme((color: null));
+  $no-color-dark-theme: mat-dark-theme((color: null));
+  @include assert(mat-get-color-config($color-config), $color-config,
+      'Expected that passing a color config to a theme will work for backwards compatibility.');
+  @include assert(mat-get-color-config($legacy-light-theme), $color-config,
+      'Expected that a color config can be read from a theme object.');
+  @include assert(mat-get-color-config($light-theme-only-density), null,
+      'Expected that by default, no color configuration is used.');
+  @include assert(mat-get-color-config($no-color-light-theme), null,
+      'Expected that no color configuration can be explicitly specified.');
+  @include assert(mat-get-color-config($no-color-dark-theme), null,
+    'Expected that no color configuration can be explicitly specified.');
+}
+
+// Test which ensures that density configurations can be properly read from
+// theme objects passed to individual component theme mixins.
+@mixin test-get-density-config() {
+  $density-config: map_get($light-theme-only-density, density);
+  $no-density-light-theme: mat-light-theme((density: null));
+  $no-density-dark-theme: mat-dark-theme((density: null));
+  @include assert(mat-get-density-config($light-theme-only-density), -3,
+    'Expected that a density config can be read from a theme object.');
+  @include assert(mat-get-density-config($light-theme-only-typography), 0,
+    'Expected that by default, the density system will be configured.');
+  @include assert(mat-get-density-config($no-density-light-theme), null,
+    'Expected that no density configuration can be explicitly specified.');
+  @include assert(mat-get-density-config($no-density-dark-theme), null,
+    'Expected that no density configuration can be explicitly specified.');
+}
+
+// Test which ensures that typography configurations can be properly read from
+// theme objects passed to individual component theme mixins.
+@mixin test-get-typography-config() {
+  $no-typography-light-theme: mat-light-theme((density: null));
+  $no-typography-dark-theme: mat-dark-theme((density: null));
+  @include assert(mat-get-typography-config($light-theme-only-typography), $typography-config,
+    'Expected that a typography config can be read from a theme object.');
+  @include assert(mat-get-typography-config($legacy-light-theme), null,
+    'Expected that by default, no typography configuration is used.');
+  @include assert(mat-get-typography-config($no-typography-light-theme), null,
+    'Expected that no typography configuration can be explicitly specified.');
+  @include assert(mat-get-typography-config($no-typography-dark-theme), null,
+    'Expected that no typography configuration can be explicitly specified.');
+}
+
+// Test which ensures that all Angular Material theme mixins accept a theme
+// object without throwing any exception.
+@mixin test-theme-mixins-successful-compilation() {
+  @include angular-material-theme($legacy-light-theme);
+  @include angular-material-theme($legacy-light-theme-default-warn);
+  @include angular-material-theme($legacy-dark-theme);
+  @include angular-material-theme($legacy-dark-theme-default-warn);
+  @include angular-material-theme($new-api-light-theme);
+  @include angular-material-theme($new-api-light-theme-default-warn);
+  @include angular-material-theme($new-api-dark-theme);
+  @include angular-material-theme($new-api-dark-theme-default-warn);
+  @include angular-material-theme($light-theme-only-density);
+  @include angular-material-theme($dark-theme-only-density);
+  @include angular-material-theme($light-theme-only-typography);
+  @include angular-material-theme($dark-theme-only-typography);
+}
+
+// Include all tests. Sass will throw if one of the tests fails.
+@include test-create-color-config();
+@include test-default-theming-system-configs();
+@include test-density-typography-config-pass-through();
+@include test-theme-mixins-successful-compilation();
+@include test-get-color-config();
+@include test-get-density-config();
+@include test-get-typography-config();

src/material/core/typography/_all-typography.scss

@@ -7,6 +7,7 @@
 @import '../../card/card-theme';
 @import '../../checkbox/checkbox-theme';
 @import '../../chips/chips-theme';
+@import '../../divider/divider-theme';
 @import '../../table/table-theme';
 @import '../../datepicker/datepicker-theme';
 @import '../../dialog/dialog-theme';
@@ -51,6 +52,7 @@
   @include mat-card-typography($config);
   @include mat-checkbox-typography($config);
   @include mat-chips-typography($config);
+  @include mat-divider-typography($config);
   @include mat-table-typography($config);
   @include mat-datepicker-typography($config);
   @include mat-dialog-typography($config);

src/material/divider/_divider-theme.scss

@@ -14,6 +14,8 @@
   }
 }
 
+@mixin mat-divider-typography($config) {}
+
 @mixin _mat-divider-density($density-scale) {}
 
 @mixin mat-divider-theme($theme) {