From 4b7e52d30aa342c3024ed078837a9bd4ca9e773b Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Wed, 30 Nov 2016 20:52:02 +0100 Subject: [PATCH] fix(interactivity-checker): improve robustness of isTabbable (#1950) * fix(focus-trap): improve robustness * Address feedback * Remove extra blank line. * Address additional feedback * Change timeout delay --- src/demo-app/demo-app-module.ts | 2 + src/demo-app/demo-app/demo-app.ts | 3 +- src/demo-app/demo-app/routes.ts | 2 + src/demo-app/platform/platform-demo.ts | 17 ++ src/lib/core/a11y/focus-trap.spec.ts | 13 +- src/lib/core/a11y/index.ts | 2 + .../core/a11y/interactivity-checker.spec.ts | 274 ++++++++++++++++-- src/lib/core/a11y/interactivity-checker.ts | 136 +++++++-- src/lib/core/core.ts | 3 + src/lib/core/platform/platform.ts | 49 ++++ src/lib/dialog/dialog.ts | 5 +- src/lib/module.ts | 3 + 12 files changed, 449 insertions(+), 60 deletions(-) create mode 100644 src/demo-app/platform/platform-demo.ts create mode 100644 src/lib/core/platform/platform.ts diff --git a/src/demo-app/demo-app-module.ts b/src/demo-app/demo-app-module.ts index ec35c947b742..afcc8e6d7382 100644 --- a/src/demo-app/demo-app-module.ts +++ b/src/demo-app/demo-app-module.ts @@ -35,6 +35,7 @@ import {PortalDemo, ScienceJoke} from './portal/portal-demo'; import {MenuDemo} from './menu/menu-demo'; import {TabsDemo, SunnyTabContent, RainyTabContent, FoggyTabContent} from './tabs/tabs-demo'; import {ProjectionDemo, ProjectionTestComponent} from './projection/projection-demo'; +import {PlatformDemo} from './platform/platform-demo'; @NgModule({ imports: [ @@ -86,6 +87,7 @@ import {ProjectionDemo, ProjectionTestComponent} from './projection/projection-d SunnyTabContent, RainyTabContent, FoggyTabContent, + PlatformDemo ], entryComponents: [ DemoApp, diff --git a/src/demo-app/demo-app/demo-app.ts b/src/demo-app/demo-app/demo-app.ts index 2b229a414054..005564a13c06 100644 --- a/src/demo-app/demo-app/demo-app.ts +++ b/src/demo-app/demo-app/demo-app.ts @@ -47,6 +47,7 @@ export class DemoApp { {name: 'Snack Bar', route: 'snack-bar'}, {name: 'Tabs', route: 'tabs'}, {name: 'Toolbar', route: 'toolbar'}, - {name: 'Tooltip', route: 'tooltip'} + {name: 'Tooltip', route: 'tooltip'}, + {name: 'Platform', route: 'platform'} ]; } diff --git a/src/demo-app/demo-app/routes.ts b/src/demo-app/demo-app/routes.ts index 285be668b898..7cc7685eb226 100644 --- a/src/demo-app/demo-app/routes.ts +++ b/src/demo-app/demo-app/routes.ts @@ -30,6 +30,7 @@ import {TooltipDemo} from '../tooltip/tooltip-demo'; import {SnackBarDemo} from '../snack-bar/snack-bar-demo'; import {ProjectionDemo} from '../projection/projection-demo'; import {TABS_DEMO_ROUTES} from '../tabs/routes'; +import {PlatformDemo} from '../platform/platform-demo'; export const DEMO_APP_ROUTES: Routes = [ {path: '', component: Home}, @@ -62,4 +63,5 @@ export const DEMO_APP_ROUTES: Routes = [ {path: 'dialog', component: DialogDemo}, {path: 'tooltip', component: TooltipDemo}, {path: 'snack-bar', component: SnackBarDemo}, + {path: 'platform', component: PlatformDemo} ]; diff --git a/src/demo-app/platform/platform-demo.ts b/src/demo-app/platform/platform-demo.ts new file mode 100644 index 000000000000..c5b746a70fe5 --- /dev/null +++ b/src/demo-app/platform/platform-demo.ts @@ -0,0 +1,17 @@ +import {Component} from '@angular/core'; +import {MdPlatform} from '@angular/material'; + +@Component({ + template: ` +

Is Android: {{ platform.ANDROID }}

+

Is iOS: {{ platform.IOS }}

+

Is Firefox: {{ platform.FIREFOX }}

+

Is Blink: {{ platform.BLINK }}

+

Is Webkit: {{ platform.WEBKIT }}

+

Is Trident: {{ platform.TRIDENT }}

+

Is Edge: {{ platform.EDGE }}

+ ` +}) +export class PlatformDemo { + constructor(public platform: MdPlatform) {} +} diff --git a/src/lib/core/a11y/focus-trap.spec.ts b/src/lib/core/a11y/focus-trap.spec.ts index 0e225d8b6655..86357817dfe2 100644 --- a/src/lib/core/a11y/focus-trap.spec.ts +++ b/src/lib/core/a11y/focus-trap.spec.ts @@ -3,17 +3,21 @@ import {By} from '@angular/platform-browser'; import {Component} from '@angular/core'; import {FocusTrap} from './focus-trap'; import {InteractivityChecker} from './interactivity-checker'; +import {MdPlatform} from '../platform/platform'; describe('FocusTrap', () => { + describe('with default element', () => { + let fixture: ComponentFixture; let focusTrapInstance: FocusTrap; + let platform: MdPlatform = new MdPlatform(); beforeEach(async(() => { TestBed.configureTestingModule({ declarations: [FocusTrap, FocusTrapTestApp], - providers: [InteractivityChecker] + providers: [InteractivityChecker, MdPlatform] }); TestBed.compileComponents(); @@ -38,8 +42,11 @@ describe('FocusTrap', () => { // focus event handler directly. focusTrapInstance.focusLastTabbableElement(); + // In iOS button elements are never tabbable, so the last element will be the input. + let lastElement = platform.IOS ? 'input' : 'button'; + expect(document.activeElement.nodeName.toLowerCase()) - .toBe('button', 'Expected button element to be focused'); + .toBe(lastElement, `Expected ${lastElement} element to be focused`); }); }); @@ -50,7 +57,7 @@ describe('FocusTrap', () => { beforeEach(async(() => { TestBed.configureTestingModule({ declarations: [FocusTrap, FocusTrapTargetTestApp], - providers: [InteractivityChecker] + providers: [InteractivityChecker, MdPlatform] }); TestBed.compileComponents(); diff --git a/src/lib/core/a11y/index.ts b/src/lib/core/a11y/index.ts index 70621bb45e90..0e285f77d60c 100644 --- a/src/lib/core/a11y/index.ts +++ b/src/lib/core/a11y/index.ts @@ -2,10 +2,12 @@ import {NgModule, ModuleWithProviders} from '@angular/core'; import {FocusTrap} from './focus-trap'; import {MdLiveAnnouncer} from './live-announcer'; import {InteractivityChecker} from './interactivity-checker'; +import {PlatformModule} from '../platform/platform'; export const A11Y_PROVIDERS = [MdLiveAnnouncer, InteractivityChecker]; @NgModule({ + imports: [PlatformModule], declarations: [FocusTrap], exports: [FocusTrap], }) diff --git a/src/lib/core/a11y/interactivity-checker.spec.ts b/src/lib/core/a11y/interactivity-checker.spec.ts index 648499781aef..ab707d665152 100644 --- a/src/lib/core/a11y/interactivity-checker.spec.ts +++ b/src/lib/core/a11y/interactivity-checker.spec.ts @@ -1,14 +1,17 @@ import {InteractivityChecker} from './interactivity-checker'; +import {MdPlatform} from '../platform/platform'; +import {async} from '@angular/core/testing'; describe('InteractivityChecker', () => { let testContainerElement: HTMLElement; let checker: InteractivityChecker; + let platform: MdPlatform = new MdPlatform(); beforeEach(() => { testContainerElement = document.createElement('div'); document.body.appendChild(testContainerElement); - checker = new InteractivityChecker(); + checker = new InteractivityChecker(platform); }); afterEach(() => { @@ -234,52 +237,237 @@ describe('InteractivityChecker', () => { }); }); - it('should return true for div and span with tabindex == 0', () => { - let elements = createElements('div', 'span'); - - elements.forEach(el => el.setAttribute('tabindex', '0')); - appendElements(elements); - elements.forEach(el => { - expect(checker.isFocusable(el)) - .toBe(true, `Expected <${el.nodeName} tabindex="0"> to be focusable`); - }); - }); }); describe('isTabbable', () => { - it('should return true for native form controls and anchor without tabindex attribute', () => { - let elements = createElements('input', 'textarea', 'select', 'button', 'a'); - appendElements(elements); - elements.forEach(el => { - expect(checker.isTabbable(el)).toBe(true, `Expected <${el.nodeName}> to be tabbable`); + it('should respect the tabindex for video elements with controls', + // Do not run for Blink, Firefox and iOS because those treat video elements + // with controls different and are covered in other tests. + runIf(!platform.BLINK && !platform.FIREFOX && !platform.IOS, () => { + let video = createFromTemplate('