Skip to content

Commit

Permalink
refactor(material/core): clean up focus indicators
Browse files Browse the repository at this point in the history
I skipped over some of the feedback on angular#25039 in order to get the focus indicator changes merged in quicker. This is a follow-up to address the remaining feedback.
  • Loading branch information
crisbeto committed Jun 15, 2022
1 parent d755c66 commit 364e396
Show file tree
Hide file tree
Showing 24 changed files with 56 additions and 60 deletions.
15 changes: 8 additions & 7 deletions src/dev-app/theme.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/material-experimental/mdc-button/button.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/material-experimental/mdc-button/fab.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 0 additions & 1 deletion src/material-experimental/mdc-checkbox/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
7 changes: 4 additions & 3 deletions src/material-experimental/mdc-chips/chip.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);

Expand Down Expand Up @@ -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: '';
}
1 change: 0 additions & 1 deletion src/material-experimental/mdc-core/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}
2 changes: 1 addition & 1 deletion src/material-experimental/mdc-list/list.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
1 change: 0 additions & 1 deletion src/material-experimental/mdc-radio/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 0 additions & 2 deletions src/material-experimental/mdc-radio/radio.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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%;
}
Expand Down
1 change: 0 additions & 1 deletion src/material-experimental/mdc-slide-toggle/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
1 change: 0 additions & 1 deletion src/material-experimental/mdc-tabs/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion src/material/_index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 3 additions & 5 deletions src/material/button/button.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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);
}
Expand Down
8 changes: 4 additions & 4 deletions src/material/chips/chips.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
6 changes: 3 additions & 3 deletions src/material/core/_core.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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');
}
16 changes: 9 additions & 7 deletions src/material/core/focus-indicators/_focus-indicators-theme.scss
Original file line number Diff line number Diff line change
@@ -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);
Expand Down
4 changes: 2 additions & 2 deletions src/material/core/focus-indicators/_focus-indicators.scss
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
@use 'sass:map';
@use '../style/focus-indicators';
@use './private';

@mixin strong-focus-indicators($config: ()) {
// Default focus indicator config.
Expand All @@ -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');
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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);
}
}

Expand All @@ -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);
Expand Down
2 changes: 2 additions & 0 deletions src/material/core/theming/tests/test-theming-bundle.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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);
5 changes: 2 additions & 3 deletions src/material/datepicker/calendar.scss
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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);
}
Expand Down
1 change: 0 additions & 1 deletion src/material/expansion/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
],
)
Expand Down
Loading

0 comments on commit 364e396

Please sign in to comment.