Skip to content

bug(focus indicators): Focus indicators are now positioned. #19345

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.

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 2 commits into from
Jun 5, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions src/material-experimental/mdc-button/_button-base.scss
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@
.mdc-button__label {
z-index: 1;
}

// The focus indicator should match the bounds of the entire button.
.mat-mdc-focus-indicator {
@include mat-fill();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Before this change, this element had essentially no styles, and thus the focus indicator actually rendered relative to the parent button. Now that we've added position: relative to this element, the fact that it has no styles is problematic (the focus indicator doesn't render with the correct bounding box). Consequently, use mat-fill to make this element match the bounds of the entire button.

}
}

// MDC's disabled buttons define a default cursor with pointer-events none. However, they select
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
@import '../../material/core/style/layout-common';
@import '../../material/core/focus-indicators/focus-indicators';


/// Mixin that turns on strong focus indicators.
///
/// @example
Expand Down
18 changes: 18 additions & 0 deletions src/material/core/_core.scss
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
// Core styles that can be used to apply material design treatments to any element.
@import './style/elevation';
@import './ripple/ripple';
@import './focus-indicators/focus-indicators';
@import './option/option-theme';
@import './option/optgroup-theme';
@import './selection/pseudo-checkbox/pseudo-checkbox-theme';
Expand All @@ -17,6 +18,9 @@
@include cdk-a11y();
@include cdk-overlay();
@include cdk-text-field();

@include _mat-strong-focus-indicators-positioning();
@include _mat-mdc-core();
}

@mixin mat-core-color($config-or-theme) {
Expand Down Expand Up @@ -67,3 +71,17 @@
}
}
}

// Mixin that renders all of the core MDC styles. Private mixin included with `mat-core`.
@mixin _mat-mdc-core() {
@include _mat-mdc-strong-focus-indicators-positioning();
}

// Mixin that ensures focus indicator host elements are positioned so that the focus indicator
// pseudo element within is positioned relative to the host. Private mixin included within
// `_mat-mdc-core`.
@mixin _mat-mdc-strong-focus-indicators-positioning() {
.mat-mdc-focus-indicator {
position: relative;
}
}
21 changes: 14 additions & 7 deletions src/material/core/focus-indicators/_focus-indicators.scss
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,11 @@
margin: -($border-width + 2px);
}

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

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

.mat-focus-indicator.mat-tab-link::before,
.mat-focus-indicator.mat-tab-label::before {
margin: 5px;
Expand All @@ -71,8 +68,9 @@
// is present.
.mat-focus-indicator.mat-option.mat-active::before,

// The focus indicator in the calendar is placed on an element inside the cell.
.mat-calendar-body-active .mat-focus-indicator::before,
// For calendar cells, render the focus indicator when the parent cell is
// focused.
.mat-calendar-body-cell:focus .mat-focus-indicator::before,

// For all other components, render the focus indicator on focus.
.mat-focus-indicator:focus::before {
Expand Down Expand Up @@ -122,3 +120,12 @@
}
}
}

// Mixin that ensures focus indicator host elements are positioned so that the focus indicator
// pseudo element within is positioned relative to the host. Private mixin included within
// `mat-core`.
@mixin _mat-strong-focus-indicators-positioning() {
.mat-focus-indicator {
position: relative;
}
}
7 changes: 6 additions & 1 deletion src/material/datepicker/calendar-body.scss
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,6 @@ $mat-calendar-range-end-body-cell-size:
}

.mat-calendar-body-cell-content {
position: absolute;
top: $mat-calendar-body-cell-content-margin;
left: $mat-calendar-body-cell-content-margin;
z-index: 1;
Expand All @@ -175,6 +174,12 @@ $mat-calendar-range-end-body-cell-size:
// Choosing a value clearly larger than the height ensures we get the correct capsule shape.
border-radius: $mat-calendar-body-cell-radius;

// Increase specificity because focus indicator styles are part of the `mat-core` mixin and can
// potentially overwrite the absolute position of the container.
&.mat-focus-indicator {
position: absolute;
}

@include cdk-high-contrast(active, off) {
border: none;
}
Expand Down
1 change: 0 additions & 1 deletion src/material/expansion/expansion-panel-header.scss
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
align-items: center;
padding: 0 24px;
border-radius: inherit;
position: relative; // Necessary for the strong focus indication.
transition: height $mat-expansion-panel-header-transition;

&:focus,
Expand Down
3 changes: 2 additions & 1 deletion src/material/expansion/expansion-panel.scss
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@
transition: margin 225ms $mat-fast-out-slow-in-timing-function,
mat-elevation-transition-property-value();

// Required so that the `box-shadow` works after we add `position: relative` to the header.
// Required so that the `box-shadow` works after the focus indicator relatively positions the
// header.
position: relative;

.mat-accordion & {
Expand Down
3 changes: 0 additions & 3 deletions src/material/list/list.scss
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,6 @@ $mat-list-item-inset-divider-offset: 72px;
width: 100%;
padding: 0;

// Required for focus indicator.
position: relative;

.mat-list-item-content {
display: flex;
flex-direction: row;
Expand Down
3 changes: 0 additions & 3 deletions src/material/sort/sort-header.scss
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,6 @@ $mat-sort-header-arrow-hint-opacity: 0.38;
font: inherit;
color: currentColor;

// Required for focus indicator.
position: relative;

// Usually we could rely on the arrow showing up to be focus indication, but if a header is
// active, the arrow will always be shown so the user has no way of telling the difference.
[mat-sort-header].cdk-keyboard-focused &,
Expand Down