fix(a11y): key manager preventing arrow key events with modifier keys (#13503)

* Skips handling arrow key events that have a modifier key. Previously we would prevent the default action which can get in the way of some of the native keyboard shortcuts (e.g. marking all of the text using shift + up arrow). This has come up before in #11987, however these changes expand it to the key manager.
* Adds an API that allows consumers to opt into having the key manager handle some modifier keys. This is an exception for some of our components where we have custom functionality for shift + arrow key.
* Fixes the `metaKey` always being set to true on our fake keyboard events.

Fixes #13496.

Author: crisbeto

src/cdk/a11y/key-manager/list-key-manager.spec.ts

@@ -5,7 +5,7 @@ import {fakeAsync, tick} from '@angular/core/testing';
 import {createKeyboardEvent} from '@angular/cdk/testing';
 import {ActiveDescendantKeyManager} from './activedescendant-key-manager';
 import {FocusKeyManager} from './focus-key-manager';
-import {ListKeyManager} from './list-key-manager';
+import {ListKeyManager, ListKeyManagerModifierKey} from './list-key-manager';
 import {FocusOrigin} from '../focus-monitor/focus-monitor';
 import {Subject} from 'rxjs';
 
@@ -103,6 +103,16 @@ describe('Key managers', () => {
         expect(spy).toHaveBeenCalled();
       });
 
+      it('should emit tabOut when the tab key is pressed with a modifier', () => {
+        const spy = jasmine.createSpy('tabOut spy');
+        keyManager.tabOut.pipe(take(1)).subscribe(spy);
+
+        Object.defineProperty(fakeKeyEvents.tab, 'shiftKey', {get: () => true});
+        keyManager.onKeydown(fakeKeyEvents.tab);
+
+        expect(spy).toHaveBeenCalled();
+      });
+
       it('should emit an event whenever the active item changes', () => {
         const spy = jasmine.createSpy('change spy');
         const subscription = keyManager.change.subscribe(spy);
@@ -338,6 +348,51 @@ describe('Key managers', () => {
           keyManager.onKeydown(this.prevKeyEvent);
           expect(this.prevKeyEvent.defaultPrevented).toBe(true);
         });
+
+        it('should not do anything for arrow keys if the alt key is held down', () => {
+          runModifierKeyTest(this, 'altKey');
+        });
+
+        it('should not do anything for arrow keys if the control key is held down', () => {
+          runModifierKeyTest(this, 'ctrlKey');
+        });
+
+        it('should not do anything for arrow keys if the meta key is held down', () => {
+          runModifierKeyTest(this, 'metaKey');
+        });
+
+        it('should not do anything for arrow keys if the shift key is held down', () => {
+          runModifierKeyTest(this, 'shiftKey');
+        });
+
+      }
+
+      /** Runs the test that asserts that we handle modifier keys correctly. */
+      function runModifierKeyTest(context: {
+        nextKeyEvent: KeyboardEvent,
+        prevKeyEvent: KeyboardEvent
+      }, modifier: ListKeyManagerModifierKey) {
+        const initialActiveIndex = keyManager.activeItemIndex;
+        const spy = jasmine.createSpy('change spy');
+        const subscription = keyManager.change.subscribe(spy);
+
+        expect(context.nextKeyEvent.defaultPrevented).toBe(false);
+        expect(context.prevKeyEvent.defaultPrevented).toBe(false);
+
+        Object.defineProperty(context.nextKeyEvent, modifier, {get: () => true});
+        Object.defineProperty(context.prevKeyEvent, modifier, {get: () => true});
+
+        keyManager.onKeydown(context.nextKeyEvent);
+        expect(context.nextKeyEvent.defaultPrevented).toBe(false);
+        expect(keyManager.activeItemIndex).toBe(initialActiveIndex);
+        expect(spy).not.toHaveBeenCalled();
+
+        keyManager.onKeydown(context.prevKeyEvent);
+        expect(context.prevKeyEvent.defaultPrevented).toBe(false);
+        expect(keyManager.activeItemIndex).toBe(initialActiveIndex);
+        expect(spy).not.toHaveBeenCalled();
+
+        subscription.unsubscribe();
       }
 
     });

src/cdk/a11y/key-manager/list-key-manager.ts

@@ -30,6 +30,9 @@ export interface ListKeyManagerOption {
   getLabel?(): string;
 }
 
+/** Modifier keys handled by the ListKeyManager. */
+export type ListKeyManagerModifierKey = 'altKey' | 'ctrlKey' | 'metaKey' | 'shiftKey';
+
 /**
  * This class manages keyboard events for selectable lists. If you pass it a query list
  * of items, it will set the active item correctly when arrow events occur.
@@ -42,6 +45,7 @@ export class ListKeyManager<T extends ListKeyManagerOption> {
   private _typeaheadSubscription = Subscription.EMPTY;
   private _vertical = true;
   private _horizontal: 'ltr' | 'rtl' | null;
+  private _allowedModifierKeys: ListKeyManagerModifierKey[] = [];
 
   /**
    * Predicate function that can be used to check whether an item should be skipped
@@ -118,6 +122,15 @@ export class ListKeyManager<T extends ListKeyManagerOption> {
     return this;
   }
 
+  /**
+   * Modifier keys which are allowed to be held down and whose default actions will be prevented
+   * as the user is pressing the arrow keys. Defaults to not allowing any modifier keys.
+   */
+  withAllowedModifierKeys(keys: ListKeyManagerModifierKey[]): this {
+    this._allowedModifierKeys = keys;
+    return this;
+  }
+
   /**
    * Turns on typeahead mode which allows users to set the active item by typing.
    * @param debounceInterval Time to wait after the last keystroke before setting the active item.
@@ -188,45 +201,43 @@ export class ListKeyManager<T extends ListKeyManagerOption> {
    */
   onKeydown(event: KeyboardEvent): void {
     const keyCode = event.keyCode;
+    const modifiers: ListKeyManagerModifierKey[] = ['altKey', 'ctrlKey', 'metaKey', 'shiftKey'];
+    const isModifierAllowed = modifiers.every(modifier => {
+      return !event[modifier] || this._allowedModifierKeys.indexOf(modifier) > -1;
+    });
 
     switch (keyCode) {
       case TAB:
         this.tabOut.next();
         return;
 
       case DOWN_ARROW:
-        if (this._vertical) {
+        if (this._vertical && isModifierAllowed) {
           this.setNextItemActive();
           break;
         } else {
           return;
         }
 
       case UP_ARROW:
-        if (this._vertical) {
+        if (this._vertical && isModifierAllowed) {
           this.setPreviousItemActive();
           break;
         } else {
           return;
         }
 
       case RIGHT_ARROW:
-        if (this._horizontal === 'ltr') {
-          this.setNextItemActive();
-          break;
-        } else if (this._horizontal === 'rtl') {
-          this.setPreviousItemActive();
+        if (this._horizontal && isModifierAllowed) {
+          this._horizontal === 'rtl' ? this.setPreviousItemActive() : this.setNextItemActive();
           break;
         } else {
           return;
         }
 
       case LEFT_ARROW:
-        if (this._horizontal === 'ltr') {
-          this.setPreviousItemActive();
-          break;
-        } else if (this._horizontal === 'rtl') {
-          this.setNextItemActive();
+        if (this._horizontal && isModifierAllowed) {
+          this._horizontal === 'rtl' ? this.setNextItemActive() : this.setPreviousItemActive();
           break;
         } else {
           return;

src/lib/list/selection-list.ts

@@ -345,7 +345,8 @@ export class MatSelectionList extends _MatSelectionListMixinBase implements Focu
       .withTypeAhead()
       // Allow disabled items to be focusable. For accessibility reasons, there must be a way for
       // screenreader users, that allows reading the different options of the list.
-      .skipPredicate(() => false);
+      .skipPredicate(() => false)
+      .withAllowedModifierKeys(['shiftKey']);
 
     if (this._tempValues) {
       this._setOptionsFromValues(this._tempValues);

src/lib/select/select.ts

@@ -864,7 +864,8 @@ export class MatSelect extends _MatSelectMixinBase implements AfterContentInit,
     this._keyManager = new ActiveDescendantKeyManager<MatOption>(this.options)
       .withTypeAhead()
       .withVerticalOrientation()
-      .withHorizontalOrientation(this._isRtl() ? 'rtl' : 'ltr');
+      .withHorizontalOrientation(this._isRtl() ? 'rtl' : 'ltr')
+      .withAllowedModifierKeys(['shiftKey']);
 
     this._keyManager.tabOut.pipe(takeUntil(this._destroy)).subscribe(() => {
       // Restore focus to the trigger before closing. Ensures that the focus