fix(material-experimental/mdc-list): error with latest MDC list canary version (#22636)

MDC updated the focus strategy for lists with multi-selection enabled.
This breaks our tests as we relied on the old strategy where the
previously focused item always becomes the focus target.

This is no longer the case as MDC now intends to focus the first
selected item of a list instead (except for non-selection lists which
are interactive; here the previously focused item is still used).

We update the tests and update the selection list so that pre-selected
options are recognized correctly so that MDC's new focus strategy works
as expected. Before this change we didn't sync the pre-selected options
with the MDC list foundation.

Author: devversion

package.json

@@ -62,7 +62,7 @@
     "@types/googlemaps": "^3.43.1",
     "@types/youtube": "^0.0.42",
     "core-js-bundle": "^3.8.2",
-    "material-components-web": "12.0.0-canary.be999eb08.0",
+    "material-components-web": "12.0.0-canary.03f525f9f.0",
     "rxjs": "^6.5.3",
     "rxjs-tslint-rules": "^4.33.1",
     "systemjs": "0.19.43",
@@ -91,53 +91,53 @@
     "@bazel/terser": "3.3.0",
     "@bazel/typescript": "3.3.0",
     "@firebase/app-types": "^0.6.1",
-    "@material/animation": "12.0.0-canary.be999eb08.0",
-    "@material/auto-init": "12.0.0-canary.be999eb08.0",
-    "@material/banner": "12.0.0-canary.be999eb08.0",
-    "@material/base": "12.0.0-canary.be999eb08.0",
-    "@material/button": "12.0.0-canary.be999eb08.0",
-    "@material/card": "12.0.0-canary.be999eb08.0",
-    "@material/checkbox": "12.0.0-canary.be999eb08.0",
-    "@material/chips": "12.0.0-canary.be999eb08.0",
-    "@material/circular-progress": "12.0.0-canary.be999eb08.0",
-    "@material/data-table": "12.0.0-canary.be999eb08.0",
-    "@material/density": "12.0.0-canary.be999eb08.0",
-    "@material/dialog": "12.0.0-canary.be999eb08.0",
-    "@material/dom": "12.0.0-canary.be999eb08.0",
-    "@material/drawer": "12.0.0-canary.be999eb08.0",
-    "@material/elevation": "12.0.0-canary.be999eb08.0",
-    "@material/fab": "12.0.0-canary.be999eb08.0",
-    "@material/feature-targeting": "12.0.0-canary.be999eb08.0",
-    "@material/floating-label": "12.0.0-canary.be999eb08.0",
-    "@material/form-field": "12.0.0-canary.be999eb08.0",
-    "@material/icon-button": "12.0.0-canary.be999eb08.0",
-    "@material/image-list": "12.0.0-canary.be999eb08.0",
-    "@material/layout-grid": "12.0.0-canary.be999eb08.0",
-    "@material/line-ripple": "12.0.0-canary.be999eb08.0",
-    "@material/linear-progress": "12.0.0-canary.be999eb08.0",
-    "@material/list": "12.0.0-canary.be999eb08.0",
-    "@material/menu": "12.0.0-canary.be999eb08.0",
-    "@material/menu-surface": "12.0.0-canary.be999eb08.0",
-    "@material/notched-outline": "12.0.0-canary.be999eb08.0",
-    "@material/radio": "12.0.0-canary.be999eb08.0",
-    "@material/ripple": "12.0.0-canary.be999eb08.0",
-    "@material/rtl": "12.0.0-canary.be999eb08.0",
-    "@material/segmented-button": "12.0.0-canary.be999eb08.0",
-    "@material/select": "12.0.0-canary.be999eb08.0",
-    "@material/shape": "12.0.0-canary.be999eb08.0",
-    "@material/slider": "12.0.0-canary.be999eb08.0",
-    "@material/snackbar": "12.0.0-canary.be999eb08.0",
-    "@material/switch": "12.0.0-canary.be999eb08.0",
-    "@material/tab": "12.0.0-canary.be999eb08.0",
-    "@material/tab-bar": "12.0.0-canary.be999eb08.0",
-    "@material/tab-indicator": "12.0.0-canary.be999eb08.0",
-    "@material/tab-scroller": "12.0.0-canary.be999eb08.0",
-    "@material/textfield": "12.0.0-canary.be999eb08.0",
-    "@material/theme": "12.0.0-canary.be999eb08.0",
-    "@material/tooltip": "12.0.0-canary.be999eb08.0",
-    "@material/top-app-bar": "12.0.0-canary.be999eb08.0",
-    "@material/touch-target": "12.0.0-canary.be999eb08.0",
-    "@material/typography": "12.0.0-canary.be999eb08.0",
+    "@material/animation": "12.0.0-canary.03f525f9f.0",
+    "@material/auto-init": "12.0.0-canary.03f525f9f.0",
+    "@material/banner": "12.0.0-canary.03f525f9f.0",
+    "@material/base": "12.0.0-canary.03f525f9f.0",
+    "@material/button": "12.0.0-canary.03f525f9f.0",
+    "@material/card": "12.0.0-canary.03f525f9f.0",
+    "@material/checkbox": "12.0.0-canary.03f525f9f.0",
+    "@material/chips": "12.0.0-canary.03f525f9f.0",
+    "@material/circular-progress": "12.0.0-canary.03f525f9f.0",
+    "@material/data-table": "12.0.0-canary.03f525f9f.0",
+    "@material/density": "12.0.0-canary.03f525f9f.0",
+    "@material/dialog": "12.0.0-canary.03f525f9f.0",
+    "@material/dom": "12.0.0-canary.03f525f9f.0",
+    "@material/drawer": "12.0.0-canary.03f525f9f.0",
+    "@material/elevation": "12.0.0-canary.03f525f9f.0",
+    "@material/fab": "12.0.0-canary.03f525f9f.0",
+    "@material/feature-targeting": "12.0.0-canary.03f525f9f.0",
+    "@material/floating-label": "12.0.0-canary.03f525f9f.0",
+    "@material/form-field": "12.0.0-canary.03f525f9f.0",
+    "@material/icon-button": "12.0.0-canary.03f525f9f.0",
+    "@material/image-list": "12.0.0-canary.03f525f9f.0",
+    "@material/layout-grid": "12.0.0-canary.03f525f9f.0",
+    "@material/line-ripple": "12.0.0-canary.03f525f9f.0",
+    "@material/linear-progress": "12.0.0-canary.03f525f9f.0",
+    "@material/list": "12.0.0-canary.03f525f9f.0",
+    "@material/menu": "12.0.0-canary.03f525f9f.0",
+    "@material/menu-surface": "12.0.0-canary.03f525f9f.0",
+    "@material/notched-outline": "12.0.0-canary.03f525f9f.0",
+    "@material/radio": "12.0.0-canary.03f525f9f.0",
+    "@material/ripple": "12.0.0-canary.03f525f9f.0",
+    "@material/rtl": "12.0.0-canary.03f525f9f.0",
+    "@material/segmented-button": "12.0.0-canary.03f525f9f.0",
+    "@material/select": "12.0.0-canary.03f525f9f.0",
+    "@material/shape": "12.0.0-canary.03f525f9f.0",
+    "@material/slider": "12.0.0-canary.03f525f9f.0",
+    "@material/snackbar": "12.0.0-canary.03f525f9f.0",
+    "@material/switch": "12.0.0-canary.03f525f9f.0",
+    "@material/tab": "12.0.0-canary.03f525f9f.0",
+    "@material/tab-bar": "12.0.0-canary.03f525f9f.0",
+    "@material/tab-indicator": "12.0.0-canary.03f525f9f.0",
+    "@material/tab-scroller": "12.0.0-canary.03f525f9f.0",
+    "@material/textfield": "12.0.0-canary.03f525f9f.0",
+    "@material/theme": "12.0.0-canary.03f525f9f.0",
+    "@material/tooltip": "12.0.0-canary.03f525f9f.0",
+    "@material/top-app-bar": "12.0.0-canary.03f525f9f.0",
+    "@material/touch-target": "12.0.0-canary.03f525f9f.0",
+    "@material/typography": "12.0.0-canary.03f525f9f.0",
     "@octokit/rest": "18.3.5",
     "@schematics/angular": "^12.0.0-rc.0",
     "@types/autoprefixer": "^9.7.2",

scripts/check-mdc-tests-config.ts

@@ -82,6 +82,10 @@ export const config = {
       'should work in a step'
     ],
     'mdc-list': [
+      // MDC does focus previously focused options, but rather always selects the first selected
+      // option. We have different test in the MDC-based list that captures this behavior.
+      'should focus the previously focused option when the list takes focus a second time',
+
       // TODO: these tests need to be double-checked for missing functionality.
       'should not apply any additional class to a list without lines',
       'should not add the mat-list-single-selected-option class (in multiple mode)',

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

@@ -289,17 +289,24 @@ describe('MDC-based MatSelectionList without forms', () => {
       expect(listOptions.slice(1).every(o => o.nativeElement.tabIndex === -1)).toBe(true);
     });
 
-    it('should focus the previously focused option when the list takes focus a second time',
-        fakeAsync(() => {
-      expect(listOptions[0].nativeElement.tabIndex).toBe(0);
-      expect(listOptions[1].nativeElement.tabIndex).toBe(-1);
+    it('should focus the first selected option when list receives focus', fakeAsync(() => {
+      dispatchMouseEvent(listOptions[2].nativeElement, 'click');
+      fixture.detectChanges();
+
+      expect(listOptions.map(o => o.nativeElement.tabIndex)).toEqual([-1, -1, 0, -1, -1]);
+
+      dispatchMouseEvent(listOptions[1].nativeElement, 'click');
+      fixture.detectChanges();
 
-      dispatchFakeEvent(listOptions[1].nativeElement, 'focusin', true);
-      dispatchFakeEvent(listOptions[1].nativeElement, 'focusout', true);
-      tick(1);
+      expect(listOptions.map(o => o.nativeElement.tabIndex)).toEqual([-1, 0, -1, -1, -1]);
 
-      expect(listOptions[0].nativeElement.tabIndex).toBe(-1);
-      expect(listOptions[1].nativeElement.tabIndex).toBe(0);
+      // De-select both options to ensure that the first item in the list-item
+      // becomes the designated option for focus.
+      dispatchMouseEvent(listOptions[1].nativeElement, 'click');
+      dispatchMouseEvent(listOptions[2].nativeElement, 'click');
+      fixture.detectChanges();
+
+      expect(listOptions.map(o => o.nativeElement.tabIndex)).toEqual([0, -1, -1, -1, -1]);
     }));
 
     it('should focus previous item when press UP ARROW', () => {
@@ -567,7 +574,7 @@ describe('MDC-based MatSelectionList without forms', () => {
 
   describe('with list option selected', () => {
     let fixture: ComponentFixture<SelectionListWithSelectedOption>;
-    let listItemEl: DebugElement;
+    let listOptionElements: DebugElement[];
     let selectionList: DebugElement;
 
     beforeEach(waitForAsync(() => {
@@ -581,16 +588,28 @@ describe('MDC-based MatSelectionList without forms', () => {
 
     beforeEach(waitForAsync(() => {
       fixture = TestBed.createComponent(SelectionListWithSelectedOption);
-      listItemEl = fixture.debugElement.query(By.directive(MatListOption))!;
+      listOptionElements = fixture.debugElement.queryAll(By.directive(MatListOption))!;
       selectionList = fixture.debugElement.query(By.directive(MatSelectionList))!;
       fixture.detectChanges();
     }));
 
     it('should set its initial selected state in the selectedOptions', () => {
-      let optionEl = listItemEl.injector.get<MatListOption>(MatListOption);
+      let options = listOptionElements.map(optionEl =>
+          optionEl.injector.get<MatListOption>(MatListOption));
       let selectedOptions = selectionList.componentInstance.selectedOptions;
-      expect(selectedOptions.isSelected(optionEl)).toBeTruthy();
+      expect(selectedOptions.isSelected(options[0])).toBeFalse();
+      expect(selectedOptions.isSelected(options[1])).toBeTrue();
+      expect(selectedOptions.isSelected(options[2])).toBeTrue();
+      expect(selectedOptions.isSelected(options[3])).toBeFalse();
     });
+
+    it('should focus the first selected option on first focus if an item is pre-selected',
+        fakeAsync(() => {
+      // MDC manages the focus through setting a `tabindex` on the designated list item. We
+      // assert that the proper tabindex is set on the pre-selected option at index 1, and
+      // ensure that other options are not reachable through tab.
+      expect(listOptionElements.map(el => el.nativeElement.tabIndex)).toEqual([-1, 0, -1, -1]);
+    }));
   });
 
   describe('with option disabled', () => {
@@ -1414,7 +1433,10 @@ class SelectionListWithDisabledOption {
 
 @Component({template: `
   <mat-selection-list>
-    <mat-list-option [selected]="true">Item</mat-list-option>
+    <mat-list-option>Not selected - Item #1</mat-list-option>
+    <mat-list-option [selected]="true">Pre-selected - Item #2</mat-list-option>
+    <mat-list-option [selected]="true">Pre-selected - Item #3</mat-list-option>
+    <mat-list-option>Not selected - Item #4</mat-list-option>
   </mat-selection-list>`})
 class SelectionListWithSelectedOption {
 }

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

@@ -28,7 +28,7 @@ import {
 } from '@angular/core';
 import {ControlValueAccessor, NG_VALUE_ACCESSOR} from '@angular/forms';
 import {ThemePalette} from '@angular/material-experimental/mdc-core';
-import {MDCListAdapter} from '@material/list';
+import {MDCListAdapter, numbers as mdcListNumbers} from '@material/list';
 import {Subject} from 'rxjs';
 import {takeUntil} from 'rxjs/operators';
 import {getInteractiveListAdapter, MatInteractiveListBase} from './interactive-list-base';
@@ -141,34 +141,28 @@ export class MatSelectionList extends MatInteractiveListBase<MatListOption>
     // binding can no longer be changed.
     this._initialized = true;
 
-    // Update the options if a control value has been set initially.
+    // Update the options if a control value has been set initially. Note that this should happen
+    // before watching for selection changes as otherwise we would sync options with MDC multiple
+    // times as part of view initialization (also the foundation would not be initialized yet).
     if (this._value) {
       this._setOptionsFromValues(this._value);
     }
 
-    // Sync external changes to the model back to the options.
-    this.selectedOptions.changed.pipe(takeUntil(this._destroyed)).subscribe(event => {
-      if (event.added) {
-        for (let item of event.added) {
-          item.selected = true;
-        }
-      }
+    // Start monitoring the selected options so that the list foundation can be
+    // updated accordingly.
+    this._watchForSelectionChange();
 
-      if (event.removed) {
-        for (let item of event.removed) {
-          item.selected = false;
-        }
-      }
+    // Initialize the list foundation, including the initial `layout()` invocation.
+    super.ngAfterViewInit();
 
-      // Sync the newly selected options with the foundation. Also reset tabindex for all
-      // items if the list is currently not focused. We do this so that always the first
-      // selected list item is focused when users tab into the selection list.
+    // List options can be pre-selected using the `selected` input. We need to sync the selected
+    // options after view initialization with the foundation so that focus can be managed
+    // accordingly. Note that this needs to happen after the initial `layout()` call because the
+    // list wouldn't know about multi-selection and throw.
+    if (this._items.length !== 0) {
       this._syncSelectedOptionsWithFoundation();
       this._resetTabindexForItemsIfBlurred();
-    });
-
-    // Complete the list foundation initialization.
-    super.ngAfterViewInit();
+    }
   }
 
   ngOnChanges(changes: SimpleChanges) {
@@ -262,13 +256,37 @@ export class MatSelectionList extends MatInteractiveListBase<MatListOption>
     }
   }
 
+  private _watchForSelectionChange() {
+    // Sync external changes to the model back to the options.
+    this.selectedOptions.changed.pipe(takeUntil(this._destroyed)).subscribe(event => {
+      if (event.added) {
+        for (let item of event.added) {
+          item.selected = true;
+        }
+      }
+
+      if (event.removed) {
+        for (let item of event.removed) {
+          item.selected = false;
+        }
+      }
+
+      // Sync the newly selected options with the foundation. Also reset tabindex for all
+      // items if the list is currently not focused. We do this so that always the first
+      // selected list item is focused when users tab into the selection list.
+      this._syncSelectedOptionsWithFoundation();
+      this._resetTabindexForItemsIfBlurred();
+    });
+  }
+
   private _syncSelectedOptionsWithFoundation() {
     if (this._multiple) {
       this._foundation.setSelectedIndex(this.selectedOptions.selected
           .map(o => this._itemsArr.indexOf(o)));
     } else {
       const selected = this.selectedOptions.selected[0];
-      const index = selected === undefined ? -1 : this._itemsArr.indexOf(selected);
+      const index = selected === undefined ?
+          mdcListNumbers.UNSET_INDEX : this._itemsArr.indexOf(selected);
       this._foundation.setSelectedIndex(index);
     }
   }