fix(list): selection list moving focus when an option is destroyed

When an option from a selection list is destroyed, the list adjusts the active item index so the user's selection isn't lost, however because we use the `FocusKeyManager`, it means that updating the index will move focus to the new item. This can cause focus to be removed from another element on the page that the user might be interacting with. These changes continue to update the active index, but they only move focus if the destroyed option was focused at the time that it was destroyed.

Author: crisbeto

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

@@ -214,6 +214,7 @@ describe('MatSelectionList without forms', () => {
     it('should restore focus if active option is destroyed', () => {
       const manager = selectionList.componentInstance._keyManager;
 
+      spyOn(listOptions[2].componentInstance, 'focus').and.callThrough();
       listOptions[3].componentInstance._handleFocus();
 
       expect(manager.activeItemIndex).toBe(3);
@@ -222,8 +223,28 @@ describe('MatSelectionList without forms', () => {
       fixture.detectChanges();
 
       expect(manager.activeItemIndex).toBe(2);
+      expect(listOptions[2].componentInstance.focus).toHaveBeenCalled();
     });
 
+    it('should not attempt to focus the next option when the destroyed option was not focused',
+      () => {
+        const manager = selectionList.componentInstance._keyManager;
+
+        // Focus and blur the option to move the active item index.
+        listOptions[3].componentInstance._handleFocus();
+        listOptions[3].componentInstance._handleBlur();
+
+        spyOn(listOptions[2].componentInstance, 'focus').and.callThrough();
+
+        expect(manager.activeItemIndex).toBe(3);
+
+        fixture.componentInstance.showLastOption = false;
+        fixture.detectChanges();
+
+        expect(manager.activeItemIndex).toBe(2);
+        expect(listOptions[2].componentInstance.focus).not.toHaveBeenCalled();
+      });
+
     it('should focus previous item when press UP ARROW', () => {
       let testListItem = listOptions[2].nativeElement as HTMLElement;
       let UP_EVENT: KeyboardEvent =

src/lib/list/selection-list.ts

@@ -98,6 +98,7 @@ export class MatListOption extends _MatListOptionMixinBase
 
   private _selected = false;
   private _disabled = false;
+  private _hasFocus = false;
 
   @ContentChild(MatListAvatarCssMatStyler) _avatar: MatListAvatarCssMatStyler;
   @ContentChild(MatListIconCssMatStyler) _icon: MatListIconCssMatStyler;
@@ -172,7 +173,13 @@ export class MatListOption extends _MatListOptionMixinBase
       Promise.resolve().then(() => this.selected = false);
     }
 
-    this.selectionList._removeOptionFromList(this);
+    const hadFocus = this._hasFocus;
+    const newActiveItem = this.selectionList._removeOptionFromList(this);
+
+    // Only move focus if this option was focused at the time it was destroyed.
+    if (hadFocus && newActiveItem) {
+      newActiveItem.focus();
+    }
   }
 
   /** Toggles the selection state of the option. */
@@ -209,10 +216,12 @@ export class MatListOption extends _MatListOptionMixinBase
 
   _handleFocus() {
     this.selectionList._setFocusedOption(this);
+    this._hasFocus = true;
   }
 
   _handleBlur() {
     this.selectionList._onTouched();
+    this._hasFocus = false;
   }
 
   /** Retrieves the DOM element of the component host. */
@@ -385,18 +394,23 @@ export class MatSelectionList extends _MatSelectionListMixinBase implements Focu
     this._keyManager.updateActiveItemIndex(this._getOptionIndex(option));
   }
 
-  /** Removes an option from the selection list and updates the active item. */
-  _removeOptionFromList(option: MatListOption) {
+  /**
+   * Removes an option from the selection list and updates the active item.
+   * @returns Currently-active item.
+   */
+  _removeOptionFromList(option: MatListOption): MatListOption | null {
     const optionIndex = this._getOptionIndex(option);
 
     if (optionIndex > -1 && this._keyManager.activeItemIndex === optionIndex) {
       // Check whether the option is the last item
       if (optionIndex > 0) {
-        this._keyManager.setPreviousItemActive();
+        this._keyManager.updateActiveItemIndex(optionIndex - 1);
       } else if (optionIndex === 0 && this.options.length > 1) {
-        this._keyManager.setNextItemActive();
+        this._keyManager.updateActiveItemIndex(Math.min(optionIndex + 1, this.options.length - 1));
       }
     }
+
+    return this._keyManager.activeItem;
   }
 
   /** Passes relevant key presses to our key manager. */