Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(interactivity-checker): improve robustness of isTabbable #1950

Merged
merged 5 commits into from Nov 30, 2016
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/demo-app/demo-app-module.ts
Expand Up @@ -34,6 +34,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: [
Expand Down Expand Up @@ -84,6 +85,7 @@ import {ProjectionDemo, ProjectionTestComponent} from './projection/projection-d
SunnyTabContent,
RainyTabContent,
FoggyTabContent,
PlatformDemo
],
entryComponents: [
DemoApp,
Expand Down
3 changes: 2 additions & 1 deletion src/demo-app/demo-app/demo-app.ts
Expand Up @@ -46,6 +46,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'}
];
}
2 changes: 2 additions & 0 deletions src/demo-app/demo-app/routes.ts
Expand Up @@ -29,6 +29,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},
Expand Down Expand Up @@ -60,4 +61,5 @@ export const DEMO_APP_ROUTES: Routes = [
{path: 'dialog', component: DialogDemo},
{path: 'tooltip', component: TooltipDemo},
{path: 'snack-bar', component: SnackBarDemo},
{path: 'platform', component: PlatformDemo}
];
20 changes: 20 additions & 0 deletions src/demo-app/platform/platform-demo.ts
@@ -0,0 +1,20 @@
import {Component} from '@angular/core';
import {MdPlatform} from '@angular/material';

@Component({
template: `
<p>Is Android: {{ platform.ANDROID }}</p>
<p>Is iOS: {{ platform.IOS }}</p>
<p>Is Firefox: {{ platform.FIREFOX }}</p>
<p>Is Blink: {{ platform.BLINK }}</p>
<p>Is Webkit: {{ platform.WEBKIT }}</p>
<p>Is Trident: {{ platform.TRIDENT }}</p>
<p>Is Edge: {{ platform.EDGE }}</p>

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extra blank line

`
})
export class PlatformDemo {

constructor(public platform: MdPlatform) {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove extra blank lines

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that's a big deal - Is there any specific convention in the Google JavaScript conventions? - Going to change it though.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there's any official convention for it, I just like to keep it consistent throughout the project. Most other files I've looked at seem to not leave space here


}
13 changes: 10 additions & 3 deletions src/lib/core/a11y/focus-trap.spec.ts
Expand Up @@ -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<FocusTrapTestApp>;
let focusTrapInstance: FocusTrap;
let platform: MdPlatform = new MdPlatform();

beforeEach(async(() => {
TestBed.configureTestingModule({
declarations: [FocusTrap, FocusTrapTestApp],
providers: [InteractivityChecker]
providers: [InteractivityChecker, MdPlatform]
});

TestBed.compileComponents();
Expand All @@ -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`);
});
});

Expand All @@ -50,7 +57,7 @@ describe('FocusTrap', () => {
beforeEach(async(() => {
TestBed.configureTestingModule({
declarations: [FocusTrap, FocusTrapTargetTestApp],
providers: [InteractivityChecker]
providers: [InteractivityChecker, MdPlatform]
});

TestBed.compileComponents();
Expand Down
2 changes: 2 additions & 0 deletions src/lib/core/a11y/index.ts
Expand Up @@ -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],
})
Expand Down