From 8ad9feccdc024483ba42be26a5127d5a70ed7469 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Sun, 17 Sep 2017 16:51:02 +0200 Subject: [PATCH] Address feedback --- src/lib/list/selection-list.spec.ts | 23 ++++++----------------- src/lib/list/selection-list.ts | 16 ++++++---------- 2 files changed, 12 insertions(+), 27 deletions(-) diff --git a/src/lib/list/selection-list.spec.ts b/src/lib/list/selection-list.spec.ts index 8192eafeafbe..1df8ed0963d2 100644 --- a/src/lib/list/selection-list.spec.ts +++ b/src/lib/list/selection-list.spec.ts @@ -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'; @@ -40,11 +40,11 @@ describe('MatSelectionList', () => { it('should add and remove focus class on focus/blur', () => { expect(listItemEl.nativeElement.classList).not.toContain('mat-list-item-focus'); - listOption[0].componentInstance._handleFocus(); + dispatchFakeEvent(listOption[0].nativeElement, 'focus'); fixture.detectChanges(); expect(listItemEl.nativeElement.className).toContain('mat-list-item-focus'); - listOption[0].componentInstance._handleBlur(); + dispatchFakeEvent(listOption[0].nativeElement, 'blur'); fixture.detectChanges(); expect(listItemEl.nativeElement.className).not.toContain('mat-list-item-focus'); }); @@ -145,12 +145,9 @@ describe('MatSelectionList', () => { createKeyboardEvent('keydown', SPACE, testListItem); let selectList = selectionList.injector.get(MatSelectionList).selectedOptions; - let options = selectionList.componentInstance.options; - let array = options.toArray(); - let focusItem = array[1]; expect(selectList.selected.length).toBe(0); - focusItem.focus(); + dispatchFakeEvent(listOption[0].nativeElement, 'focus'); selectionList.componentInstance._keydown(SPACE_EVENT); fixture.detectChanges(); @@ -164,25 +161,20 @@ describe('MatSelectionList', () => { listOption[3].componentInstance._handleFocus(); expect(manager.activeItemIndex).toBe(3); - expect(listOption[3].componentInstance._hasFocus).toBe(true); fixture.componentInstance.showLastOption = false; fixture.detectChanges(); expect(manager.activeItemIndex).toBe(2); - expect(listOption[2].componentInstance._hasFocus).toBe(true); }); 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); @@ -196,12 +188,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); diff --git a/src/lib/list/selection-list.ts b/src/lib/list/selection-list.ts index bb31fdf240ac..1b6b466c70eb 100644 --- a/src/lib/list/selection-list.ts +++ b/src/lib/list/selection-list.ts @@ -64,7 +64,7 @@ export interface MatSelectionListOptionEvent { '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', @@ -159,10 +159,6 @@ export class MatListOption extends _MatListOptionMixinBase this.selectionList._setFocusedOption(this); } - _handleBlur() { - this._hasFocus = false; - } - /** Retrieves the DOM element of the component host. */ _getHostElement(): HTMLElement { return this._element.nativeElement; @@ -242,18 +238,18 @@ export class MatSelectionList extends _MatSelectionListMixinBase /** Sets the focused option of the selection-list. */ _setFocusedOption(option: MatListOption) { - this._keyManager.updateActiveItemIndex(this._getIndexFromOption(option)); + this._keyManager.updateActiveItemIndex(this._getOptionIndex(option)); } /** Removes an option from the selection list and updates the active item. */ _removeOptionFromList(option: MatListOption) { if (option._hasFocus) { - const optionIndex = this._getIndexFromOption(option); + const optionIndex = this._getOptionIndex(option); // Check whether the option is the last item - if (optionIndex - 1 >= 0) { + if (optionIndex > 0) { this._keyManager.setPreviousItemActive(); - } else if (optionIndex < this.options.length - 1) { + } else if (optionIndex === 0 && this.options.length > 1) { this._keyManager.setNextItemActive(); } } @@ -296,7 +292,7 @@ export class MatSelectionList extends _MatSelectionListMixinBase } /** Returns the index of the specified list option. */ - private _getIndexFromOption(option: MatListOption): number { + private _getOptionIndex(option: MatListOption): number { return this.options.toArray().indexOf(option); } }