Skip to content

Commit b52a405

Browse files
committed
fix(selection-list): restore focus if active item is destroyed
* Removes the `destroyed` and `onFocus` observables and instead just calls methods on the injected `MdSelectionList` instance. * Fixes that the active item is not updating if the active item is being destroyed.
1 parent 34b5620 commit b52a405

File tree

2 files changed

+45
-63
lines changed

2 files changed

+45
-63
lines changed

src/lib/list/selection-list.spec.ts

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,11 @@ describe('MdSelectionList', () => {
3030

3131
beforeEach(async(() => {
3232
fixture = TestBed.createComponent(SelectionListWithListOptions);
33+
fixture.detectChanges();
34+
3335
listOption = fixture.debugElement.queryAll(By.directive(MdListOption));
3436
listItemEl = fixture.debugElement.query(By.css('.mat-list-item'));
3537
selectionList = fixture.debugElement.query(By.directive(MdSelectionList));
36-
fixture.detectChanges();
3738
}));
3839

3940
it('should add and remove focus class on focus/blur', () => {
@@ -152,6 +153,21 @@ describe('MdSelectionList', () => {
152153
expect(selectList.selected.length).toBe(1);
153154
});
154155

156+
it('should restore focus if active option is destroyed', () => {
157+
const manager = selectionList.componentInstance._keyManager;
158+
159+
listOption[3].componentInstance._handleFocus();
160+
161+
expect(manager.activeItemIndex).toBe(3);
162+
expect(listOption[3].componentInstance._hasFocus).toBe(true);
163+
164+
fixture.componentInstance.showLastOption = false;
165+
fixture.detectChanges();
166+
167+
expect(manager.activeItemIndex).toBe(2);
168+
expect(listOption[2].componentInstance._hasFocus).toBe(true);
169+
});
170+
155171
it('should focus previous item when press UP ARROW', () => {
156172
let testListItem = listOption[2].nativeElement as HTMLElement;
157173
let UP_EVENT: KeyboardEvent =
@@ -398,11 +414,12 @@ describe('MdSelectionList', () => {
398414
<md-list-option checkboxPosition="before" value="sent-mail">
399415
Sent Mail
400416
</md-list-option>
401-
<md-list-option checkboxPosition="before" value="drafts">
417+
<md-list-option checkboxPosition="before" value="drafts" *ngIf="showLastOption">
402418
Drafts
403419
</md-list-option>
404420
</mat-selection-list>`})
405421
class SelectionListWithListOptions {
422+
showLastOption: boolean = true;
406423
}
407424

408425
@Component({template: `

src/lib/list/selection-list.ts

Lines changed: 26 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -35,13 +35,8 @@ import {
3535
MdLineSetter,
3636
mixinDisabled,
3737
mixinDisableRipple,
38-
RxChain,
3938
SPACE,
40-
startWith,
41-
switchMap,
4239
} from '@angular/material/core';
43-
import {merge} from 'rxjs/observable/merge';
44-
import {Subscription} from 'rxjs/Subscription';
4540

4641

4742
/** @docs-private */
@@ -57,8 +52,6 @@ export interface MdSelectionListOptionEvent {
5752
option: MdListOption;
5853
}
5954

60-
const FOCUSED_STYLE: string = 'mat-list-item-focus';
61-
6255
/**
6356
* Component for list-options of selection-list. Each list-option can automatically
6457
* generate a checkbox and can put current item into the selectionModel of selection-list
@@ -76,6 +69,7 @@ const FOCUSED_STYLE: string = 'mat-list-item-focus';
7669
'(click)': '_handleClick()',
7770
'tabindex': '-1',
7871
'[class.mat-list-item-disabled]': 'disabled',
72+
'[class.mat-list-item-focus]': '_hasFocus',
7973
'[attr.aria-selected]': 'selected.toString()',
8074
'[attr.aria-disabled]': 'disabled.toString()',
8175
},
@@ -112,18 +106,12 @@ export class MdListOption extends _MdListOptionMixinBase
112106
get selected() { return this._selected; }
113107
set selected(value: boolean) { this._selected = coerceBooleanProperty(value); }
114108

115-
/** Emitted when the option is focused. */
116-
onFocus = new EventEmitter<MdSelectionListOptionEvent>();
117-
118109
/** Emitted when the option is selected. */
119110
@Output() selectChange = new EventEmitter<MdSelectionListOptionEvent>();
120111

121112
/** Emitted when the option is deselected. */
122113
@Output() deselected = new EventEmitter<MdSelectionListOptionEvent>();
123-
124-
/** Emitted when the option is destroyed. */
125-
@Output() destroyed = new EventEmitter<MdSelectionListOptionEvent>();
126-
114+
127115
constructor(private _renderer: Renderer2,
128116
private _element: ElementRef,
129117
private _changeDetector: ChangeDetectorRef,
@@ -141,7 +129,7 @@ export class MdListOption extends _MdListOptionMixinBase
141129
}
142130

143131
ngOnDestroy(): void {
144-
this.destroyed.emit({option: this});
132+
this.selectionList._removeOptionFromList(this);
145133
}
146134

147135
/** Toggles the selection state of the option. */
@@ -154,7 +142,6 @@ export class MdListOption extends _MdListOptionMixinBase
154142
/** Allows for programmatic focusing of the option. */
155143
focus(): void {
156144
this._element.nativeElement.focus();
157-
this.onFocus.emit({option: this});
158145
}
159146

160147
/** Whether this list item should show a ripple effect when clicked. */
@@ -170,11 +157,11 @@ export class MdListOption extends _MdListOptionMixinBase
170157

171158
_handleFocus() {
172159
this._hasFocus = true;
173-
this._renderer.addClass(this._element.nativeElement, FOCUSED_STYLE);
160+
this.selectionList._setFocusedOption(this);
174161
}
175162

176163
_handleBlur() {
177-
this._renderer.removeClass(this._element.nativeElement, FOCUSED_STYLE);
164+
this._hasFocus = false;
178165
}
179166

180167
/** Retrieves the DOM element of the component host. */
@@ -204,17 +191,11 @@ export class MdListOption extends _MdListOptionMixinBase
204191
changeDetection: ChangeDetectionStrategy.OnPush
205192
})
206193
export class MdSelectionList extends _MdSelectionListMixinBase
207-
implements FocusableOption, CanDisable, CanDisableRipple, AfterContentInit, OnDestroy {
194+
implements FocusableOption, CanDisable, CanDisableRipple, AfterContentInit {
208195

209196
/** Tab index for the selection-list. */
210197
_tabIndex = 0;
211198

212-
/** Subscription to all list options' onFocus events */
213-
private _optionFocusSubscription = Subscription.EMPTY;
214-
215-
/** Subscription to all list options' destroy events */
216-
private _optionDestroyStream = Subscription.EMPTY;
217-
218199
/** The FocusKeyManager which handles focus. */
219200
_keyManager: FocusKeyManager<MdListOption>;
220201

@@ -234,14 +215,6 @@ export class MdSelectionList extends _MdSelectionListMixinBase
234215
if (this.disabled) {
235216
this._tabIndex = -1;
236217
}
237-
238-
this._optionFocusSubscription = this._onFocusSubscription();
239-
this._optionDestroyStream = this._onDestroySubscription();
240-
}
241-
242-
ngOnDestroy(): void {
243-
this._optionDestroyStream.unsubscribe();
244-
this._optionFocusSubscription.unsubscribe();
245218
}
246219

247220
/** Focus the selection-list. */
@@ -267,36 +240,23 @@ export class MdSelectionList extends _MdSelectionListMixinBase
267240
});
268241
}
269242

270-
/** Map all the options' destroy event subscriptions and merge them into one stream. */
271-
private _onDestroySubscription(): Subscription {
272-
return RxChain.from(this.options.changes)
273-
.call(startWith, this.options)
274-
.call(switchMap, (options: MdListOption[]) => {
275-
return merge(...options.map(option => option.destroyed));
276-
}).subscribe((e: MdSelectionListOptionEvent) => {
277-
let optionIndex: number = this.options.toArray().indexOf(e.option);
278-
if (e.option._hasFocus) {
279-
// Check whether the option is the last item
280-
if (optionIndex < this.options.length - 1) {
281-
this._keyManager.setActiveItem(optionIndex);
282-
} else if (optionIndex - 1 >= 0) {
283-
this._keyManager.setActiveItem(optionIndex - 1);
284-
}
285-
}
286-
e.option.destroyed.unsubscribe();
287-
});
243+
/** Sets the focused option of the selection-list. */
244+
_setFocusedOption(option: MdListOption) {
245+
this._keyManager.updateActiveItemIndex(this._getIndexFromOption(option));
288246
}
289247

290-
/** Map all the options' onFocus event subscriptions and merge them into one stream. */
291-
private _onFocusSubscription(): Subscription {
292-
return RxChain.from(this.options.changes)
293-
.call(startWith, this.options)
294-
.call(switchMap, (options: MdListOption[]) => {
295-
return merge(...options.map(option => option.onFocus));
296-
}).subscribe((e: MdSelectionListOptionEvent) => {
297-
let optionIndex: number = this.options.toArray().indexOf(e.option);
298-
this._keyManager.updateActiveItemIndex(optionIndex);
299-
});
248+
/** Removes an option from the selection list and updates the active item. */
249+
_removeOptionFromList(option: MdListOption) {
250+
if (option._hasFocus) {
251+
const optionIndex = this._getIndexFromOption(option);
252+
253+
// Check whether the option is the last item
254+
if (optionIndex - 1 >= 0) {
255+
this._keyManager.setPreviousItemActive();
256+
} else if (optionIndex < this.options.length - 1) {
257+
this._keyManager.setNextItemActive();
258+
}
259+
}
300260
}
301261

302262
/** Passes relevant key presses to our key manager. */
@@ -334,4 +294,9 @@ export class MdSelectionList extends _MdSelectionListMixinBase
334294
private _isValidIndex(index: number): boolean {
335295
return index >= 0 && index < this.options.length;
336296
}
297+
298+
/** Returns the index of the specified list option. */
299+
private _getIndexFromOption(option: MdListOption): number {
300+
return this.options.toArray().indexOf(option);
301+
}
337302
}

0 commit comments

Comments
 (0)