From 67d9333ce48912ea68de514008eede9d2fdce5c4 Mon Sep 17 00:00:00 2001 From: Miles Malerba Date: Thu, 23 Apr 2020 19:10:24 -0700 Subject: [PATCH 01/10] feat(material-expeirmental/mdc-list): add support for focus/hover states and ripples --- src/material-experimental/mdc-list/action-list.ts | 4 +++- src/material-experimental/mdc-list/nav-list.ts | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/material-experimental/mdc-list/action-list.ts b/src/material-experimental/mdc-list/action-list.ts index b7df2137b024..533c905dea15 100644 --- a/src/material-experimental/mdc-list/action-list.ts +++ b/src/material-experimental/mdc-list/action-list.ts @@ -23,4 +23,6 @@ import {MatListBase} from './list-base'; {provide: MatListBase, useExisting: MatActionList}, ] }) -export class MatActionList extends MatListBase {} +export class MatActionList extends MatListBase { + _hasRipple = true; +} diff --git a/src/material-experimental/mdc-list/nav-list.ts b/src/material-experimental/mdc-list/nav-list.ts index 28b4dd07cd19..6cd1f9890129 100644 --- a/src/material-experimental/mdc-list/nav-list.ts +++ b/src/material-experimental/mdc-list/nav-list.ts @@ -33,4 +33,6 @@ import {MatListBase} from './list-base'; {provide: MatList, useExisting: MatNavList}, ] }) -export class MatNavList extends MatListBase {} +export class MatNavList extends MatListBase { + _hasRipple = true; +} From ad9a5f3f8762a6a49e16450a9e8aa7cb247ae4c9 Mon Sep 17 00:00:00 2001 From: Miles Malerba Date: Fri, 24 Apr 2020 12:52:58 -0700 Subject: [PATCH 02/10] add state styles --- src/material-experimental/mdc-list/action-list.ts | 4 +--- src/material-experimental/mdc-list/nav-list.ts | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/material-experimental/mdc-list/action-list.ts b/src/material-experimental/mdc-list/action-list.ts index 533c905dea15..b7df2137b024 100644 --- a/src/material-experimental/mdc-list/action-list.ts +++ b/src/material-experimental/mdc-list/action-list.ts @@ -23,6 +23,4 @@ import {MatListBase} from './list-base'; {provide: MatListBase, useExisting: MatActionList}, ] }) -export class MatActionList extends MatListBase { - _hasRipple = true; -} +export class MatActionList extends MatListBase {} diff --git a/src/material-experimental/mdc-list/nav-list.ts b/src/material-experimental/mdc-list/nav-list.ts index 6cd1f9890129..28b4dd07cd19 100644 --- a/src/material-experimental/mdc-list/nav-list.ts +++ b/src/material-experimental/mdc-list/nav-list.ts @@ -33,6 +33,4 @@ import {MatListBase} from './list-base'; {provide: MatList, useExisting: MatNavList}, ] }) -export class MatNavList extends MatListBase { - _hasRipple = true; -} +export class MatNavList extends MatListBase {} From baaa23bdb42390642d91965c65e26050a06e1f9b Mon Sep 17 00:00:00 2001 From: Miles Malerba Date: Mon, 8 Jun 2020 14:16:22 -0700 Subject: [PATCH 03/10] add adapter for MDCList --- .../mdc-list/list-base.ts | 63 ++++++++++++++++--- src/material-experimental/mdc-list/list.ts | 3 + .../mdc-list/selection-list.ts | 3 + 3 files changed, 60 insertions(+), 9 deletions(-) diff --git a/src/material-experimental/mdc-list/list-base.ts b/src/material-experimental/mdc-list/list-base.ts index 090043279999..51f33a6a9194 100644 --- a/src/material-experimental/mdc-list/list-base.ts +++ b/src/material-experimental/mdc-list/list-base.ts @@ -11,12 +11,15 @@ import { AfterContentInit, Directive, ElementRef, + forwardRef, HostBinding, NgZone, OnDestroy, - QueryList + QueryList, + ViewChildren } from '@angular/core'; import {RippleConfig, RippleRenderer, RippleTarget, setLines} from '@angular/material/core'; +import {MDCListAdapter} from '@material/list'; import {Subscription} from 'rxjs'; import {startWith} from 'rxjs/operators'; @@ -37,6 +40,46 @@ export abstract class MatListBase { // tslint:disable-next-line:no-host-decorator-in-concrete @HostBinding('class.mdc-list--non-interactive') _isNonInteractive: boolean = false; + + @ViewChildren(forwardRef(() => MatListItemBase)) _items: QueryList; + + protected adapter: MDCListAdapter = { + getListItemCount: () => this._items.length, + listItemAtIndexHasClass: + (index, className) => this._itemAtIndex(index)._element.classList.contains(className), + addClassForElementIndex: + (index, className) => this._itemAtIndex(index)._element.classList.add(className), + removeClassForElementIndex: + (index, className) => this._itemAtIndex(index)._element.classList.remove(className), + getAttributeForElementIndex: + (index, attr) => this._itemAtIndex(index)._element.getAttribute(attr), + setAttributeForElementIndex: + (index, attr, value) => this._itemAtIndex(index)._element.setAttribute(attr, value), + setTabIndexForListItemChildren: + (index, value) => this._itemAtIndex(index)._element.tabIndex = value as unknown as number, + getFocusedElementIndex: + () => this._items.map(i => i._element).findIndex(e => e === this.doc?.activeELement), + isFocusInsideList: () => this.element.nativeElement.contains(this.doc?.activeElement), + isRootFocused: () => this.element.nativeElement === this.doc?.activeElement, + focusItemAtIndex: index => this._itemAtIndex(index)._element.focus(), + + // The following methods have a dummy implementation in the base class because they are only + // applicable to certain types of lists + hasCheckboxAtIndex: () => false, + hasRadioAtIndex: () => false, + setCheckedCheckboxOrRadioAtIndex: () => {}, + isCheckboxCheckedAtIndex: () => false, + notifyAction: () => {}, + + // TODO(mmalerba): Determine if we need to implement this. + getPrimaryTextAtIndex: () => '', + }; + + constructor(protected element: ElementRef, protected doc: any) {} + + private _itemAtIndex(index: number): MatListItemBase { + return this._items.toArray()[index]; + } } @Directive() @@ -49,20 +92,22 @@ export abstract class MatListItemBase implements AfterContentInit, OnDestroy, Ri // TODO(mmalerba): Add @Input for disabling ripple. rippleDisabled: boolean; + _element: HTMLElement; + private _subscriptions = new Subscription(); private _rippleRenderer: RippleRenderer; - constructor(protected _element: ElementRef, protected _ngZone: NgZone, listBase: MatListBase, - platform: Platform) { - const el = this._element.nativeElement; + constructor(private _elementRef: ElementRef, protected _ngZone: NgZone, + listBase: MatListBase, platform: Platform) { + this._element = this._elementRef.nativeElement; this.rippleDisabled = listBase._isNonInteractive; if (!listBase._isNonInteractive) { - el.classList.add('mat-mdc-list-item-interactive'); + this._element.classList.add('mat-mdc-list-item-interactive'); } this._rippleRenderer = - new RippleRenderer(this, this._ngZone, el, platform); - this._rippleRenderer.setupTriggerEvents(el); + new RippleRenderer(this, this._ngZone, this._element, platform); + this._rippleRenderer.setupTriggerEvents(this._element); } ngAfterContentInit() { @@ -77,14 +122,14 @@ export abstract class MatListItemBase implements AfterContentInit, OnDestroy, Ri this._ngZone.runOutsideAngular(() => { this._subscriptions.add(this.lines.changes.pipe(startWith(this.lines)) .subscribe((lines: QueryList>) => { - this._element.nativeElement.classList + this._element.classList .toggle('mat-mdc-list-item-single-line', lines.length <= 1); lines.forEach((line: ElementRef, index: number) => { toggleClass(line.nativeElement, 'mdc-list-item__primary-text', index === 0 && lines.length > 1); toggleClass(line.nativeElement, 'mdc-list-item__secondary-text', index !== 0); }); - setLines(lines, this._element, 'mat-mdc'); + setLines(lines, this._elementRef, 'mat-mdc'); })); }); } diff --git a/src/material-experimental/mdc-list/list.ts b/src/material-experimental/mdc-list/list.ts index 36396665a861..37ec4bc5bbdc 100644 --- a/src/material-experimental/mdc-list/list.ts +++ b/src/material-experimental/mdc-list/list.ts @@ -79,6 +79,9 @@ export class MatList extends MatListBase { templateUrl: 'list-item.html', encapsulation: ViewEncapsulation.None, changeDetection: ChangeDetectionStrategy.OnPush, + providers: [ + {provide: MatListItemBase, useExisting: MatListItem}, + ] }) export class MatListItem extends MatListItemBase { @ContentChildren(MatLine, {read: ElementRef, descendants: true}) lines: diff --git a/src/material-experimental/mdc-list/selection-list.ts b/src/material-experimental/mdc-list/selection-list.ts index c6f464b8ecd7..65b11ec00ba6 100644 --- a/src/material-experimental/mdc-list/selection-list.ts +++ b/src/material-experimental/mdc-list/selection-list.ts @@ -95,6 +95,9 @@ export class MatSelectionList extends MatListBase implements ControlValueAccesso templateUrl: 'list-option.html', encapsulation: ViewEncapsulation.None, changeDetection: ChangeDetectionStrategy.OnPush, + providers: [ + {provide: MatListItemBase, useExisting: MatListOption}, + ] }) export class MatListOption extends MatListItemBase { static ngAcceptInputType_disabled: BooleanInput; From 3630a5c2995a1d432615a38bd37eee120906674e Mon Sep 17 00:00:00 2001 From: Miles Malerba Date: Mon, 8 Jun 2020 16:50:41 -0700 Subject: [PATCH 04/10] set up MDCListFoundation --- .../mdc-list/list-base.ts | 156 +++++++++++------- 1 file changed, 99 insertions(+), 57 deletions(-) diff --git a/src/material-experimental/mdc-list/list-base.ts b/src/material-experimental/mdc-list/list-base.ts index 51f33a6a9194..17c0494b7433 100644 --- a/src/material-experimental/mdc-list/list-base.ts +++ b/src/material-experimental/mdc-list/list-base.ts @@ -7,19 +7,22 @@ */ import {Platform} from '@angular/cdk/platform'; +import {DOCUMENT} from '@angular/common'; import { AfterContentInit, + AfterViewInit, + ContentChildren, Directive, ElementRef, - forwardRef, HostBinding, + HostListener, + Inject, NgZone, OnDestroy, - QueryList, - ViewChildren + QueryList } from '@angular/core'; import {RippleConfig, RippleRenderer, RippleTarget, setLines} from '@angular/material/core'; -import {MDCListAdapter} from '@material/list'; +import {MDCListAdapter, MDCListFoundation} from '@material/list'; import {Subscription} from 'rxjs'; import {startWith} from 'rxjs/operators'; @@ -31,60 +34,12 @@ function toggleClass(el: Element, className: string, on: boolean) { } } -@Directive() -/** @docs-private */ -export abstract class MatListBase { - // @HostBinding is used in the class as it is expected to be extended. Since @Component decorator - // metadata is not inherited by child classes, instead the host binding data is defined in a way - // that can be inherited. - // tslint:disable-next-line:no-host-decorator-in-concrete - @HostBinding('class.mdc-list--non-interactive') - _isNonInteractive: boolean = false; - - @ViewChildren(forwardRef(() => MatListItemBase)) _items: QueryList; - - protected adapter: MDCListAdapter = { - getListItemCount: () => this._items.length, - listItemAtIndexHasClass: - (index, className) => this._itemAtIndex(index)._element.classList.contains(className), - addClassForElementIndex: - (index, className) => this._itemAtIndex(index)._element.classList.add(className), - removeClassForElementIndex: - (index, className) => this._itemAtIndex(index)._element.classList.remove(className), - getAttributeForElementIndex: - (index, attr) => this._itemAtIndex(index)._element.getAttribute(attr), - setAttributeForElementIndex: - (index, attr, value) => this._itemAtIndex(index)._element.setAttribute(attr, value), - setTabIndexForListItemChildren: - (index, value) => this._itemAtIndex(index)._element.tabIndex = value as unknown as number, - getFocusedElementIndex: - () => this._items.map(i => i._element).findIndex(e => e === this.doc?.activeELement), - isFocusInsideList: () => this.element.nativeElement.contains(this.doc?.activeElement), - isRootFocused: () => this.element.nativeElement === this.doc?.activeElement, - focusItemAtIndex: index => this._itemAtIndex(index)._element.focus(), - - // The following methods have a dummy implementation in the base class because they are only - // applicable to certain types of lists - hasCheckboxAtIndex: () => false, - hasRadioAtIndex: () => false, - setCheckedCheckboxOrRadioAtIndex: () => {}, - isCheckboxCheckedAtIndex: () => false, - notifyAction: () => {}, - - // TODO(mmalerba): Determine if we need to implement this. - getPrimaryTextAtIndex: () => '', - }; - - constructor(protected element: ElementRef, protected doc: any) {} - - private _itemAtIndex(index: number): MatListItemBase { - return this._items.toArray()[index]; - } -} - @Directive() /** @docs-private */ export abstract class MatListItemBase implements AfterContentInit, OnDestroy, RippleTarget { + @HostBinding('tabindex') + _tabIndex = -1; + lines: QueryList>; rippleConfig: RippleConfig = {}; @@ -98,8 +53,8 @@ export abstract class MatListItemBase implements AfterContentInit, OnDestroy, Ri private _rippleRenderer: RippleRenderer; - constructor(private _elementRef: ElementRef, protected _ngZone: NgZone, - listBase: MatListBase, platform: Platform) { + protected constructor(private _elementRef: ElementRef, protected _ngZone: NgZone, + listBase: MatListBase, platform: Platform) { this._element = this._elementRef.nativeElement; this.rippleDisabled = listBase._isNonInteractive; if (!listBase._isNonInteractive) { @@ -139,3 +94,90 @@ export abstract class MatListItemBase implements AfterContentInit, OnDestroy, Ri this._rippleRenderer._removeTriggerEvents(); } } + +@Directive() +/** @docs-private */ +export abstract class MatListBase implements AfterViewInit, OnDestroy { + @HostBinding('class.mdc-list--non-interactive') + _isNonInteractive: boolean = false; + + @HostListener('keydown', ['$event']) + _handleKeydown(event: KeyboardEvent) { + const index = this._indexForElement(event.target as HTMLElement); + this._foundation.handleKeydown( + event, this._itemAtIndex(index)._element === event.target, index); + } + + @HostListener('click', ['$event']) + _handleClick(event: MouseEvent) { + this._foundation.handleClick(this._indexForElement(event.target as HTMLElement), false); + } + + @HostListener('focusin', ['$event']) + _handleFocusin(event: FocusEvent) { + this._foundation.handleFocusIn(event, this._indexForElement(event.target as HTMLElement)); + } + + @HostListener('focusout', ['$event']) + _handleFocusout(event: FocusEvent) { + this._foundation.handleFocusOut(event, this._indexForElement(event.target as HTMLElement)); + } + + @ContentChildren(MatListItemBase) _items: QueryList; + + protected _adapter: MDCListAdapter = { + getListItemCount: () => this._items.length, + listItemAtIndexHasClass: + (index, className) => this._itemAtIndex(index)._element.classList.contains(className), + addClassForElementIndex: + (index, className) => this._itemAtIndex(index)._element.classList.add(className), + removeClassForElementIndex: + (index, className) => this._itemAtIndex(index)._element.classList.remove(className), + getAttributeForElementIndex: + (index, attr) => this._itemAtIndex(index)._element.getAttribute(attr), + setAttributeForElementIndex: + (index, attr, value) => this._itemAtIndex(index)._element.setAttribute(attr, value), + setTabIndexForListItemChildren: + (index, value) => this._itemAtIndex(index)._element.tabIndex = value as unknown as number, + getFocusedElementIndex: () => this._indexForElement(this._document?.activeELement), + isFocusInsideList: () => this._element.nativeElement.contains(this._document?.activeElement), + isRootFocused: () => this._element.nativeElement === this._document?.activeElement, + focusItemAtIndex: index => this._itemAtIndex(index)._element.focus(), + + // The following methods have a dummy implementation in the base class because they are only + // applicable to certain types of lists. They should be implemented for the concrete classes + // where they are applicable. + hasCheckboxAtIndex: () => false, + hasRadioAtIndex: () => false, + setCheckedCheckboxOrRadioAtIndex: () => {}, + isCheckboxCheckedAtIndex: () => false, + notifyAction: () => {}, + + // TODO(mmalerba): Determine if we need to implement this. + getPrimaryTextAtIndex: () => '', + }; + + protected _foundation: MDCListFoundation; + + constructor(protected _element: ElementRef, + @Inject(DOCUMENT) protected _document: any) { + this._foundation = new MDCListFoundation(this._adapter); + } + + ngAfterViewInit() { + this._foundation.init(); + this._foundation.layout(); + } + + ngOnDestroy() { + this._foundation.destroy(); + } + + private _itemAtIndex(index: number): MatListItemBase { + return this._items.toArray()[index]; + } + + private _indexForElement(element: HTMLElement | null) { + return element ? this._items.toArray().findIndex(i => i._element.contains(element)) : -1; + } +} From 12a0c1a8a55822aea404ce0032290ac047e5c268 Mon Sep 17 00:00:00 2001 From: Miles Malerba Date: Wed, 10 Jun 2020 15:26:47 -0700 Subject: [PATCH 05/10] refactor so only interactive lists set up the foundation --- .../mdc-list/action-list.ts | 4 +- .../mdc-list/list-base.ts | 37 +++++++++++++------ src/material-experimental/mdc-list/list.ts | 4 +- .../mdc-list/nav-list.ts | 4 +- 4 files changed, 31 insertions(+), 18 deletions(-) diff --git a/src/material-experimental/mdc-list/action-list.ts b/src/material-experimental/mdc-list/action-list.ts index b7df2137b024..0a4e9ad05848 100644 --- a/src/material-experimental/mdc-list/action-list.ts +++ b/src/material-experimental/mdc-list/action-list.ts @@ -7,7 +7,7 @@ */ import {ChangeDetectionStrategy, Component, ViewEncapsulation} from '@angular/core'; -import {MatListBase} from './list-base'; +import {MatInteractiveListBase, MatListBase} from './list-base'; @Component({ selector: 'mat-action-list', @@ -23,4 +23,4 @@ import {MatListBase} from './list-base'; {provide: MatListBase, useExisting: MatActionList}, ] }) -export class MatActionList extends MatListBase {} +export class MatActionList extends MatInteractiveListBase {} diff --git a/src/material-experimental/mdc-list/list-base.ts b/src/material-experimental/mdc-list/list-base.ts index 17c0494b7433..743bba1adadd 100644 --- a/src/material-experimental/mdc-list/list-base.ts +++ b/src/material-experimental/mdc-list/list-base.ts @@ -69,6 +69,15 @@ export abstract class MatListItemBase implements AfterContentInit, OnDestroy, Ri this._monitorLines(); } + ngOnDestroy() { + this._subscriptions.unsubscribe(); + this._rippleRenderer._removeTriggerEvents(); + } + + _setTabIndexForChildren(value: number) { + this._element.querySelectorAll('a, button').forEach(el => (el as HTMLElement).tabIndex = value); + } + /** * Subscribes to changes in `MatLine` content children and annotates them appropriately when they * change. @@ -88,19 +97,18 @@ export abstract class MatListItemBase implements AfterContentInit, OnDestroy, Ri })); }); } - - ngOnDestroy() { - this._subscriptions.unsubscribe(); - this._rippleRenderer._removeTriggerEvents(); - } } @Directive() /** @docs-private */ -export abstract class MatListBase implements AfterViewInit, OnDestroy { +export abstract class MatListBase { @HostBinding('class.mdc-list--non-interactive') - _isNonInteractive: boolean = false; + _isNonInteractive: boolean = true; +} +@Directive() +export abstract class MatInteractiveListBase extends MatListBase + implements AfterViewInit, OnDestroy { @HostListener('keydown', ['$event']) _handleKeydown(event: KeyboardEvent) { const index = this._indexForElement(event.target as HTMLElement); @@ -138,7 +146,7 @@ export abstract class MatListBase implements AfterViewInit, OnDestroy { setAttributeForElementIndex: (index, attr, value) => this._itemAtIndex(index)._element.setAttribute(attr, value), setTabIndexForListItemChildren: - (index, value) => this._itemAtIndex(index)._element.tabIndex = value as unknown as number, + (index, value) => this._itemAtIndex(index)._setTabIndexForChildren(Number(value)), getFocusedElementIndex: () => this._indexForElement(this._document?.activeELement), isFocusInsideList: () => this._element.nativeElement.contains(this._document?.activeElement), isRootFocused: () => this._element.nativeElement === this._document?.activeElement, @@ -151,21 +159,27 @@ export abstract class MatListBase implements AfterViewInit, OnDestroy { hasRadioAtIndex: () => false, setCheckedCheckboxOrRadioAtIndex: () => {}, isCheckboxCheckedAtIndex: () => false, - notifyAction: () => {}, - // TODO(mmalerba): Determine if we need to implement this. + // TODO(mmalerba): Determine if we need to implement these. getPrimaryTextAtIndex: () => '', + notifyAction: () => {}, }; protected _foundation: MDCListFoundation; constructor(protected _element: ElementRef, - @Inject(DOCUMENT) protected _document: any) { + @Inject(DOCUMENT) protected _document: any) { + super(); + this._isNonInteractive = false; this._foundation = new MDCListFoundation(this._adapter); } ngAfterViewInit() { this._foundation.init(); + const first = this._items.toArray()[0]?._element; + if (first) { + first.tabIndex = 0; + } this._foundation.layout(); } @@ -181,3 +195,4 @@ export abstract class MatListBase implements AfterViewInit, OnDestroy { return element ? this._items.toArray().findIndex(i => i._element.contains(element)) : -1; } } + diff --git a/src/material-experimental/mdc-list/list.ts b/src/material-experimental/mdc-list/list.ts index 37ec4bc5bbdc..b49250c113c8 100644 --- a/src/material-experimental/mdc-list/list.ts +++ b/src/material-experimental/mdc-list/list.ts @@ -66,9 +66,7 @@ export class MatListSubheaderCssMatStyler {} {provide: MatListBase, useExisting: MatList}, ] }) -export class MatList extends MatListBase { - _isNonInteractive = true; -} +export class MatList extends MatListBase {} @Component({ selector: 'mat-list-item, a[mat-list-item], button[mat-list-item]', diff --git a/src/material-experimental/mdc-list/nav-list.ts b/src/material-experimental/mdc-list/nav-list.ts index 28b4dd07cd19..7d22950b8ff9 100644 --- a/src/material-experimental/mdc-list/nav-list.ts +++ b/src/material-experimental/mdc-list/nav-list.ts @@ -8,7 +8,7 @@ import {ChangeDetectionStrategy, Component, ViewEncapsulation} from '@angular/core'; import {MatList} from './list'; -import {MatListBase} from './list-base'; +import {MatInteractiveListBase, MatListBase} from './list-base'; @Component({ selector: 'mat-nav-list', @@ -33,4 +33,4 @@ import {MatListBase} from './list-base'; {provide: MatList, useExisting: MatNavList}, ] }) -export class MatNavList extends MatListBase {} +export class MatNavList extends MatInteractiveListBase {} From ccdc864ab1592361c5328b79bbac42f560b70e9a Mon Sep 17 00:00:00 2001 From: Miles Malerba Date: Thu, 11 Jun 2020 14:14:09 -0700 Subject: [PATCH 06/10] address feedback --- .../mdc-list/list-base.ts | 67 ++++++++++++------- 1 file changed, 42 insertions(+), 25 deletions(-) diff --git a/src/material-experimental/mdc-list/list-base.ts b/src/material-experimental/mdc-list/list-base.ts index 743bba1adadd..aec807a29d54 100644 --- a/src/material-experimental/mdc-list/list-base.ts +++ b/src/material-experimental/mdc-list/list-base.ts @@ -47,22 +47,19 @@ export abstract class MatListItemBase implements AfterContentInit, OnDestroy, Ri // TODO(mmalerba): Add @Input for disabling ripple. rippleDisabled: boolean; - _element: HTMLElement; - private _subscriptions = new Subscription(); private _rippleRenderer: RippleRenderer; - protected constructor(private _elementRef: ElementRef, protected _ngZone: NgZone, + protected constructor(public _elementRef: ElementRef, protected _ngZone: NgZone, listBase: MatListBase, platform: Platform) { - this._element = this._elementRef.nativeElement; this.rippleDisabled = listBase._isNonInteractive; if (!listBase._isNonInteractive) { - this._element.classList.add('mat-mdc-list-item-interactive'); + this._elementRef.nativeElement.classList.add('mat-mdc-list-item-interactive'); } this._rippleRenderer = - new RippleRenderer(this, this._ngZone, this._element, platform); - this._rippleRenderer.setupTriggerEvents(this._element); + new RippleRenderer(this, this._ngZone, this._elementRef.nativeElement, platform); + this._rippleRenderer.setupTriggerEvents(this._elementRef.nativeElement); } ngAfterContentInit() { @@ -74,8 +71,15 @@ export abstract class MatListItemBase implements AfterContentInit, OnDestroy, Ri this._rippleRenderer._removeTriggerEvents(); } + /** + * Changes the tabindex of all button and anchor children of this item. + * + * This method is used by the `MatInteractiveBaseList` to implement the + * `setTabIndexForListItemChildren` method on the `MDCListAdapter` + */ _setTabIndexForChildren(value: number) { - this._element.querySelectorAll('a, button').forEach(el => (el as HTMLElement).tabIndex = value); + this._elementRef.nativeElement.querySelectorAll('a, button') + .forEach(el => el.tabIndex = value); } /** @@ -86,7 +90,7 @@ export abstract class MatListItemBase implements AfterContentInit, OnDestroy, Ri this._ngZone.runOutsideAngular(() => { this._subscriptions.add(this.lines.changes.pipe(startWith(this.lines)) .subscribe((lines: QueryList>) => { - this._element.classList + this._elementRef.nativeElement.classList .toggle('mat-mdc-list-item-single-line', lines.length <= 1); lines.forEach((line: ElementRef, index: number) => { toggleClass(line.nativeElement, @@ -113,7 +117,7 @@ export abstract class MatInteractiveListBase extends MatListBase _handleKeydown(event: KeyboardEvent) { const index = this._indexForElement(event.target as HTMLElement); this._foundation.handleKeydown( - event, this._itemAtIndex(index)._element === event.target, index); + event, this._elementAtIndex(index) === event.target, index); } @HostListener('click', ['$event']) @@ -136,21 +140,20 @@ export abstract class MatInteractiveListBase extends MatListBase protected _adapter: MDCListAdapter = { getListItemCount: () => this._items.length, listItemAtIndexHasClass: - (index, className) => this._itemAtIndex(index)._element.classList.contains(className), + (index, className) => this._elementAtIndex(index).classList.contains(className), addClassForElementIndex: - (index, className) => this._itemAtIndex(index)._element.classList.add(className), + (index, className) => this._elementAtIndex(index).classList.add(className), removeClassForElementIndex: - (index, className) => this._itemAtIndex(index)._element.classList.remove(className), - getAttributeForElementIndex: - (index, attr) => this._itemAtIndex(index)._element.getAttribute(attr), + (index, className) => this._elementAtIndex(index).classList.remove(className), + getAttributeForElementIndex: (index, attr) => this._elementAtIndex(index).getAttribute(attr), setAttributeForElementIndex: - (index, attr, value) => this._itemAtIndex(index)._element.setAttribute(attr, value), + (index, attr, value) => this._elementAtIndex(index).setAttribute(attr, value), setTabIndexForListItemChildren: (index, value) => this._itemAtIndex(index)._setTabIndexForChildren(Number(value)), - getFocusedElementIndex: () => this._indexForElement(this._document?.activeELement), + getFocusedElementIndex: () => this._indexForElement(this._document?.activeElement), isFocusInsideList: () => this._element.nativeElement.contains(this._document?.activeElement), isRootFocused: () => this._element.nativeElement === this._document?.activeElement, - focusItemAtIndex: index => this._itemAtIndex(index)._element.focus(), + focusItemAtIndex: index => this._elementAtIndex(index).focus(), // The following methods have a dummy implementation in the base class because they are only // applicable to certain types of lists. They should be implemented for the concrete classes @@ -167,32 +170,46 @@ export abstract class MatInteractiveListBase extends MatListBase protected _foundation: MDCListFoundation; - constructor(protected _element: ElementRef, - @Inject(DOCUMENT) protected _document: any) { + protected _document: Document; + + private _itemsArr: MatListItemBase[] = []; + + private _subscriptions = new Subscription(); + + constructor(protected _element: ElementRef, @Inject(DOCUMENT) document: any) { super(); + this._document = document; this._isNonInteractive = false; this._foundation = new MDCListFoundation(this._adapter); } ngAfterViewInit() { this._foundation.init(); - const first = this._items.toArray()[0]?._element; + const first = this._itemAtIndex(0); if (first) { - first.tabIndex = 0; + first._elementRef.nativeElement.tabIndex = 0; } this._foundation.layout(); + this._subscriptions.add( + this._items.changes.subscribe(() => this._itemsArr = this._items.toArray())); } ngOnDestroy() { this._foundation.destroy(); + this._subscriptions.unsubscribe(); } private _itemAtIndex(index: number): MatListItemBase { - return this._items.toArray()[index]; + return this._itemsArr[index]; + } + + private _elementAtIndex(index: number): HTMLElement { + return this._itemAtIndex(index)._elementRef.nativeElement; } - private _indexForElement(element: HTMLElement | null) { - return element ? this._items.toArray().findIndex(i => i._element.contains(element)) : -1; + private _indexForElement(element: Element | null) { + return element ? + this._itemsArr.findIndex(i => i._elementRef.nativeElement.contains(element)) : -1; } } From 3618ddb02fafa7d5e9add0c3ceb3fcb3bd552938 Mon Sep 17 00:00:00 2001 From: Miles Malerba Date: Thu, 11 Jun 2020 14:23:30 -0700 Subject: [PATCH 07/10] use descendants:true for content children --- src/material-experimental/mdc-list/list-base.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/material-experimental/mdc-list/list-base.ts b/src/material-experimental/mdc-list/list-base.ts index aec807a29d54..e82886a1e228 100644 --- a/src/material-experimental/mdc-list/list-base.ts +++ b/src/material-experimental/mdc-list/list-base.ts @@ -135,7 +135,7 @@ export abstract class MatInteractiveListBase extends MatListBase this._foundation.handleFocusOut(event, this._indexForElement(event.target as HTMLElement)); } - @ContentChildren(MatListItemBase) _items: QueryList; + @ContentChildren(MatListItemBase, {descendants: true}) _items: QueryList; protected _adapter: MDCListAdapter = { getListItemCount: () => this._items.length, From 6963015eb7842687eb0232b80262131683798c4f Mon Sep 17 00:00:00 2001 From: Miles Malerba Date: Fri, 12 Jun 2020 10:54:36 -0700 Subject: [PATCH 08/10] don't change tabindex on child elements of list-item --- .../mdc-list/list-base.ts | 27 +++++++++---------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/src/material-experimental/mdc-list/list-base.ts b/src/material-experimental/mdc-list/list-base.ts index e82886a1e228..6df78367bb62 100644 --- a/src/material-experimental/mdc-list/list-base.ts +++ b/src/material-experimental/mdc-list/list-base.ts @@ -71,17 +71,6 @@ export abstract class MatListItemBase implements AfterContentInit, OnDestroy, Ri this._rippleRenderer._removeTriggerEvents(); } - /** - * Changes the tabindex of all button and anchor children of this item. - * - * This method is used by the `MatInteractiveBaseList` to implement the - * `setTabIndexForListItemChildren` method on the `MDCListAdapter` - */ - _setTabIndexForChildren(value: number) { - this._elementRef.nativeElement.querySelectorAll('a, button') - .forEach(el => el.tabIndex = value); - } - /** * Subscribes to changes in `MatLine` content children and annotates them appropriately when they * change. @@ -148,13 +137,20 @@ export abstract class MatInteractiveListBase extends MatListBase getAttributeForElementIndex: (index, attr) => this._elementAtIndex(index).getAttribute(attr), setAttributeForElementIndex: (index, attr, value) => this._elementAtIndex(index).setAttribute(attr, value), - setTabIndexForListItemChildren: - (index, value) => this._itemAtIndex(index)._setTabIndexForChildren(Number(value)), getFocusedElementIndex: () => this._indexForElement(this._document?.activeElement), isFocusInsideList: () => this._element.nativeElement.contains(this._document?.activeElement), isRootFocused: () => this._element.nativeElement === this._document?.activeElement, focusItemAtIndex: index => this._elementAtIndex(index).focus(), + // MDC uses this method to disable focusable children of list items. However, we believe that + // this is not an accessible pattern and should be avoided, therefore we intentionally do not + // implement this method. In addition, implementing this would require violating Angular + // Material's general principle of not having components modify DOM elements they do not own. + // A user who feels they really need this feature can simply listen to the `(focus)` and + // `(blur)` events on the list item and enable/disable focus on the children themselves as + // appropriate. + setTabIndexForListItemChildren: () => {}, + // The following methods have a dummy implementation in the base class because they are only // applicable to certain types of lists. They should be implemented for the concrete classes // where they are applicable. @@ -185,13 +181,14 @@ export abstract class MatInteractiveListBase extends MatListBase ngAfterViewInit() { this._foundation.init(); - const first = this._itemAtIndex(0); + const first = this._items.first; if (first) { first._elementRef.nativeElement.tabIndex = 0; } this._foundation.layout(); this._subscriptions.add( - this._items.changes.subscribe(() => this._itemsArr = this._items.toArray())); + this._items.changes.pipe(startWith(null)) + .subscribe(() => this._itemsArr = this._items.toArray())); } ngOnDestroy() { From e40e8687d810fda143f9415ad3143ac7f511e928 Mon Sep 17 00:00:00 2001 From: Miles Malerba Date: Fri, 12 Jun 2020 11:11:30 -0700 Subject: [PATCH 09/10] fix tabIndex initialization --- src/material-experimental/mdc-list/list-base.ts | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/material-experimental/mdc-list/list-base.ts b/src/material-experimental/mdc-list/list-base.ts index 6df78367bb62..f8cac525783a 100644 --- a/src/material-experimental/mdc-list/list-base.ts +++ b/src/material-experimental/mdc-list/list-base.ts @@ -37,9 +37,6 @@ function toggleClass(el: Element, className: string, on: boolean) { @Directive() /** @docs-private */ export abstract class MatListItemBase implements AfterContentInit, OnDestroy, RippleTarget { - @HostBinding('tabindex') - _tabIndex = -1; - lines: QueryList>; rippleConfig: RippleConfig = {}; @@ -180,15 +177,14 @@ export abstract class MatInteractiveListBase extends MatListBase } ngAfterViewInit() { - this._foundation.init(); - const first = this._items.first; - if (first) { - first._elementRef.nativeElement.tabIndex = 0; - } - this._foundation.layout(); this._subscriptions.add( this._items.changes.pipe(startWith(null)) .subscribe(() => this._itemsArr = this._items.toArray())); + this._foundation.init(); + for (let i = 0; this._items.length; i++) { + this._elementAtIndex(i).tabIndex = i === 0 ? 0 : -1; + } + this._foundation.layout(); } ngOnDestroy() { From 287a5885ffb95cf484054167254cc324eac1b2bf Mon Sep 17 00:00:00 2001 From: Miles Malerba Date: Fri, 19 Jun 2020 11:11:31 -0700 Subject: [PATCH 10/10] move logic out of lifecycle hooks --- .../mdc-list/list-base.ts | 43 +++++++++++++------ 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/src/material-experimental/mdc-list/list-base.ts b/src/material-experimental/mdc-list/list-base.ts index f8cac525783a..898f470e9f82 100644 --- a/src/material-experimental/mdc-list/list-base.ts +++ b/src/material-experimental/mdc-list/list-base.ts @@ -49,14 +49,8 @@ export abstract class MatListItemBase implements AfterContentInit, OnDestroy, Ri private _rippleRenderer: RippleRenderer; protected constructor(public _elementRef: ElementRef, protected _ngZone: NgZone, - listBase: MatListBase, platform: Platform) { - this.rippleDisabled = listBase._isNonInteractive; - if (!listBase._isNonInteractive) { - this._elementRef.nativeElement.classList.add('mat-mdc-list-item-interactive'); - } - this._rippleRenderer = - new RippleRenderer(this, this._ngZone, this._elementRef.nativeElement, platform); - this._rippleRenderer.setupTriggerEvents(this._elementRef.nativeElement); + private _listBase: MatListBase, private _platform: Platform) { + this._initRipple(); } ngAfterContentInit() { @@ -68,6 +62,23 @@ export abstract class MatListItemBase implements AfterContentInit, OnDestroy, Ri this._rippleRenderer._removeTriggerEvents(); } + _initDefaultTabIndex(tabIndex: number) { + const el = this._elementRef.nativeElement; + if (!el.hasAttribute('tabIndex')) { + el.tabIndex = tabIndex; + } + } + + private _initRipple() { + this.rippleDisabled = this._listBase._isNonInteractive; + if (!this._listBase._isNonInteractive) { + this._elementRef.nativeElement.classList.add('mat-mdc-list-item-interactive'); + } + this._rippleRenderer = + new RippleRenderer(this, this._ngZone, this._elementRef.nativeElement, this._platform); + this._rippleRenderer.setupTriggerEvents(this._elementRef.nativeElement); + } + /** * Subscribes to changes in `MatLine` content children and annotates them appropriately when they * change. @@ -177,13 +188,8 @@ export abstract class MatInteractiveListBase extends MatListBase } ngAfterViewInit() { - this._subscriptions.add( - this._items.changes.pipe(startWith(null)) - .subscribe(() => this._itemsArr = this._items.toArray())); + this._initItems(); this._foundation.init(); - for (let i = 0; this._items.length; i++) { - this._elementAtIndex(i).tabIndex = i === 0 ? 0 : -1; - } this._foundation.layout(); } @@ -192,6 +198,15 @@ export abstract class MatInteractiveListBase extends MatListBase this._subscriptions.unsubscribe(); } + private _initItems() { + this._subscriptions.add( + this._items.changes.pipe(startWith(null)) + .subscribe(() => this._itemsArr = this._items.toArray())); + for (let i = 0; this._itemsArr.length; i++) { + this._itemsArr[i]._initDefaultTabIndex(i === 0 ? 0 : -1); + } + } + private _itemAtIndex(index: number): MatListItemBase { return this._itemsArr[index]; }