refactor(material/core): clean up focus indicators (#25078)

crisbeto · web-flow · commit 427e99ab6e15 · 2022-06-15T13:01:45.000+02:00
I skipped over some of the feedback on #25039 in order to get the focus indicator changes merged in quicker. This is a follow-up to address the remaining feedback.
diff --git a/src/dev-app/theme.scss b/src/dev-app/theme.scss
@@ -32,10 +32,12 @@ $candy-app-theme: mat.define-light-theme((
 @include experimental.popover-edit-theme($candy-app-theme);
 
 .demo-strong-focus {
-  // Include base styles for strong focus indicators.
-  $indicators-config: (border-color: mat.get-color-from-palette($candy-app-primary));
-  @include mat.strong-focus-indicators($indicators-config);
-  @include experimental.mdc-strong-focus-indicators($indicators-config);
+  // Note: we can theme the indicators directly through `strong-focus-indicators` as well.
+  // Use the theme so we have some coverage over the entire API surface.
+  @include mat.strong-focus-indicators();
+  @include mat.strong-focus-indicators-theme($candy-app-theme);
+  @include experimental.mdc-strong-focus-indicators();
+  @include experimental.mdc-strong-focus-indicators-theme($candy-app-theme);
 }
 
 // Include the alternative theme styles inside of a block with a CSS class. You can make this
@@ -57,9 +59,8 @@ $candy-app-theme: mat.define-light-theme((
 
   // Include the dark theme colors for focus indicators.
   &.demo-strong-focus {
-    $indicators-config: (border-color: mat.get-color-from-palette($dark-primary));
-    @include mat.strong-focus-indicators($indicators-config);
-    @include experimental.mdc-strong-focus-indicators($indicators-config);
+    @include mat.strong-focus-indicators-theme($dark-colors);
+    @include experimental.mdc-strong-focus-indicators-theme($dark-colors);
   }
 }
 
diff --git a/src/material-experimental/mdc-button/button.scss b/src/material-experimental/mdc-button/button.scss
@@ -113,15 +113,15 @@
 .mat-mdc-unelevated-button,
 .mat-mdc-raised-button {
   .mat-mdc-focus-indicator::before {
-    $default-border-width: mat.$focus-indicators-private-default-border-width;
+    $default-border-width: mat.$private-strong-focus-indicators-default-border-width;
     $border-width: var(--mat-mdc-focus-indicator-border-width, #{$default-border-width});
     $offset: calc(#{$border-width} + 2px);
     margin: calc(#{$offset} * -1);
   }
 }
 
 .mat-mdc-outlined-button .mat-mdc-focus-indicator::before {
-  $default-border-width: mat.$focus-indicators-private-default-border-width;
+  $default-border-width: mat.$private-strong-focus-indicators-default-border-width;
   $border-width: var(--mat-mdc-focus-indicator-border-width, #{$default-border-width});
   $offset: calc(#{$border-width} + 3px);
   margin: calc(#{$offset} * -1);
diff --git a/src/material-experimental/mdc-button/fab.scss b/src/material-experimental/mdc-button/fab.scss
@@ -67,7 +67,7 @@
   }
 
   .mat-mdc-focus-indicator::before {
-    $default-border-width: mat.$focus-indicators-private-default-border-width;
+    $default-border-width: mat.$private-strong-focus-indicators-default-border-width;
     $border-width: var(--mat-mdc-focus-indicator-border-width, #{$default-border-width});
     $offset: calc(#{$border-width} + 2px);
     margin: calc(#{$offset} * -1);
diff --git a/src/material-experimental/mdc-checkbox/BUILD.bazel b/src/material-experimental/mdc-checkbox/BUILD.bazel
@@ -49,7 +49,6 @@ sass_binary(
     ],
     deps = [
         ":mdc_checkbox_scss_lib",
-        "//src/cdk:sass_lib",
         "//src/material:sass_lib",
         "//src/material-experimental/mdc-helpers:mdc_helpers_scss_lib",
         "//src/material-experimental/mdc-helpers:mdc_scss_deps_lib",
diff --git a/src/material-experimental/mdc-chips/chip.scss b/src/material-experimental/mdc-chips/chip.scss
@@ -179,7 +179,7 @@
   // For the chip element, default inset/offset values are necessary to ensure that
   // the focus indicator is sufficiently contrastive and renders appropriately.
   .mat-mdc-focus-indicator::before {
-    $default-border-width: mat.$focus-indicators-private-default-border-width;
+    $default-border-width: mat.$private-strong-focus-indicators-default-border-width;
     $border-width: var(--mat-mdc-focus-indicator-border-width, #{$default-border-width});
     $offset: calc(#{$border-width} + 2px);
     margin: calc(#{$offset} * -1);
@@ -196,7 +196,7 @@
   }
 
   &::before {
-    $default-border-width: mat.$focus-indicators-private-default-border-width;
+    $default-border-width: mat.$private-strong-focus-indicators-default-border-width;
     $offset: var(--mat-mdc-focus-indicator-border-width, #{$default-border-width});
     margin: calc(#{$offset} * -1);
 
@@ -228,7 +228,8 @@
   }
 }
 
-// In the chips the individual actions have focus so we target a different element.
+// The chip has multiple focus targets so we have to put the indicator on
+// a separate element, rather than on the focusable element itself.
 .mat-mdc-chip-action:focus .mat-mdc-focus-indicator::before {
   content: '';
 }
diff --git a/src/material-experimental/mdc-core/BUILD.bazel b/src/material-experimental/mdc-core/BUILD.bazel
@@ -43,7 +43,6 @@ sass_binary(
         "external/npm/node_modules",
     ],
     deps = [
-        "//src/cdk:sass_lib",
         "//src/material:sass_lib",
         "//src/material-experimental/mdc-helpers:mdc_helpers_scss_lib",
         "//src/material-experimental/mdc-helpers:mdc_scss_deps_lib",
diff --git a/src/material-experimental/mdc-helpers/_focus-indicators-theme.scss b/src/material-experimental/mdc-helpers/_focus-indicators-theme.scss
@@ -2,28 +2,30 @@
 @use 'sass:map';
 @use 'sass:meta';
 
+$_prefix: 'mat-mdc';
+
 // stylelint-disable-next-line material/theme-mixin-api
 @mixin color($config-or-theme-or-color) {
   @if meta.type-of($config-or-theme-or-color) == 'color' {
-    @include mat.focus-indicators-private-private-customize-focus-indicators((
+    @include mat.private-strong-focus-indicators-customize-focus-indicators((
       border-color: $config-or-theme-or-color
-    ), 'mat-mdc');
+    ), $_prefix);
   }
   @else {
     $config: mat.get-color-config($config-or-theme-or-color);
     $border-color: mat.get-color-from-palette(map.get($config, primary));
-    @include mat.focus-indicators-private-private-customize-focus-indicators((
+    @include mat.private-strong-focus-indicators-customize-focus-indicators((
       border-color: $border-color
-    ), 'mat-mdc');
+    ), $_prefix);
   }
 }
 
 // stylelint-disable-next-line material/theme-mixin-api
 @mixin theme($theme-or-color-config-or-color) {
   @if meta.type-of($theme-or-color-config-or-color) == 'color' {
-    @include mat.focus-indicators-private-private-customize-focus-indicators((
+    @include mat.private-strong-focus-indicators-customize-focus-indicators((
       border-color: $theme-or-color-config-or-color
-    ), 'mat-mdc');
+    ), $_prefix);
   }
   @else {
     $theme: mat.private-legacy-get-theme($theme-or-color-config-or-color);
diff --git a/src/material-experimental/mdc-helpers/_focus-indicators.scss b/src/material-experimental/mdc-helpers/_focus-indicators.scss
@@ -11,5 +11,5 @@
   // Merge default config with user config.
   $config: map.merge($default-config, $config);
 
-  @include mat.focus-indicators-private-private-customize-focus-indicators($config, 'mat-mdc');
+  @include mat.private-strong-focus-indicators-customize-focus-indicators($config, 'mat-mdc');
 }
diff --git a/src/material-experimental/mdc-list/list.scss b/src/material-experimental/mdc-list/list.scss
@@ -59,7 +59,7 @@
 // focus/selected/hover state. Hence, we need to have a separate list-item spanning
 // element that can be used for strong focus indicators.
 .mat-mdc-list-item {
-  & > .mat-mdc-focus-indicator {
+  > .mat-mdc-focus-indicator {
     @include mat.private-fill();
     pointer-events: none;
   }
diff --git a/src/material-experimental/mdc-radio/BUILD.bazel b/src/material-experimental/mdc-radio/BUILD.bazel
@@ -46,7 +46,6 @@ sass_binary(
         "external/npm/node_modules",
     ],
     deps = [
-        "//src/cdk:sass_lib",
         "//src/material:sass_lib",
         "//src/material-experimental/mdc-helpers:mdc_helpers_scss_lib",
         "//src/material-experimental/mdc-helpers:mdc_scss_deps_lib",
diff --git a/src/material-experimental/mdc-radio/radio.scss b/src/material-experimental/mdc-radio/radio.scss
@@ -70,8 +70,6 @@
       opacity: mdc-radio-theme.$ripple-opacity;
     }
 
-    // Radio components have to set `border-radius: 50%` in order to support density scaling
-    // which will clip a square focus indicator so we have to turn it into a circle.
     &::before {
       border-radius: 50%;
     }
diff --git a/src/material-experimental/mdc-slide-toggle/BUILD.bazel b/src/material-experimental/mdc-slide-toggle/BUILD.bazel
@@ -50,7 +50,6 @@ sass_binary(
         "external/npm/node_modules",
     ],
     deps = [
-        "//src/cdk:sass_lib",
         "//src/material:sass_lib",
         "//src/material-experimental/mdc-helpers:mdc_helpers_scss_lib",
         "//src/material-experimental/mdc-helpers:mdc_scss_deps_lib",
diff --git a/src/material-experimental/mdc-tabs/BUILD.bazel b/src/material-experimental/mdc-tabs/BUILD.bazel
@@ -46,7 +46,6 @@ sass_library(
     name = "mdc_tabs_scss_lib",
     srcs = glob(["**/_*.scss"]),
     deps = [
-        "//src/cdk:sass_lib",
         "//src/material:sass_lib",
         "//src/material-experimental/mdc-helpers:mdc_helpers_scss_lib",
         "//src/material-experimental/mdc-helpers:mdc_scss_deps_lib",
diff --git a/src/material/_index.scss b/src/material/_index.scss
@@ -32,12 +32,12 @@
 @forward './table/table-flex-styles' show private-table-flex-styles;
 @forward './core/style/menu-common' as private-menu-common-*;
 @forward './core/style/button-common' as private-button-common-*;
-@forward './core/style/focus-indicators' as focus-indicators-private-*;
 
 // Structural
 @forward './core/core' show core;
 @forward './core/ripple/ripple' show ripple;
 @forward './core/focus-indicators/focus-indicators' show strong-focus-indicators;
+@forward './core/focus-indicators/private' as private-strong-focus-indicators-*;
 @forward './core/style/elevation' show elevation, overridable-elevation, elevation-transition;
 
 // Theme bundles
diff --git a/src/material/button/button.scss b/src/material/button/button.scss
@@ -2,7 +2,7 @@
 
 @use './button-base';
 @use '../core/style/layout-common';
-@use '../core/style/focus-indicators';
+@use '../core/focus-indicators/private';
 
 // TODO(jelbourn): Measure perf benefits for translate3d and will-change.
 // TODO(jelbourn): Figure out if anchor hover underline actually happens in any browser.
@@ -163,16 +163,14 @@
 .mat-fab,
 .mat-mini-fab {
   &::before {
-    $border-width:
-      var(--mat-focus-indicator-border-width, #{focus-indicators.$default-border-width});
+    $border-width: var(--mat-focus-indicator-border-width, #{private.$default-border-width});
     $offset: calc(#{$border-width} + 2px);
     margin: calc(#{$offset} * -1);
   }
 }
 
 .mat-stroked-button::before {
-  $border-width:
-    var(--mat-focus-indicator-border-width, #{focus-indicators.$default-border-width});
+  $border-width: var(--mat-focus-indicator-border-width, #{private.$default-border-width});
   $offset: calc(#{$border-width} + 3px);
   margin: calc(#{$offset} * -1);
 }
diff --git a/src/material/chips/chips.scss b/src/material/chips/chips.scss
@@ -3,8 +3,8 @@
 @use '../core/style/variables';
 @use '../core/style/elevation';
 @use '../core/style/layout-common';
-@use '../core/style/private';
-@use '../core/style/focus-indicators';
+@use '../core/style/private' as style-private;
+@use '../core/focus-indicators/private' as focus-indicators-private;
 
 $chip-min-height: 32px;
 $chip-vertical-padding: 7px;
@@ -41,15 +41,15 @@ $chip-remove-size: 18px;
   // the focus indicator is sufficiently contrastive and renders appropriately.
   &::before {
     $border-width:
-      var(--mat-focus-indicator-border-width, #{focus-indicators.$default-border-width});
+      var(--mat-focus-indicator-border-width, #{focus-indicators-private.$default-border-width});
     $offset: calc(#{$border-width} + 2px);
     margin: calc(#{$offset} * -1);
   }
 }
 
 .mat-standard-chip {
   @include elevation.elevation-transition;
-  @include private.private-animation-noop;
+  @include style-private.private-animation-noop;
   display: inline-flex;
   padding: $chip-vertical-padding $chip-horizontal-padding;
   border-radius: 16px;
diff --git a/src/material/core/_core.scss b/src/material/core/_core.scss
@@ -2,7 +2,7 @@
 
 // Core styles that can be used to apply material design treatments to any element.
 @use './ripple/ripple';
-@use './style/focus-indicators';
+@use './focus-indicators/private';
 @use './typography/all-typography';
 
 // Mixin that renders all of the core styles that are not theme-dependent.
@@ -13,6 +13,6 @@
   @include cdk.overlay();
   @include cdk.text-field-autosize();
   @include cdk.text-field-autofill();
-  @include focus-indicators.private-structural-styling('mat');
-  @include focus-indicators.private-structural-styling('mat-mdc');
+  @include private.structural-styling('mat');
+  @include private.structural-styling('mat-mdc');
 }
diff --git a/src/material/core/focus-indicators/_focus-indicators-theme.scss b/src/material/core/focus-indicators/_focus-indicators-theme.scss
@@ -1,30 +1,32 @@
 @use 'sass:map';
 @use 'sass:meta';
 @use '../theming/theming';
-@use '../style/focus-indicators';
+@use './private';
+
+$_prefix: 'mat';
 
 // stylelint-disable-next-line material/theme-mixin-api
 @mixin color($config-or-theme-or-color) {
   @if meta.type-of($config-or-theme-or-color) == 'color' {
-    @include focus-indicators.private-customize-focus-indicators((
+    @include private.customize-focus-indicators((
       border-color: $config-or-theme-or-color
-    ), 'mat');
+    ), $_prefix);
   }
   @else {
     $config: theming.get-color-config($config-or-theme-or-color);
     $border-color: theming.get-color-from-palette(map.get($config, primary));
-    @include focus-indicators.private-customize-focus-indicators((
+    @include private.customize-focus-indicators((
       border-color: $border-color
-    ), 'mat');
+    ), $_prefix);
   }
 }
 
 // stylelint-disable-next-line material/theme-mixin-api
 @mixin theme($theme-or-color-config-or-color) {
   @if meta.type-of($theme-or-color-config-or-color) == 'color' {
-    @include focus-indicators.private-customize-focus-indicators((
+    @include private.customize-focus-indicators((
       border-color: $theme-or-color-config-or-color
-    ), 'mat');
+    ), $_prefix);
   }
   @else {
     $theme: theming.private-legacy-get-theme($theme-or-color-config-or-color);
diff --git a/src/material/core/focus-indicators/_focus-indicators.scss b/src/material/core/focus-indicators/_focus-indicators.scss
@@ -1,5 +1,5 @@
 @use 'sass:map';
-@use '../style/focus-indicators';
+@use './private';
 
 @mixin strong-focus-indicators($config: ()) {
   // Default focus indicator config.
@@ -11,5 +11,5 @@
   // Merge default config with user config.
   $config: map.merge($default-config, $config);
 
-  @include focus-indicators.private-customize-focus-indicators($config, 'mat');
+  @include private.customize-focus-indicators($config, 'mat');
 }
diff --git a/src/material/core/focus-indicators/_private.scss b/src/material/core/focus-indicators/_private.scss
@@ -9,7 +9,7 @@ $default-border-color: transparent;
 $default-border-radius: 4px;
 
 // Mixin that renders the focus indicator structural styles.
-@mixin private-structural-styling($prefix) {
+@mixin structural-styling($prefix) {
   .#{$prefix}-focus-indicator {
     position: relative;
 
@@ -45,7 +45,7 @@ $default-border-radius: 4px;
 
   // Enable the indicator in high contrast mode.
   @include cdk.high-contrast(active, off) {
-    @include private-customize-focus-indicators((display: block), $prefix);
+    @include customize-focus-indicators((display: block), $prefix);
   }
 }
 
@@ -59,7 +59,7 @@ $default-border-radius: 4px;
 }
 
 // Mixin that dedups CSS variables for the strong-focus-indicators mixin.
-@mixin private-customize-focus-indicators($config, $prefix) {
+@mixin customize-focus-indicators($config, $prefix) {
   $border-style: map.get($config, border-style);
   $border-width: map.get($config, border-width);
   $border-radius: map.get($config, border-radius);
diff --git a/src/material/core/theming/tests/test-theming-bundle.scss b/src/material/core/theming/tests/test-theming-bundle.scss
@@ -24,3 +24,5 @@ $theme: mat.define-light-theme((
 @include mat.all-component-colors($theme);
 @include mat.core-theme($theme);
 @include mat.button-theme($theme);
+@include mat.strong-focus-indicators();
+@include mat.strong-focus-indicators-theme($theme);
diff --git a/src/material/datepicker/calendar.scss b/src/material/datepicker/calendar.scss
@@ -1,7 +1,7 @@
 @use '@angular/cdk';
 
 @use '../core/style/layout-common';
-@use '../core/style/focus-indicators';
+@use '../core/focus-indicators/private';
 
 $calendar-padding: 8px !default;
 $calendar-header-divider-width: 1px !default;
@@ -129,8 +129,7 @@ $calendar-next-icon-transform: translateX(-2px) rotate(45deg);
 // For the calendar element, default inset/offset values are necessary to ensure that
 // the focus indicator is sufficiently contrastive and renders appropriately.
 .mat-calendar-body-cell-content::before {
-  $border-width:
-    var(--mat-focus-indicator-border-width, #{focus-indicators.$default-border-width});
+  $border-width: var(--mat-focus-indicator-border-width, #{private.$default-border-width});
   $offset: calc(#{$border-width} + 3px);
   margin: calc(#{$offset} * -1);
 }
diff --git a/src/material/expansion/BUILD.bazel b/src/material/expansion/BUILD.bazel
@@ -48,7 +48,6 @@ sass_binary(
     name = "expansion_panel_scss",
     src = "expansion-panel.scss",
     deps = [
-        "//src/cdk:sass_lib",
         "//src/material/core:core_scss_lib",
     ],
 )
diff --git a/src/material/sort/sort-header.scss b/src/material/sort/sort-header.scss

Original file line number	Diff line number	Diff line change
`@@ -67,7 +67,7 @@`
`67`	`67`	`}`
`68`	`68`
`69`	`69`	`.mat-mdc-focus-indicator::before {`
`70`		`- $default-border-width: mat.$focus-indicators-private-default-border-width;`
	`70`	`+ $default-border-width: mat.$private-strong-focus-indicators-default-border-width;`
`71`	`71`	`$border-width: var(--mat-mdc-focus-indicator-border-width, #{$default-border-width});`
`72`	`72`	`$offset: calc(#{$border-width} + 2px);`
`73`	`73`	`margin: calc(#{$offset} * -1);`
Original file line number	Diff line number	Diff line change
`@@ -179,7 +179,7 @@`
`179`	`179`	`// For the chip element, default inset/offset values are necessary to ensure that`
`180`	`180`	`// the focus indicator is sufficiently contrastive and renders appropriately.`
`181`	`181`	`.mat-mdc-focus-indicator::before {`
`182`		`- $default-border-width: mat.$focus-indicators-private-default-border-width;`
	`182`	`+ $default-border-width: mat.$private-strong-focus-indicators-default-border-width;`
`183`	`183`	`$border-width: var(--mat-mdc-focus-indicator-border-width, #{$default-border-width});`
`184`	`184`	`$offset: calc(#{$border-width} + 2px);`
`185`	`185`	`margin: calc(#{$offset} * -1);`
`@@ -196,7 +196,7 @@`
`196`	`196`	`}`
`197`	`197`
`198`	`198`	`&::before {`
`199`		`- $default-border-width: mat.$focus-indicators-private-default-border-width;`
	`199`	`+ $default-border-width: mat.$private-strong-focus-indicators-default-border-width;`
`200`	`200`	`$offset: var(--mat-mdc-focus-indicator-border-width, #{$default-border-width});`
`201`	`201`	`margin: calc(#{$offset} * -1);`
`202`	`202`
`@@ -228,7 +228,8 @@`
`228`	`228`	`}`
`229`	`229`	`}`
`230`	`230`
`231`		`-// In the chips the individual actions have focus so we target a different element.`
	`231`	`+// The chip has multiple focus targets so we have to put the indicator on`
	`232`	`+// a separate element, rather than on the focusable element itself.`
`232`	`233`	`.mat-mdc-chip-action:focus .mat-mdc-focus-indicator::before {`
`233`	`234`	`content: '';`
`234`	`235`	`}`
Original file line number	Diff line number	Diff line change
`@@ -11,5 +11,5 @@`
`11`	`11`	`// Merge default config with user config.`
`12`	`12`	`$config: map.merge($default-config, $config);`
`13`	`13`
`14`		`- @include mat.focus-indicators-private-private-customize-focus-indicators($config, 'mat-mdc');`
	`14`	`+ @include mat.private-strong-focus-indicators-customize-focus-indicators($config, 'mat-mdc');`
`15`	`15`	`}`
Original file line number	Diff line number	Diff line change
`@@ -59,7 +59,7 @@`
`59`	`59`	`// focus/selected/hover state. Hence, we need to have a separate list-item spanning`
`60`	`60`	`// element that can be used for strong focus indicators.`
`61`	`61`	`.mat-mdc-list-item {`
`62`		`- & > .mat-mdc-focus-indicator {`
	`62`	`+ > .mat-mdc-focus-indicator {`
`63`	`63`	`@include mat.private-fill();`
`64`	`64`	`pointer-events: none;`
`65`	`65`	`}`