mdc-list: support for strong focus indicators and fix incorrect cursor for non-interactive items (#20476)

* feat(material-experimental/mdc-list): add support for strong focus indicators

Adds support for strong focus indicators to the MDC-based list.
Also simplifies SCSS code for the hover/focus/selected styles.

* fix(material-experimental/mdc-list): do not show pointer cursor for non-interactive list items

The MDC list currently always shows the pointer cursor for all kinds
of list items, regardless of them being interactive or not (through
instantiation of the foundation).

This seems to be unexpected as the pointer cursor incorrectly suggests
interactivity of a list item to users. We manually reset the cursor
for non-interactive list items and report an issue upstream:

material-components/material-components-web#6443

Note: MDC list does no longer seem to support the `non-interactive`
class we previously used, so we replace it with our own.

Author: devversion

src/material-experimental/mdc-helpers/_focus-indicators.scss

@@ -73,8 +73,10 @@
   .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.
+  // For buttons and list items, render the focus indicator when the parent
+  // button or list item is focused.
   .mat-mdc-button-base:focus .mat-mdc-focus-indicator::before,
+  .mat-mdc-list-item:focus > .mat-mdc-focus-indicator::before,
 
   // For options, render the focus indicator when the class .mat-mdc-option-active is present.
   .mat-mdc-focus-indicator.mat-mdc-option-active::before,

src/material-experimental/mdc-list/list-base.ts

@@ -80,11 +80,6 @@ export abstract class MatListItemBase implements AfterContentInit, OnDestroy, Ri
       this._initInteractiveListItem();
     }
 
-    // Only interactive list items are commonly focusable, but in some situations,
-    // consumers provide a custom tabindex. We still would want to have strong focus
-    // indicator support in such scenarios.
-    this._hostElement.classList.add('mat-mdc-focus-indicator');
-
     // If no type attribute is specified for a host `<button>` element, set it to `button`. If a
     // type attribute is already specified, we do nothing. We do this for backwards compatibility.
     // TODO: Determine if we intend to continue doing this for the MDC-based list.
@@ -143,7 +138,7 @@ export abstract class MatListItemBase implements AfterContentInit, OnDestroy, Ri
 @Directive()
 /** @docs-private */
 export abstract class MatListBase {
-  @HostBinding('class.mdc-list--non-interactive')
+  @HostBinding('class.mat-mdc-list-non-interactive')
   _isNonInteractive: boolean = true;
 
   /** Whether ripples for all list items is disabled. */

src/material-experimental/mdc-list/list-item.html

@@ -15,3 +15,9 @@
 </span>
 
 <ng-content select="mat-divider"></ng-content>
+
+<!--
+  Strong focus indicator element. MDC uses the `::before` pseudo element for the default
+  focus/hover/selected state, so we need a separate element.
+-->
+<div class="mat-mdc-focus-indicator"></div>

src/material-experimental/mdc-list/list-option.html

@@ -50,3 +50,9 @@
 
 <!-- Divider -->
 <ng-content select="mat-divider"></ng-content>
+
+<!--
+  Strong focus indicator element. MDC uses the `::before` pseudo element for the default
+  focus/hover/selected state, so we need a separate element.
+-->
+<div class="mat-mdc-focus-indicator"></div>

src/material-experimental/mdc-list/list.scss

@@ -97,14 +97,24 @@
   }
 }
 
-// MDC's state styles are included with their ripple which we don't use. Instead we add
-// the focus and hover styles ourselves using this pseudo-element
+// MDC's hover and focus state styles are included with their ripple which we don't use.
+// Instead we add the focus, hover and selected styles ourselves using this pseudo-element
 .mat-mdc-list-item-interactive::before {
+  @include mat-fill();
   content: '';
-  position: absolute;
-  top: 0;
-  left: 0;
-  bottom: 0;
-  right: 0;
   opacity: 0;
 }
+
+// MDC always sets the cursor to `pointer`. We do not want to show this for non-interactive
+// lists. See: https://github.com/material-components/material-components-web/issues/6443
+.mat-mdc-list-non-interactive .mdc-list-item {
+  cursor: default;
+}
+
+// The MDC-based list items already use the `::before` pseudo element for the standard
+// 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 {
+  @include mat-fill();
+  pointer-events: none;
+}

src/material-experimental/mdc-list/list.spec.ts

@@ -23,17 +23,14 @@ describe('MDC-based MatList', () => {
     TestBed.compileComponents();
   }));
 
-  it('should apply an additional class lists without lines', () => {
+  it('should apply an additional class to lists without lines', () => {
     const fixture = TestBed.createComponent(ListWithOneItem);
     const listItem = fixture.debugElement.query(By.css('mat-list-item'))!;
     fixture.detectChanges();
-    expect(listItem.nativeElement.classList.length).toBe(4);
+    expect(listItem.nativeElement.classList.length).toBe(3);
     expect(listItem.nativeElement.classList).toContain('mat-mdc-list-item');
     expect(listItem.nativeElement.classList).toContain('mdc-list-item');
     expect(listItem.nativeElement.classList).toContain('mat-mdc-list-item-single-line');
-
-    // This spec also ensures the focus indicator is present.
-    expect(listItem.nativeElement.classList).toContain('mat-mdc-focus-indicator');
   });
 
   it('should apply mat-mdc-2-line class to lists with two lines', () => {
@@ -63,6 +60,16 @@ describe('MDC-based MatList', () => {
     expect(listItems[1].nativeElement.className).toContain('mat-mdc-multi-line');
   });
 
+  it('should have a strong focus indicator configured for all list-items', () => {
+    const fixture = TestBed.createComponent(ListWithManyLines);
+    fixture.detectChanges();
+    const listItems = fixture.debugElement.children[0].queryAll(By.css('mat-list-item'))
+      .map(debugEl => debugEl.nativeElement as HTMLElement);
+
+    expect(listItems.every(i => i.querySelector('.mat-mdc-focus-indicator') !== null))
+      .toBe(true, 'Expected all list items to have a strong focus indicator element.');
+  });
+
   it('should not clear custom classes provided by user', () => {
     const fixture = TestBed.createComponent(ListWithItemWithCssClass);
     fixture.detectChanges();
@@ -77,11 +84,10 @@ describe('MDC-based MatList', () => {
     fixture.detectChanges();
 
     const listItem = fixture.debugElement.children[0].query(By.css('mat-list-item'))!;
-    expect(listItem.nativeElement.classList.length).toBe(4);
+    expect(listItem.nativeElement.classList.length).toBe(3);
     expect(listItem.nativeElement.classList).toContain('mat-mdc-2-line');
     expect(listItem.nativeElement.classList).toContain('mat-mdc-list-item');
     expect(listItem.nativeElement.classList).toContain('mdc-list-item');
-    expect(listItem.nativeElement.classList).toContain('mat-mdc-focus-indicator');
 
     fixture.debugElement.componentInstance.showThirdLine = true;
     fixture.detectChanges();

src/material-experimental/mdc-list/selection-list.spec.ts

@@ -735,10 +735,11 @@ describe('MDC-based MatSelectionList without forms', () => {
     });
 
     it('should have a focus indicator', () => {
-      const optionNativeElements = listOptions.map(option => option.nativeElement);
+      const optionNativeElements = listOptions.map(option =>
+        option.nativeElement as HTMLElement);
 
       expect(optionNativeElements
-        .every(element => element.classList.contains('mat-mdc-focus-indicator'))).toBe(true);
+        .every(element => element.querySelector('.mat-mdc-focus-indicator') !== null)).toBe(true);
     });
 
   });