Skip to content

feat(focus indicators): Add config map to base focus indicators mixin and tweak some default styles. #19206

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 1, 2020
Merged

feat(focus indicators): Add config map to base focus indicators mixin and tweak some default styles. #19206

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0ce88fa
feat(focus indicators): Add config map to base focus indicators mixin…
zelliott Apr 29, 2020
13012cc
feat(focus indicators0: Delete unused file.
zelliott Apr 29, 2020
d87c7ce
feat(focus indicators): Updated CODEOWNERS as focus indicators filena…
zelliott Apr 29, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/CODEOWNERS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
/src/material/core/common-behaviors/** @jelbourn @devversion
/src/material/core/datetime/** @mmalerba
/src/material/core/error/** @crisbeto @mmalerba
/src/material/core/focus-indicator/** @jelbourn @zelliott
/src/material/core/focus-indicators/** @jelbourn @zelliott
/src/material/core/gestures/** @jelbourn
/src/material/core/label/** @mmalerba
/src/material/core/line/** @jelbourn
Expand Down
2 changes: 1 addition & 1 deletion src/dev-app/theme.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
@import '../material/core/theming/all-theme';
@import '../material/core/focus-indicator/focus-indicator';
@import '../material/core/focus-indicators/focus-indicators';
@import '../material-experimental/column-resize/column-resize';
@import '../material-experimental/mdc-helpers/mdc-helpers';
@import '../material-experimental/mdc-theming/all-theme';
Expand Down
54 changes: 37 additions & 17 deletions src/material-experimental/mdc-helpers/_mdc-helpers.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,37 +217,55 @@ $mat-typography-level-mappings: (
///
/// @example
/// .my-app {
/// @include mat-mdc-strong-focus-indicators();
/// @include mat-mdc-strong-focus-indicators($config);
/// }
@mixin mat-mdc-strong-focus-indicators() {
// Base styles for the focus indicators.
@mixin mat-mdc-strong-focus-indicators($config: ()) {
// Default focus indicator config.
$default-config: (
border-style: solid,
border-width: 3px,
border-radius: 4px,
);

// Merge default config with user config.
$config: map-merge($default-config, $config);
$border-style: map-get($config, border-style);
$border-width: map-get($config, border-width);
$border-radius: map-get($config, border-radius);

// Base styles for focus indicators.
.mat-mdc-focus-indicator::before {
@include mat-fill();

border-radius: $mat-focus-indicator-border-radius;
border: $mat-focus-indicator-border-width $mat-focus-indicator-border-style transparent;
box-sizing: border-box;
pointer-events: none;
border: $border-width $border-style transparent;
border-radius: $border-radius;
}

// By default, all focus indicators are flush with the bounding box of their
// host element. For particular elements (listed below), default inset/offset
// values are necessary to ensure that the focus indicator is sufficiently
// contrastive and renders appropriately.

.mat-mdc-button-base .mat-mdc-focus-indicator::before,
.mat-mdc-unelevated-button .mat-mdc-focus-indicator::before,
.mat-mdc-raised-button .mat-mdc-focus-indicator::before,
.mdc-fab .mat-mdc-focus-indicator::before,
.mat-mdc-focus-indicator.mdc-chip::before {
margin: $mat-focus-indicator-border-width * -2;
margin: -($border-width + 2px);
}

.mat-mdc-outlined-button .mat-mdc-focus-indicator::before {
margin: -($border-width + 3px);
}

.mat-mdc-focus-indicator.mat-mdc-chip-remove::before,
.mat-mdc-focus-indicator.mat-chip-row-focusable-text-content::before {
margin: $mat-focus-indicator-border-width * -1;
margin: -$border-width;
}

.mat-mdc-focus-indicator.mat-mdc-tab::before,
.mat-mdc-focus-indicator.mat-mdc-tab-link::before {
margin: $mat-focus-indicator-border-width * 2;
margin: 5px;
}

// Render the focus indicator on focus. Defining a pseudo element's
Expand All @@ -258,6 +276,8 @@ $mat-typography-level-mappings: (
.mdc-checkbox__native-control:focus ~ .mat-mdc-focus-indicator::before,
.mat-mdc-slide-toggle-focused .mat-mdc-focus-indicator::before,
.mat-mdc-radio-button.cdk-focused .mat-mdc-focus-indicator::before,

// For buttons, render the focus indicator when the parent button is focused.
.mat-mdc-button-base:focus .mat-mdc-focus-indicator::before,

// For all other components, render the focus indicator on focus.
Expand All @@ -268,24 +288,24 @@ $mat-typography-level-mappings: (

/// Mixin that sets the color of the focus indicators.
///
/// @param {color|map} $themeOrMap
/// @param {color|map} $theme-or-color
/// If theme, focus indicators are set to the primary color of the theme. If
/// color, focus indicators are set to that color.
///
/// @example
/// .demo-dark-theme {
/// @include mat-mdc-strong-focus-indicators-theme($darkThemeMap);
/// @include mat-mdc-strong-focus-indicators-theme($dark-theme-map);
/// }
///
/// @example
/// .demo-red-theme {
/// @include mat-mdc-strong-focus-indicators-theme(#F00);
/// @include mat-mdc-strong-focus-indicators-theme(#f00);
/// }
@mixin mat-mdc-strong-focus-indicators-theme($themeOrColor) {
@mixin mat-mdc-strong-focus-indicators-theme($theme-or-color) {
.mat-mdc-focus-indicator::before {
border-color: if(
type-of($themeOrColor) == 'map',
mat-color(map_get($themeOrColor, primary)),
$themeOrColor);
type-of($theme-or-color) == 'map',
mat-color(map_get($theme-or-color, primary)),
$theme-or-color);
}
}
58 changes: 35 additions & 23 deletions ...ore/focus-indicator/_focus-indicator.scss → ...e/focus-indicators/_focus-indicators.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,48 +1,60 @@
@import '../theming/theming';
@import '../style/layout-common';

// Focus indicator styles.
$mat-focus-indicator-border-radius: 4px;
$mat-focus-indicator-border-width: 2px;
$mat-focus-indicator-border-style: solid;

/// Mixin that turns on strong focus indicators.
///
/// @example
/// .my-app {
/// @include mat-strong-focus-indicators();
/// @include mat-strong-focus-indicators($config);
/// }
@mixin mat-strong-focus-indicators() {
@mixin mat-strong-focus-indicators($config: ()) {
// Default focus indicator config.
$default-config: (
border-style: solid,
border-width: 3px,
border-radius: 4px,
);

// Merge default config with user config.
$config: map-merge($default-config, $config);
$border-style: map-get($config, border-style);
$border-width: map-get($config, border-width);
$border-radius: map-get($config, border-radius);

// Base styles for the focus indicators.
// Base styles for focus indicators.
.mat-focus-indicator::before {
@include mat-fill();

border-radius: $mat-focus-indicator-border-radius;
border: $mat-focus-indicator-border-width $mat-focus-indicator-border-style transparent;
box-sizing: border-box;
pointer-events: none;
border: $border-width $border-style transparent;
border-radius: $border-radius;
}

// By default, all focus indicators are flush with the bounding box of their
// host element. For particular elements (listed below), default inset/offset
// values are necessary to ensure that the focus indicator is sufficiently
// contrastive and renders appropriately.

.mat-focus-indicator.mat-button-base::before,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the default inset/offset styles to (1) work nicely with the $border-width parameter and (2) more-accurately target the elements that actually need default inset/offset values. For example, previously all mat-button-base had an offset focus indicator, now only flat button, raised button, fab, and mini fab do. The rationale behind this is that offseting focus indicators is a bit dangerous from a design perspective, as it's easy for them to be clipped by adjacent elements or containers with overflow: hidden.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jelbourn & @mmalerba: If a developer wishes to add an offset to flat & raised buttons but not text buttons, how would we allow that with the per-component customization mixins? We were thinking there would be a mat-button-strong-focus-indicator mixin, but maybe we need to be a bit more specific and have mat-flat-button-strong-focus-indicator, mat-raised-button-strong-focus-indicator, etc. The button mixin could also accept a $type parameter. WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably separate mixins, to be consistent with what we've done so far. so... many... mixins...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: I was thinking that in a follow-up PR these default inset/offset styles would be replaced by the per-component customization mixins with the default inset/offset values.

.mat-focus-indicator.mat-card::before,
.mat-focus-indicator.mat-flat-button::before,
.mat-focus-indicator.mat-raised-button::before,
.mat-focus-indicator.mat-fab::before,
.mat-focus-indicator.mat-mini-fab::before,
.mat-focus-indicator.mat-chip::before,
.mat-focus-indicator.mat-sort-header-button::before {
margin: $mat-focus-indicator-border-width * -2;
margin: -($border-width + 2px);
}

.mat-focus-indicator.mat-stroked-button::before {
margin: -($border-width + 3px);
}

.mat-focus-indicator.mat-calendar-body-cell::before {
margin: $mat-focus-indicator-border-width * -1;
margin: -$border-width;
}

.mat-focus-indicator.mat-tab-link::before,
.mat-focus-indicator.mat-tab-label::before {
margin: $mat-focus-indicator-border-width * 2;
margin: 5px;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI: The reason why some of these. values are a function of $border-width and others are not is because the focus indicator grows inward as its width is increased.

}

// Render the focus indicator on focus. Defining a pseudo element's
Expand All @@ -66,24 +78,24 @@ $mat-focus-indicator-border-style: solid;

/// Mixin that sets the color of the focus indicators.
///
/// @param {color|map} $themeOrMap
/// @param {color|map} $theme-or-color
/// If theme, focus indicators are set to the primary color of the theme. If
/// color, focus indicators are set to that color.
///
/// @example
/// .demo-dark-theme {
/// @include mat-strong-focus-indicators-theme($darkThemeMap);
/// @include mat-strong-focus-indicators-theme($dark-theme-map);
/// }
///
/// @example
/// .demo-red-theme {
/// @include mat-strong-focus-indicators-theme(#F00);
/// @include mat-strong-focus-indicators-theme(#f00);
/// }
@mixin mat-strong-focus-indicators-theme($themeOrColor) {
@mixin mat-strong-focus-indicators-theme($theme-or-color) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jelbourn & @mmalerba: Do we still want to have a standalone mat-strong-focus-indicators-theme mixin? One option is to merge this into the mat-strong-focus-indicators mixin (i.e. add a border-color key into the $config), but then if developers want to toggle focus indicator styles on light/dark backgrounds, a bunch of redundant styles are re-declared. Thoughts?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO it makes sense to keep it, because the color depends on the theme.

.mat-focus-indicator::before {
border-color: if(
type-of($themeOrColor) == 'map',
mat-color(map_get($themeOrColor, primary)),
$themeOrColor);
type-of($theme-or-color) == 'map',
mat-color(map_get($theme-or-color, primary)),
$theme-or-color);
}
}