Skip to content

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

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 1 commit into from
May 7, 2021
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
96 changes: 48 additions & 48 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
4 changes: 4 additions & 0 deletions scripts/check-mdc-tests-config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)',
Expand Down
50 changes: 36 additions & 14 deletions src/material-experimental/mdc-list/selection-list.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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(() => {
Expand All @@ -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', () => {
Expand Down Expand Up @@ -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 {
}
Expand Down
62 changes: 40 additions & 22 deletions src/material-experimental/mdc-list/selection-list.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
}
}
Expand Down
Loading