Address feedback

angular · Sep 29, 2017 · 8ad9fec · 8ad9fec
1 parent 573969c
commit 8ad9fec
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 27 deletions.
diff --git 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>(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
@@ -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);
   }
 }