From 610b3a31e27172f0857159e2ffccff6db4c4ad70 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Wed, 15 Jun 2022 10:21:54 +0200 Subject: [PATCH] refactor(material/core): clean up focus indicators 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. --- src/dev-app/theme.scss | 15 ++++++++------- src/material-experimental/mdc-button/button.scss | 4 ++-- src/material-experimental/mdc-button/fab.scss | 2 +- .../mdc-checkbox/BUILD.bazel | 1 - src/material-experimental/mdc-chips/chip.scss | 7 ++++--- src/material-experimental/mdc-core/BUILD.bazel | 1 - .../mdc-helpers/_focus-indicators-theme.scss | 14 ++++++++------ .../mdc-helpers/_focus-indicators.scss | 2 +- src/material-experimental/mdc-list/list.scss | 2 +- src/material-experimental/mdc-radio/BUILD.bazel | 1 - src/material-experimental/mdc-radio/radio.scss | 2 -- .../mdc-slide-toggle/BUILD.bazel | 1 - src/material-experimental/mdc-tabs/BUILD.bazel | 1 - src/material/_index.scss | 2 +- src/material/button/button.scss | 8 +++----- src/material/chips/chips.scss | 8 ++++---- src/material/core/_core.scss | 6 +++--- .../_focus-indicators-theme.scss | 16 +++++++++------- .../core/focus-indicators/_focus-indicators.scss | 4 ++-- .../_private.scss} | 6 +++--- src/material/datepicker/calendar.scss | 5 ++--- src/material/expansion/BUILD.bazel | 1 - src/material/sort/sort-header.scss | 5 ++--- 23 files changed, 54 insertions(+), 60 deletions(-) rename src/material/core/{style/_focus-indicators.scss => focus-indicators/_private.scss} (93%) diff --git a/src/dev-app/theme.scss b/src/dev-app/theme.scss index 1ab706e393c4..e948eef09d4d 100644 --- 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 index e8f2d0b0191c..18610d335eee 100644 --- a/src/material-experimental/mdc-button/button.scss +++ b/src/material-experimental/mdc-button/button.scss @@ -113,7 +113,7 @@ .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); @@ -121,7 +121,7 @@ } .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 index 52bd19ecb530..a0fcbc2c58d8 100644 --- 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 index 4f250ecce22b..1b75cf09fd74 100644 --- 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 index 1f69a6cfc0fe..0eadab02e6c2 100644 --- 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 index 24cd7a9cd74d..3e68616bcf8b 100644 --- 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 index 521d4471941a..0435598a7804 100644 --- 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 index 2ee84f5ca533..f9a40777d732 100644 --- 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 index f56d0650bf37..6d05348d2a54 100644 --- 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 index 2f476b97eed2..4eda49bada04 100644 --- 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 index 0daaa551ab7b..140c91efa0c3 100644 --- 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 index 9e57202edb7c..aae86a388f10 100644 --- 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 index 44e69b3551cf..310dbe44eefa 100644 --- 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 index 8219c737103f..1b40f7d7d817 100644 --- 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 index c7d1c9b8e809..c4b61d7b8749 100644 --- 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 index 09950f40e440..89266a948199 100644 --- 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,7 +41,7 @@ $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); } @@ -49,7 +49,7 @@ $chip-remove-size: 18px; .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 index f15f55e7c2ac..a77709e9ac8e 100644 --- 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 index 5c6b6fbd364c..78f894aed131 100644 --- 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 index ed2cb76ac3d5..3fbb011fcfb3 100644 --- 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/style/_focus-indicators.scss b/src/material/core/focus-indicators/_private.scss similarity index 93% rename from src/material/core/style/_focus-indicators.scss rename to src/material/core/focus-indicators/_private.scss index 8a9f875483e6..601fc90156b3 100644 --- a/src/material/core/style/_focus-indicators.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/datepicker/calendar.scss b/src/material/datepicker/calendar.scss index e4c3a4cf8575..dfc182b1a1e5 100644 --- 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 index d5dcd7781c86..43716c20f5eb 100644 --- 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 index 62ebae076cb0..ee772bb68223 100644 --- a/src/material/sort/sort-header.scss +++ b/src/material/sort/sort-header.scss @@ -1,6 +1,6 @@ @use '@angular/cdk'; -@use '../core/style/focus-indicators'; +@use '../core/focus-indicators/private'; $header-arrow-margin: 6px; $header-arrow-container-size: 12px; @@ -33,8 +33,7 @@ $header-arrow-hint-opacity: 0.38; // For the sort-header element, default inset/offset values are necessary to ensure that // the focus indicator is sufficiently contrastive and renders appropriately. &::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); }