fix(stepper): use up/down arrows for navigating vertical stepper

Currently both vertical and horizontal steppers use the left/right arrows to move focus steps. Based on the a11y guidelines (https://www.w3.org/TR/wai-aria-practices-1.1/#tabpanel) the vertical stepper should use the up/down arrows instead.

Author: crisbeto

src/cdk/stepper/stepper.ts

@@ -27,7 +27,7 @@ import {
   OnChanges,
   OnDestroy
 } from '@angular/core';
-import {LEFT_ARROW, RIGHT_ARROW, ENTER, SPACE} from '@angular/cdk/keycodes';
+import {LEFT_ARROW, RIGHT_ARROW, DOWN_ARROW, UP_ARROW, ENTER, SPACE} from '@angular/cdk/keycodes';
 import {CdkStepLabel} from './step-label';
 import {coerceBooleanProperty} from '@angular/cdk/coercion';
 import {AbstractControl} from '@angular/forms';
@@ -43,6 +43,9 @@ let nextId = 0;
  */
 export type StepContentPositionState = 'previous' | 'current' | 'next';
 
+/** Possible orientation of a stepper. */
+export type StepperOrientation = 'horizontal' | 'vertical';
+
 /** Change event emitted on selection changes. */
 export class StepperSelectionEvent {
   /** Index of the step now selected. */
@@ -182,6 +185,8 @@ export class CdkStepper implements OnDestroy {
   /** Used to track unique ID for each stepper component. */
   _groupId: number;
 
+  protected _orientation: StepperOrientation = 'horizontal';
+
   constructor(
     @Optional() private _dir: Directionality,
     private _changeDetectorRef: ChangeDetectorRef) {
@@ -252,30 +257,30 @@ export class CdkStepper implements OnDestroy {
   }
 
   _onKeydown(event: KeyboardEvent) {
-    switch (event.keyCode) {
-      case RIGHT_ARROW:
-        if (this._layoutDirection() === 'rtl') {
-          this._focusPreviousStep();
-        } else {
-          this._focusNextStep();
-        }
-        break;
-      case LEFT_ARROW:
-        if (this._layoutDirection() === 'rtl') {
-          this._focusNextStep();
-        } else {
-          this._focusPreviousStep();
-        }
-        break;
-      case SPACE:
-      case ENTER:
-        this.selectedIndex = this._focusIndex;
-        break;
-      default:
-        // Return to avoid calling preventDefault on keys that are not explicitly handled.
-        return;
+    const keyCode = event.keyCode;
+
+    // Note that the left/right arrows work both in vertical and horizontal mode.
+    if (keyCode === RIGHT_ARROW) {
+      this._layoutDirection() === 'rtl' ? this._focusPreviousStep() : this._focusNextStep();
+      event.preventDefault();
+    }
+
+    if (keyCode === LEFT_ARROW) {
+      this._layoutDirection() === 'rtl' ? this._focusNextStep() : this._focusPreviousStep();
+      event.preventDefault();
+    }
+
+    // Note that the up/down arrows only work in vertical mode.
+    // See: https://www.w3.org/TR/wai-aria-practices-1.1/#tabpanel
+    if (this._orientation === 'vertical' && (keyCode === UP_ARROW || keyCode === DOWN_ARROW)) {
+      keyCode === UP_ARROW ? this._focusPreviousStep() : this._focusNextStep();
+      event.preventDefault();
+    }
+
+    if (keyCode === SPACE || keyCode === ENTER) {
+      this.selectedIndex = this._focusIndex;
+      event.preventDefault();
     }
-    event.preventDefault();
   }
 
   private _focusNextStep() {

src/lib/stepper/stepper.spec.ts

@@ -1,12 +1,13 @@
 import {Directionality} from '@angular/cdk/bidi';
-import {ENTER, LEFT_ARROW, RIGHT_ARROW, SPACE} from '@angular/cdk/keycodes';
+import {ENTER, LEFT_ARROW, RIGHT_ARROW, UP_ARROW, DOWN_ARROW, SPACE} from '@angular/cdk/keycodes';
 import {dispatchKeyboardEvent} from '@angular/cdk/testing';
 import {Component, DebugElement} from '@angular/core';
 import {async, ComponentFixture, TestBed, inject} from '@angular/core/testing';
 import {AbstractControl, AsyncValidatorFn, FormControl, FormGroup, ReactiveFormsModule,
   ValidationErrors, Validators} from '@angular/forms';
 import {By} from '@angular/platform-browser';
 import {NoopAnimationsModule} from '@angular/platform-browser/animations';
+import {StepperOrientation} from '@angular/cdk/stepper';
 import {map} from 'rxjs/operators/map';
 import {take} from 'rxjs/operators/take';
 import {Observable} from 'rxjs/Observable';
@@ -18,7 +19,7 @@ import {MatStepperIntl} from './stepper-intl';
 
 const VALID_REGEX = /valid/;
 
-describe('MatHorizontalStepper', () => {
+fdescribe('MatHorizontalStepper', () => {
   let dir = 'ltr';
 
   beforeEach(async(() => {
@@ -87,9 +88,9 @@ describe('MatHorizontalStepper', () => {
       assertCorrectStepAnimationDirection(fixture);
     });
 
-    it('should support keyboard events to move and select focus', () => {
+    it('should support using the left/right arrows to move focus', () => {
       let stepHeaders = fixture.debugElement.queryAll(By.css('.mat-horizontal-stepper-header'));
-      assertCorrectKeyboardInteraction(fixture, stepHeaders);
+      assertCorrectKeyboardInteraction(fixture, stepHeaders, 'horizontal');
     });
 
     it('should not set focus on header of selected step if header is not clicked', () => {
@@ -202,7 +203,7 @@ describe('MatHorizontalStepper', () => {
   });
 });
 
-describe('MatVerticalStepper', () => {
+fdescribe('MatVerticalStepper', () => {
   let dir = 'ltr';
 
   beforeEach(async(() => {
@@ -271,9 +272,14 @@ describe('MatVerticalStepper', () => {
       assertCorrectStepAnimationDirection(fixture);
     });
 
-    it('should support keyboard events to move and select focus', () => {
+    it('should support using the left/right arrows to move focus', () => {
       let stepHeaders = fixture.debugElement.queryAll(By.css('.mat-vertical-stepper-header'));
-      assertCorrectKeyboardInteraction(fixture, stepHeaders);
+      assertCorrectKeyboardInteraction(fixture, stepHeaders, 'horizontal');
+    });
+
+    it('should support using the up/down arrows to move focus', () => {
+      let stepHeaders = fixture.debugElement.queryAll(By.css('.mat-vertical-stepper-header'));
+      assertCorrectKeyboardInteraction(fixture, stepHeaders, 'vertical');
     });
 
     it('should not set focus on header of selected step if header is not clicked', () => {
@@ -516,20 +522,23 @@ function assertCorrectStepAnimationDirection(fixture: ComponentFixture<any>, rtl
 
 /** Asserts that keyboard interaction works correctly. */
 function assertCorrectKeyboardInteraction(fixture: ComponentFixture<any>,
-                                          stepHeaders: DebugElement[]) {
+                                          stepHeaders: DebugElement[],
+                                          orientation: StepperOrientation) {
   let stepperComponent = fixture.debugElement.query(By.directive(MatStepper)).componentInstance;
+  let nextKey = orientation === 'vertical' ? DOWN_ARROW : RIGHT_ARROW;
+  let prevKey = orientation === 'vertical' ? UP_ARROW : LEFT_ARROW;
 
   expect(stepperComponent._focusIndex).toBe(0);
   expect(stepperComponent.selectedIndex).toBe(0);
 
   let stepHeaderEl = stepHeaders[0].nativeElement;
-  dispatchKeyboardEvent(stepHeaderEl, 'keydown', RIGHT_ARROW);
+  dispatchKeyboardEvent(stepHeaderEl, 'keydown', nextKey);
   fixture.detectChanges();
 
   expect(stepperComponent._focusIndex)
-      .toBe(1, 'Expected index of focused step to increase by 1 after RIGHT_ARROW event.');
+      .toBe(1, 'Expected index of focused step to increase by 1 after pressing the next key.');
   expect(stepperComponent.selectedIndex)
-      .toBe(0, 'Expected index of selected step to remain unchanged after RIGHT_ARROW event.');
+      .toBe(0, 'Expected index of selected step to remain unchanged after pressing the next key.');
 
   stepHeaderEl = stepHeaders[1].nativeElement;
   dispatchKeyboardEvent(stepHeaderEl, 'keydown', ENTER);
@@ -542,26 +551,25 @@ function assertCorrectKeyboardInteraction(fixture: ComponentFixture<any>,
           'Expected index of selected step to change to index of focused step after ENTER event.');
 
   stepHeaderEl = stepHeaders[1].nativeElement;
-  dispatchKeyboardEvent(stepHeaderEl, 'keydown', LEFT_ARROW);
+  dispatchKeyboardEvent(stepHeaderEl, 'keydown', prevKey);
   fixture.detectChanges();
 
   expect(stepperComponent._focusIndex)
-      .toBe(0, 'Expected index of focused step to decrease by 1 after LEFT_ARROW event.');
-  expect(stepperComponent.selectedIndex)
-      .toBe(1, 'Expected index of selected step to remain unchanged after LEFT_ARROW event.');
+      .toBe(0, 'Expected index of focused step to decrease by 1 after pressing the previous key.');
+  expect(stepperComponent.selectedIndex).toBe(1,
+      'Expected index of selected step to remain unchanged after pressing the previous key.');
 
   // When the focus is on the last step and right arrow key is pressed, the focus should cycle
   // through to the first step.
   stepperComponent._focusIndex = 2;
   stepHeaderEl = stepHeaders[2].nativeElement;
-  dispatchKeyboardEvent(stepHeaderEl, 'keydown', RIGHT_ARROW);
+  dispatchKeyboardEvent(stepHeaderEl, 'keydown', nextKey);
   fixture.detectChanges();
 
-  expect(stepperComponent._focusIndex)
-      .toBe(0,
-          'Expected index of focused step to cycle through to index 0 after RIGHT_ARROW event.');
+  expect(stepperComponent._focusIndex).toBe(0,
+      'Expected index of focused step to cycle through to index 0 after pressing the next key.');
   expect(stepperComponent.selectedIndex)
-      .toBe(1, 'Expected index of selected step to remain unchanged after RIGHT_ARROW event.');
+      .toBe(1, 'Expected index of selected step to remain unchanged after pressing the next key.');
 
   stepHeaderEl = stepHeaders[0].nativeElement;
   dispatchKeyboardEvent(stepHeaderEl, 'keydown', SPACE);

src/lib/stepper/stepper.ts

@@ -8,6 +8,7 @@
 
 import {animate, state, style, transition, trigger} from '@angular/animations';
 import {CdkStep, CdkStepper} from '@angular/cdk/stepper';
+import {Directionality} from '@angular/cdk/bidi';
 import {
   AfterContentInit,
   Component,
@@ -21,7 +22,9 @@ import {
   SkipSelf,
   ViewChildren,
   ViewEncapsulation,
+  ChangeDetectorRef,
   ChangeDetectionStrategy,
+  Optional,
 } from '@angular/core';
 import {FormControl, FormGroupDirective, NgForm} from '@angular/forms';
 import {ErrorStateMatcher} from '@angular/material/core';
@@ -129,4 +132,9 @@ export class MatHorizontalStepper extends MatStepper { }
   preserveWhitespaces: false,
   changeDetection: ChangeDetectionStrategy.OnPush,
 })
-export class MatVerticalStepper extends MatStepper { }
+export class MatVerticalStepper extends MatStepper {
+  constructor(@Optional() dir: Directionality, changeDetectorRef: ChangeDetectorRef) {
+    super(dir, changeDetectorRef);
+    this._orientation = 'vertical';
+  }
+}