Skip to content

fix(selection-list): restore focus if active item is destroyed #7125

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
Sep 29, 2017
Merged

fix(selection-list): restore focus if active item is destroyed #7125

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
50 changes: 30 additions & 20 deletions src/lib/list/selection-list.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {DOWN_ARROW, SPACE, UP_ARROW} from '@angular/cdk/keycodes';
import {Platform} from '@angular/cdk/platform';
import {createKeyboardEvent} from '@angular/cdk/testing';
import {createKeyboardEvent, dispatchFakeEvent} from '@angular/cdk/testing';
import {Component, DebugElement} from '@angular/core';
import {async, ComponentFixture, inject, TestBed} from '@angular/core/testing';
import {By} from '@angular/platform-browser';
Expand Down Expand Up @@ -28,24 +28,29 @@ describe('MatSelectionList', () => {
TestBed.compileComponents();
}));


beforeEach(async(() => {
fixture = TestBed.createComponent(SelectionListWithListOptions);
fixture.detectChanges();

listOption = fixture.debugElement.queryAll(By.directive(MatListOption));
listItemEl = fixture.debugElement.query(By.css('.mat-list-item'));
selectionList = fixture.debugElement.query(By.directive(MatSelectionList));
fixture.detectChanges();
}));

it('should add and remove focus class on focus/blur', () => {
expect(listItemEl.nativeElement.classList).not.toContain('mat-list-item-focus');
// Use the second list item, because the first one is always disabled.
const listItem = listOption[1].nativeElement;

listOption[0].componentInstance._handleFocus();
expect(listItem.classList).not.toContain('mat-list-item-focus');

dispatchFakeEvent(listItem, 'focus');
fixture.detectChanges();
expect(listItemEl.nativeElement.className).toContain('mat-list-item-focus');
expect(listItem.className).toContain('mat-list-item-focus');

listOption[0].componentInstance._handleBlur();
dispatchFakeEvent(listItem, 'blur');
fixture.detectChanges();
expect(listItemEl.nativeElement.className).not.toContain('mat-list-item-focus');
expect(listItem.className).not.toContain('mat-list-item-focus');
});

it('should be able to set a value on a list option', () => {
Expand Down Expand Up @@ -144,29 +149,36 @@ describe('MatSelectionList', () => {
createKeyboardEvent('keydown', SPACE, testListItem);
let selectList =
selectionList.injector.get<MatSelectionList>(MatSelectionList).selectedOptions;
let options = selectionList.componentInstance.options;
let array = options.toArray();
let focusItem = array[1];
expect(selectList.selected.length).toBe(0);

focusItem.focus();
dispatchFakeEvent(testListItem, 'focus');
selectionList.componentInstance._keydown(SPACE_EVENT);

fixture.detectChanges();

expect(selectList.selected.length).toBe(1);
});

it('should restore focus if active option is destroyed', () => {
const manager = selectionList.componentInstance._keyManager;

listOption[3].componentInstance._handleFocus();

expect(manager.activeItemIndex).toBe(3);

fixture.componentInstance.showLastOption = false;
fixture.detectChanges();

expect(manager.activeItemIndex).toBe(2);
});

it('should focus previous item when press UP ARROW', () => {
let testListItem = listOption[2].nativeElement as HTMLElement;
let UP_EVENT: KeyboardEvent =
createKeyboardEvent('keydown', UP_ARROW, testListItem);
let options = selectionList.componentInstance.options;
let array = options.toArray();
let focusItem = array[2];
let manager = selectionList.componentInstance._keyManager;

focusItem.focus();
dispatchFakeEvent(listOption[2].nativeElement, 'focus');
expect(manager.activeItemIndex).toEqual(2);

selectionList.componentInstance._keydown(UP_EVENT);
Expand All @@ -180,12 +192,9 @@ describe('MatSelectionList', () => {
let testListItem = listOption[2].nativeElement as HTMLElement;
let DOWN_EVENT: KeyboardEvent =
createKeyboardEvent('keydown', DOWN_ARROW, testListItem);
let options = selectionList.componentInstance.options;
let array = options.toArray();
let focusItem = array[2];
let manager = selectionList.componentInstance._keyManager;

focusItem.focus();
dispatchFakeEvent(listOption[2].nativeElement, 'focus');
expect(manager.activeItemIndex).toEqual(2);

selectionList.componentInstance._keydown(DOWN_EVENT);
Expand Down Expand Up @@ -404,11 +413,12 @@ describe('MatSelectionList', () => {
<mat-list-option checkboxPosition="before" value="sent-mail">
Sent Mail
</mat-list-option>
<mat-list-option checkboxPosition="before" value="drafts">
<mat-list-option checkboxPosition="before" value="drafts" *ngIf="showLastOption">
Drafts
</mat-list-option>
</mat-selection-list>`})
class SelectionListWithListOptions {
showLastOption: boolean = true;
}

@Component({template: `
Expand Down
87 changes: 25 additions & 62 deletions src/lib/list/selection-list.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import {FocusableOption, FocusKeyManager} from '@angular/cdk/a11y';
import {coerceBooleanProperty} from '@angular/cdk/coercion';
import {SelectionModel} from '@angular/cdk/collections';
import {SPACE} from '@angular/cdk/keycodes';
import {RxChain, startWith, switchMap} from '@angular/cdk/rxjs';
import {
AfterContentInit,
ChangeDetectionStrategy,
Expand All @@ -37,8 +36,6 @@ import {
mixinDisabled,
mixinDisableRipple,
} from '@angular/material/core';
import {merge} from 'rxjs/observable/merge';
import {Subscription} from 'rxjs/Subscription';


/** @docs-private */
Expand All @@ -54,8 +51,6 @@ export interface MatSelectionListOptionEvent {
option: MatListOption;
}

const FOCUSED_STYLE: string = 'mat-list-item-focus';

/**
* Component for list-options of selection-list. Each list-option can automatically
* generate a checkbox and can put current item into the selectionModel of selection-list
Expand All @@ -69,10 +64,11 @@ const FOCUSED_STYLE: string = 'mat-list-item-focus';
'role': 'option',
'class': 'mat-list-item mat-list-option',
'(focus)': '_handleFocus()',
'(blur)': '_handleBlur()',
'(blur)': '_hasFocus = false',
'(click)': '_handleClick()',
'tabindex': '-1',
'[class.mat-list-item-disabled]': 'disabled',
'[class.mat-list-item-focus]': '_hasFocus',
'[attr.aria-selected]': 'selected.toString()',
'[attr.aria-disabled]': 'disabled.toString()',
},
Expand Down Expand Up @@ -109,18 +105,12 @@ export class MatListOption extends _MatListOptionMixinBase
get selected() { return this._selected; }
set selected(value: boolean) { this._selected = coerceBooleanProperty(value); }

/** Emitted when the option is focused. */
onFocus = new EventEmitter<MatSelectionListOptionEvent>();

/** Emitted when the option is selected. */
@Output() selectChange = new EventEmitter<MatSelectionListOptionEvent>();

/** Emitted when the option is deselected. */
@Output() deselected = new EventEmitter<MatSelectionListOptionEvent>();

/** Emitted when the option is destroyed. */
@Output() destroyed = new EventEmitter<MatSelectionListOptionEvent>();

constructor(private _renderer: Renderer2,
private _element: ElementRef,
private _changeDetector: ChangeDetectorRef,
Expand All @@ -138,7 +128,7 @@ export class MatListOption extends _MatListOptionMixinBase
}

ngOnDestroy(): void {
this.destroyed.emit({option: this});
this.selectionList._removeOptionFromList(this);
}

/** Toggles the selection state of the option. */
Expand All @@ -151,7 +141,6 @@ export class MatListOption extends _MatListOptionMixinBase
/** Allows for programmatic focusing of the option. */
focus(): void {
this._element.nativeElement.focus();
this.onFocus.emit({option: this});
}

/** Whether this list item should show a ripple effect when clicked. */
Expand All @@ -167,11 +156,7 @@ export class MatListOption extends _MatListOptionMixinBase

_handleFocus() {
this._hasFocus = true;
this._renderer.addClass(this._element.nativeElement, FOCUSED_STYLE);
}

_handleBlur() {
this._renderer.removeClass(this._element.nativeElement, FOCUSED_STYLE);
this.selectionList._setFocusedOption(this);
}

/** Retrieves the DOM element of the component host. */
Expand Down Expand Up @@ -202,17 +187,11 @@ export class MatListOption extends _MatListOptionMixinBase
changeDetection: ChangeDetectionStrategy.OnPush
})
export class MatSelectionList extends _MatSelectionListMixinBase
implements FocusableOption, CanDisable, CanDisableRipple, AfterContentInit, OnDestroy {
implements FocusableOption, CanDisable, CanDisableRipple, AfterContentInit {

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

/** Subscription to all list options' onFocus events */
private _optionFocusSubscription = Subscription.EMPTY;

/** Subscription to all list options' destroy events */
private _optionDestroyStream = Subscription.EMPTY;

/** The FocusKeyManager which handles focus. */
_keyManager: FocusKeyManager<MatListOption>;

Expand All @@ -232,14 +211,6 @@ export class MatSelectionList extends _MatSelectionListMixinBase
if (this.disabled) {
this._tabIndex = -1;
}

this._optionFocusSubscription = this._onFocusSubscription();
this._optionDestroyStream = this._onDestroySubscription();
}

ngOnDestroy(): void {
this._optionDestroyStream.unsubscribe();
this._optionFocusSubscription.unsubscribe();
}

/** Focus the selection-list. */
Expand All @@ -265,36 +236,23 @@ export class MatSelectionList extends _MatSelectionListMixinBase
});
}

/** Map all the options' destroy event subscriptions and merge them into one stream. */
private _onDestroySubscription(): Subscription {
return RxChain.from(this.options.changes)
.call(startWith, this.options)
.call(switchMap, (options: MatListOption[]) => {
return merge(...options.map(option => option.destroyed));
}).subscribe((e: MatSelectionListOptionEvent) => {
let optionIndex: number = this.options.toArray().indexOf(e.option);
if (e.option._hasFocus) {
// Check whether the option is the last item
if (optionIndex < this.options.length - 1) {
this._keyManager.setActiveItem(optionIndex);
} else if (optionIndex - 1 >= 0) {
this._keyManager.setActiveItem(optionIndex - 1);
}
}
e.option.destroyed.unsubscribe();
});
/** Sets the focused option of the selection-list. */
_setFocusedOption(option: MatListOption) {
this._keyManager.updateActiveItemIndex(this._getOptionIndex(option));
}

/** Map all the options' onFocus event subscriptions and merge them into one stream. */
private _onFocusSubscription(): Subscription {
return RxChain.from(this.options.changes)
.call(startWith, this.options)
.call(switchMap, (options: MatListOption[]) => {
return merge(...options.map(option => option.onFocus));
}).subscribe((e: MatSelectionListOptionEvent) => {
let optionIndex: number = this.options.toArray().indexOf(e.option);
this._keyManager.updateActiveItemIndex(optionIndex);
});
/** Removes an option from the selection list and updates the active item. */
_removeOptionFromList(option: MatListOption) {
if (option._hasFocus) {
const optionIndex = this._getOptionIndex(option);

// Check whether the option is the last item
if (optionIndex > 0) {
this._keyManager.setPreviousItemActive();
} else if (optionIndex === 0 && this.options.length > 1) {
this._keyManager.setNextItemActive();
}
}
}

/** Passes relevant key presses to our key manager. */
Expand Down Expand Up @@ -332,4 +290,9 @@ export class MatSelectionList extends _MatSelectionListMixinBase
private _isValidIndex(index: number): boolean {
return index >= 0 && index < this.options.length;
}

/** Returns the index of the specified list option. */
private _getOptionIndex(option: MatListOption): number {
return this.options.toArray().indexOf(option);
}
}