Skip to content

Commit

Permalink
fix(material-experimental/mdc-list): make internal input non-interact…
Browse files Browse the repository at this point in the history
…ive (#21438)

Due to how MDC's checkbox styles are set up, we always have to keep an `input` inside
the `mat-list-option`, however our current setup where we only set `tabindex="-1"` on it
is invalid, because the input is still interactive either through clicking on it, or by using
a screen reader's forms mode.

These changes completely remove the input from the accessibility model by setting it
to `display: none`.

(cherry picked from commit c268246)
  • Loading branch information
crisbeto authored and annieyw committed Jan 9, 2021
1 parent bc5c3a7 commit 22bd693
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 15 deletions.
12 changes: 3 additions & 9 deletions src/material-experimental/mdc-list/list-option.html
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,8 @@

<ng-template #checkbox>
<div class="mdc-checkbox" [class.mdc-checkbox--disabled]="disabled">
<!--
Note: We stop propagation of the change event for the indicator checkbox so that
no accidental change event leaks out of the list option or selection list when
the checkbox is directly clicked.
-->
<input type="checkbox" tabindex="-1" class="mdc-checkbox__native-control"
[checked]="selected" [disabled]="disabled" [attr.aria-describedby]="_optionTextId"
(change)="$event.stopPropagation()" />
<input type="checkbox" class="mdc-checkbox__native-control"
[checked]="selected" [disabled]="disabled"/>
<div class="mdc-checkbox__background">
<svg class="mdc-checkbox__checkmark"
viewBox="0 0 24 24">
Expand All @@ -37,7 +31,7 @@
</span>

<!-- Text -->
<span class="mdc-list-item__text" #text [id]="_optionTextId">
<span class="mdc-list-item__text" #text>
<ng-content></ng-content>
</span>

Expand Down
10 changes: 10 additions & 0 deletions src/material-experimental/mdc-list/list-option.scss
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,13 @@
// The MDC-based list-option uses the MDC checkbox for the selection indicators.
// We need to ensure that the checkbox styles are included for the list-option.
@include mdc-checkbox-without-ripple($query: $mat-base-styles-query);

// The internal checkbox is purely decorative, but because it's an `input`, the user can still
// focus it by tabbing or clicking. Furthermore, `mat-list-option` has the `option` role which
// doesn't allow a nested `input`. We use `display: none` both to remove it from the tab order
// and to prevent focus from reaching it through the screen reader's forms mode. Ideally we'd
// remove the `input` completely, but we can't because MDC uses a `:checked` selector to
// toggle the selected styles.
.mat-mdc-list-option .mdc-checkbox__native-control {
display: none;
}
6 changes: 0 additions & 6 deletions src/material-experimental/mdc-list/list-option.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,6 @@ export interface SelectionList extends MatListBase {
_onTouched: () => void;
}

/** Unique id for created list options. */
let uniqueId = 0;

@Component({
selector: 'mat-list-option',
exportAs: 'matListOption',
Expand Down Expand Up @@ -86,9 +83,6 @@ export class MatListOption extends MatListItemBase implements OnInit, OnDestroy
@ContentChildren(MatLine, {read: ElementRef, descendants: true}) lines:
QueryList<ElementRef<Element>>;

/** Unique id for the text. Used for describing the underlying checkbox input. */
_optionTextId: string = `mat-mdc-list-option-text-${uniqueId++}`;

/** Whether the label should appear before or after the checkbox. Defaults to 'after' */
@Input() checkboxPosition: 'before' | 'after' = 'after';

Expand Down

0 comments on commit 22bd693

Please sign in to comment.