Skip to content

Commit 721d0b3

Browse files
committed
fixup! feat(material-experimental/mdc-list): implement selection list
Address feedback. Cleanup code and circular dependency.
1 parent 3a0e6fa commit 721d0b3

File tree

12 files changed

+119
-68
lines changed

12 files changed

+119
-68
lines changed

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,10 @@ import {MatListBase} from './list-base';
2424
]
2525
})
2626
export class MatActionList extends MatListBase {
27-
// An action list is considered interactive, but does not extend the interactive
28-
// list base class. We do this because as per MDC, items of interactive lists are
29-
// only reachable through keyboard shortcuts. We want all items for the action list
30-
// to be reachable through tab key though. This ensures best accessibility.
27+
// An navigation list is considered interactive, but does not extend the interactive list
28+
// base class. We do this because as per MDC, items of interactive lists are only reachable
29+
// through keyboard shortcuts. We want all items for the navigation list to be reachable
30+
// through tab key as we do not intend to provide any special accessibility treatment. The
31+
// accessibility treatment depends on how the end-user will interact with it.
3132
_isNonInteractive = false;
3233
}

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

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ export abstract class MatInteractiveListBase<T extends MatListItemBase>
4747
@HostListener('focusin', ['$event'])
4848
_handleFocusin(event: FocusEvent) {
4949
const itemIndex = this._indexForElement(event.target as HTMLElement);
50-
const tabIndex = this._itemsArr[itemIndex]?._getHostElement().tabIndex;
50+
const tabIndex = this._itemsArr[itemIndex]?._hostElement.tabIndex;
5151

5252
// If the newly focused item is not the designated item that should have received focus
5353
// first through keyboard interaction, the tabindex of the previously designated list item
@@ -128,7 +128,7 @@ export abstract class MatInteractiveListBase<T extends MatListItemBase>
128128
*/
129129
private _clearTabindexForAllItems() {
130130
for (let items of this._itemsArr) {
131-
items._elementRef.nativeElement.setAttribute('tabindex', '-1');
131+
items._hostElement.setAttribute('tabindex', '-1');
132132
}
133133
}
134134

@@ -138,16 +138,20 @@ export abstract class MatInteractiveListBase<T extends MatListItemBase>
138138
*/
139139
protected _resetTabindexToFirstSelectedOrFocusedItem() {
140140
this._clearTabindexForAllItems();
141-
(this._foundation as any)['setTabindexToFirstSelectedOrFocusedItem']();
141+
// MDC does not expose the method for setting the tabindex to the first selected
142+
// or previously focused item. We can still access the method as private class
143+
// members are accessible in the transpiled JavaScript. Tracked upstream with:
144+
// TODO: https://github.com/material-components/material-components-web/issues/6375
145+
(this._foundation as any).setTabindexToFirstSelectedOrFocusedItem();
142146
}
143147

144148
_elementAtIndex(index: number): HTMLElement|undefined {
145-
return this._itemsArr[index]?._elementRef.nativeElement;
149+
return this._itemsArr[index]?._hostElement;
146150
}
147151

148152
_indexForElement(element: Element | null): number {
149153
return element ?
150-
this._itemsArr.findIndex(i => i._elementRef.nativeElement.contains(element)) : -1;
154+
this._itemsArr.findIndex(i => i._hostElement.contains(element)) : -1;
151155
}
152156
}
153157

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

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,9 @@ export abstract class MatListItemBase implements AfterContentInit, OnDestroy, Ri
3939
/** Element reference referring to the primary list item text. */
4040
abstract _itemText: ElementRef<HTMLElement>;
4141

42+
/** Host element for the list item. */
43+
_hostElement: HTMLElement;
44+
4245
@Input()
4346
get disableRipple(): boolean {
4447
return this.disabled || this._disableRipple || this._listBase.disableRipple;
@@ -71,22 +74,23 @@ export abstract class MatListItemBase implements AfterContentInit, OnDestroy, Ri
7174

7275
constructor(public _elementRef: ElementRef<HTMLElement>, protected _ngZone: NgZone,
7376
private _listBase: MatListBase, private _platform: Platform) {
77+
this._hostElement = this._elementRef.nativeElement;
78+
7479
if (!this._listBase._isNonInteractive) {
7580
this._initInteractiveListItem();
7681
}
7782

7883
// Only interactive list items are commonly focusable, but in some situations,
7984
// consumers provide a custom tabindex. We still would want to have strong focus
8085
// indicator support in such scenarios.
81-
this._elementRef.nativeElement.classList.add('mat-mdc-focus-indicator');
86+
this._hostElement.classList.add('mat-mdc-focus-indicator');
8287

83-
// If no type attributed is specified for a host `<button>` element, set it to
84-
// "button". If a type attribute is already specified, do nothing. We do this
85-
// for backwards compatibility with the old list.
88+
// If no type attribute is specified for a host `<button>` element, set it to `button`. If a
89+
// type attribute is already specified, we do nothing. We do this for backwards compatibility.
8690
// TODO: Determine if we intend to continue doing this for the MDC-based list.
87-
const element = _elementRef.nativeElement;
88-
if (element.nodeName.toLowerCase() === 'button' && !element.hasAttribute('type')) {
89-
element.setAttribute('type', 'button');
91+
if (this._hostElement.nodeName.toLowerCase() === 'button' &&
92+
!this._hostElement.hasAttribute('type')) {
93+
this._hostElement.setAttribute('type', 'button');
9094
}
9195
}
9296

@@ -106,16 +110,11 @@ export abstract class MatListItemBase implements AfterContentInit, OnDestroy, Ri
106110
return this._itemText ? (this._itemText.nativeElement.textContent || '') : '';
107111
}
108112

109-
/** Gets the host element of the list item. */
110-
_getHostElement(): HTMLElement {
111-
return this._elementRef.nativeElement;
112-
}
113-
114113
private _initInteractiveListItem() {
115-
this._elementRef.nativeElement.classList.add('mat-mdc-list-item-interactive');
114+
this._hostElement.classList.add('mat-mdc-list-item-interactive');
116115
this._rippleRenderer =
117-
new RippleRenderer(this, this._ngZone, this._elementRef.nativeElement, this._platform);
118-
this._rippleRenderer.setupTriggerEvents(this._elementRef.nativeElement);
116+
new RippleRenderer(this, this._ngZone, this._hostElement, this._platform);
117+
this._rippleRenderer.setupTriggerEvents(this._hostElement);
119118
}
120119

121120
/**
@@ -126,8 +125,7 @@ export abstract class MatListItemBase implements AfterContentInit, OnDestroy, Ri
126125
this._ngZone.runOutsideAngular(() => {
127126
this._subscriptions.add(this.lines.changes.pipe(startWith(this.lines))
128127
.subscribe((lines: QueryList<ElementRef<Element>>) => {
129-
this._elementRef.nativeElement.classList
130-
.toggle('mat-mdc-list-item-single-line', lines.length <= 1);
128+
toggleClass(this._hostElement, 'mat-mdc-list-item-single-line', lines.length <= 1);
131129
lines.forEach((line: ElementRef<Element>, index: number) => {
132130
toggleClass(line.nativeElement,
133131
'mdc-list-item__primary-text', index === 0 && lines.length > 1);

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

Lines changed: 38 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,16 @@
77
*/
88

99
import {BooleanInput, coerceBooleanProperty} from '@angular/cdk/coercion';
10+
import {SelectionModel} from '@angular/cdk/collections';
1011
import {Platform} from '@angular/cdk/platform';
1112
import {
1213
ChangeDetectionStrategy,
1314
ChangeDetectorRef,
1415
Component,
1516
ContentChildren,
1617
ElementRef,
18+
Inject,
19+
InjectionToken,
1720
Input,
1821
NgZone,
1922
OnDestroy,
@@ -24,8 +27,29 @@ import {
2427
} from '@angular/core';
2528
import {MatLine, ThemePalette} from '@angular/material/core';
2629
import {MatListAvatarCssMatStyler, MatListIconCssMatStyler} from './list';
27-
import {MatListItemBase} from './list-base';
28-
import {MAT_LIST_OPTION, MatSelectionList} from './selection-list';
30+
import {MatListBase, MatListItemBase} from './list-base';
31+
32+
/**
33+
* Injection token that can be used to reference instances of an `SelectionList`. It serves
34+
* as alternative token to an actual implementation which would result in circular references.
35+
* @docs-private
36+
*/
37+
export const SELECTION_LIST = new InjectionToken<SelectionList>('SelectionList');
38+
39+
/**
40+
* Interface describing the containing list of an list option. This is used to avoid
41+
* circular dependencies between the list-option and the selection list.
42+
* @docs-private
43+
*/
44+
export interface SelectionList extends MatListBase {
45+
multiple: boolean;
46+
color: ThemePalette;
47+
selectedOptions: SelectionModel<MatListOption>;
48+
compareWith: (o1: any, o2: any) => boolean;
49+
_value: string[]|null;
50+
_reportValueChange: () => void;
51+
_onTouched: () => void;
52+
}
2953

3054
/** Unique id for created list options. */
3155
let uniqueId = 0;
@@ -39,7 +63,7 @@ let uniqueId = 0;
3963
'role': 'option',
4064
// As per MDC, only list items in single selection mode should receive the `--selected`
4165
// class. For multi selection, the checkbox is used as indicator.
42-
'[class.mdc-list-item--selected]': 'selected && !selectionList.multiple',
66+
'[class.mdc-list-item--selected]': 'selected && !_selectionList.multiple',
4367
'[class.mat-mdc-list-item-with-avatar]': '_hasIconOrAvatar()',
4468
'[class.mat-accent]': 'color !== "primary" && color !== "warn"',
4569
'[class.mat-warn]': 'color === "warn"',
@@ -49,7 +73,6 @@ let uniqueId = 0;
4973
encapsulation: ViewEncapsulation.None,
5074
changeDetection: ChangeDetectionStrategy.OnPush,
5175
providers: [
52-
{provide: MAT_LIST_OPTION, useExisting: MatListOption},
5376
{provide: MatListItemBase, useExisting: MatListOption},
5477
]
5578
})
@@ -75,7 +98,7 @@ export class MatListOption extends MatListItemBase implements OnInit, OnDestroy
7598

7699
/** Theme color of the list option. This sets the color of the checkbox. */
77100
@Input()
78-
get color(): ThemePalette { return this._color || this.selectionList.color; }
101+
get color(): ThemePalette { return this._color || this._selectionList.color; }
79102
set color(newValue: ThemePalette) { this._color = newValue; }
80103
private _color: ThemePalette;
81104

@@ -93,13 +116,13 @@ export class MatListOption extends MatListItemBase implements OnInit, OnDestroy
93116

94117
/** Whether the option is selected. */
95118
@Input()
96-
get selected(): boolean { return this.selectionList.selectedOptions.isSelected(this); }
119+
get selected(): boolean { return this._selectionList.selectedOptions.isSelected(this); }
97120
set selected(value: boolean) {
98121
const isSelected = coerceBooleanProperty(value);
99122

100123
if (isSelected !== this._selected) {
101124
this._setSelected(isSelected);
102-
this.selectionList._reportValueChange();
125+
this._selectionList._reportValueChange();
103126
}
104127
}
105128
private _selected = false;
@@ -108,9 +131,9 @@ export class MatListOption extends MatListItemBase implements OnInit, OnDestroy
108131
element: ElementRef,
109132
ngZone: NgZone,
110133
platform: Platform,
111-
public selectionList: MatSelectionList,
134+
@Inject(SELECTION_LIST) public _selectionList: SelectionList,
112135
private _changeDetectorRef: ChangeDetectorRef) {
113-
super(element, ngZone, selectionList, platform);
136+
super(element, ngZone, _selectionList, platform);
114137

115138
// By default, we mark all options as unselected. The MDC list foundation will
116139
// automatically update the attribute based on selection. Note that we need to
@@ -120,7 +143,7 @@ export class MatListOption extends MatListItemBase implements OnInit, OnDestroy
120143
}
121144

122145
ngOnInit() {
123-
const list = this.selectionList;
146+
const list = this._selectionList;
124147

125148
if (list._value && list._value.some(value => list.compareWith(value, this._value))) {
126149
this._setSelected(true);
@@ -159,7 +182,7 @@ export class MatListOption extends MatListItemBase implements OnInit, OnDestroy
159182

160183
/** Allows for programmatic focusing of the option. */
161184
focus(): void {
162-
this._elementRef.nativeElement.focus();
185+
this._hostElement.focus();
163186
}
164187

165188
_isReversed(): boolean {
@@ -168,7 +191,7 @@ export class MatListOption extends MatListItemBase implements OnInit, OnDestroy
168191

169192
/** Whether the list-option has a checkbox. */
170193
_hasCheckbox() {
171-
return this.selectionList.multiple;
194+
return this._selectionList.multiple;
172195
}
173196

174197
/** Whether the list-option has icons or avatars. */
@@ -177,7 +200,7 @@ export class MatListOption extends MatListItemBase implements OnInit, OnDestroy
177200
}
178201

179202
_handleBlur() {
180-
this.selectionList._onTouched();
203+
this._selectionList._onTouched();
181204
}
182205

183206
/**
@@ -192,9 +215,9 @@ export class MatListOption extends MatListItemBase implements OnInit, OnDestroy
192215
this._selected = selected;
193216

194217
if (selected) {
195-
this.selectionList.selectedOptions.select(this);
218+
this._selectionList.selectedOptions.select(this);
196219
} else {
197-
this.selectionList.selectedOptions.deselect(this);
220+
this._selectionList.selectedOptions.deselect(this);
198221
}
199222

200223
this._changeDetectorRef.markForCheck();

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ describe('MDC-based MatList', () => {
107107
expect(items.length).toBeGreaterThan(0);
108108

109109
items.forEach(item => {
110-
dispatchMouseEvent(item._elementRef.nativeElement, 'mousedown');
110+
dispatchMouseEvent(item._hostElement, 'mousedown');
111111
expect(fixture.nativeElement.querySelector('.mat-ripple-element')).toBe(null);
112112
});
113113
}));

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,10 @@ import {MatListBase} from './list-base';
2424
]
2525
})
2626
export class MatNavList extends MatListBase {
27-
// An navigation list is considered interactive, but does not extend the interactive
28-
// list base class. We do this because as per MDC, items of interactive lists are
29-
// only reachable through keyboard shortcuts. We want all items for the navigation list
30-
// to be reachable through tab key though. This ensures best accessibility.
27+
// An navigation list is considered interactive, but does not extend the interactive list
28+
// base class. We do this because as per MDC, items of interactive lists are only reachable
29+
// through keyboard shortcuts. We want all items for the navigation list to be reachable
30+
// through tab key as we do not intend to provide any special accessibility treatment. The
31+
// accessibility treatment depends on how the end-user will interact with it.
3132
_isNonInteractive = false;
3233
}

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ describe('MDC-based MatSelectionList without forms', () => {
189189
it('should not add the mdc-list-item--selected class (in multiple mode)', () => {
190190
let testListItem = listOptions[2].injector.get<MatListOption>(MatListOption);
191191

192-
dispatchMouseEvent(testListItem._elementRef.nativeElement, 'click');
192+
dispatchMouseEvent(testListItem._hostElement, 'click');
193193
fixture.detectChanges();
194194

195195
expect(listOptions[2].nativeElement.classList.contains('mdc-list-item--selected'))
@@ -204,7 +204,7 @@ describe('MDC-based MatSelectionList without forms', () => {
204204
expect(selectList.selected.length).toBe(0);
205205
expect(listOptions[0].nativeElement.getAttribute('aria-disabled')).toBe('true');
206206

207-
dispatchMouseEvent(testListItem._elementRef.nativeElement, 'click');
207+
dispatchMouseEvent(testListItem._hostElement, 'click');
208208
fixture.detectChanges();
209209

210210
expect(selectList.selected.length).toBe(0);
@@ -843,7 +843,7 @@ describe('MDC-based MatSelectionList without forms', () => {
843843

844844
expect(selectList.selected.length).toBe(0);
845845

846-
dispatchMouseEvent(testListItem._elementRef.nativeElement, 'click');
846+
dispatchMouseEvent(testListItem._hostElement, 'click');
847847
fixture.detectChanges();
848848

849849
expect(selectList.selected.length).toBe(0);
@@ -953,14 +953,14 @@ describe('MDC-based MatSelectionList without forms', () => {
953953

954954
expect(selectList.selected.length).toBe(0);
955955

956-
dispatchMouseEvent(testListItem1._elementRef.nativeElement, 'click');
956+
dispatchMouseEvent(testListItem1._hostElement, 'click');
957957
fixture.detectChanges();
958958

959959
expect(selectList.selected).toEqual([testListItem1]);
960960
expect(listOptions[1].nativeElement.classList.contains('mdc-list-item--selected'))
961961
.toBe(true);
962962

963-
dispatchMouseEvent(testListItem2._elementRef.nativeElement, 'click');
963+
dispatchMouseEvent(testListItem2._hostElement, 'click');
964964
fixture.detectChanges();
965965

966966
expect(selectList.selected).toEqual([testListItem2]);
@@ -981,12 +981,12 @@ describe('MDC-based MatSelectionList without forms', () => {
981981

982982
expect(selectList.selected.length).toBe(0);
983983

984-
dispatchMouseEvent(testListItem1._elementRef.nativeElement, 'click');
984+
dispatchMouseEvent(testListItem1._hostElement, 'click');
985985
fixture.detectChanges();
986986

987987
expect(selectList.selected).toEqual([testListItem1]);
988988

989-
dispatchMouseEvent(testListItem1._elementRef.nativeElement, 'click');
989+
dispatchMouseEvent(testListItem1._hostElement, 'click');
990990
fixture.detectChanges();
991991

992992
expect(selectList.selected).toEqual([testListItem1]);
@@ -1095,7 +1095,7 @@ describe('MDC-based MatSelectionList with forms', () => {
10951095
expect(fixture.componentInstance.selectedOptions.length)
10961096
.toBe(0, 'Expected no options to be selected by default');
10971097

1098-
dispatchMouseEvent(listOptions[0]._elementRef.nativeElement, 'click');
1098+
dispatchMouseEvent(listOptions[0]._hostElement, 'click');
10991099
fixture.detectChanges();
11001100

11011101
tick();

0 commit comments

Comments
 (0)