Skip to content

chore(focus-classes): fix failing tests when browser is blurred #2935

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

Merged
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 29 additions & 38 deletions src/lib/core/style/focus-classes.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,26 +4,16 @@ import {StyleModule} from './index';
import {By} from '@angular/platform-browser';
import {TAB} from '../keyboard/keycodes';
import {FocusOriginMonitor} from './focus-classes';
import {PlatformModule} from '../platform/index';
import {Platform} from '../platform/platform';


// NOTE: Firefox only fires focus & blur events when it is the currently active window.
// This is not always the case on our CI setup, therefore we disable tests that depend on these
// events firing for Firefox. We may be able to fix this by configuring our CI to start Firefox with
// the following preference: focusmanager.testmode = true


describe('FocusOriginMonitor', () => {
let fixture: ComponentFixture<PlainButton>;
let buttonElement: HTMLElement;
let buttonRenderer: Renderer;
let focusOriginMonitor: FocusOriginMonitor;
let platform: Platform;

beforeEach(async(() => {
TestBed.configureTestingModule({
imports: [StyleModule, PlatformModule],
imports: [StyleModule],
declarations: [
PlainButton,
],
Expand All @@ -32,21 +22,27 @@ describe('FocusOriginMonitor', () => {
TestBed.compileComponents();
}));

beforeEach(inject([FocusOriginMonitor, Platform], (fom: FocusOriginMonitor, pfm: Platform) => {
beforeEach(inject([FocusOriginMonitor], (fom: FocusOriginMonitor) => {
fixture = TestBed.createComponent(PlainButton);
fixture.detectChanges();

buttonElement = fixture.debugElement.query(By.css('button')).nativeElement;
buttonRenderer = fixture.componentInstance.renderer;
focusOriginMonitor = fom;
platform = pfm;

focusOriginMonitor.registerElementForFocusClasses(buttonElement, buttonRenderer);

// On Saucelabs, browsers will run simultaneously and therefore can't focus all browser windows
// at the same time. This is problematic when testing focus states. Chrome and Firefox
// only fire FocusEvents when the window is focused. This issue also appears locally.
let _nativeButtonFocus = buttonElement.focus.bind(buttonElement);
Copy link
Contributor

Choose a reason for hiding this comment

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

can you extract this into a separate method e.g. fixFocus(el: Element)? also both this and dispatchFocusEvent would be good things to put in a separate test utilities directory

Copy link
Member Author

@devversion devversion Feb 5, 2017

Choose a reason for hiding this comment

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

Done. I called it patchElementFocus - also added a comment, so we can move all helpers to a global utility file as in #2902

buttonElement.focus = () => {
document.hasFocus() ? _nativeButtonFocus() : dispatchFocusEvent(buttonElement);
};

}));

it('manually registered element should receive focus classes', async(() => {
if (platform.FIREFOX) { return; }

buttonElement.focus();
fixture.detectChanges();

Expand All @@ -59,8 +55,6 @@ describe('FocusOriginMonitor', () => {
}));

it('should detect focus via keyboard', async(() => {
if (platform.FIREFOX) { return; }

// Simulate focus via keyboard.
dispatchKeydownEvent(document, TAB);
buttonElement.focus();
Expand All @@ -79,8 +73,6 @@ describe('FocusOriginMonitor', () => {
}));

it('should detect focus via mouse', async(() => {
if (platform.FIREFOX) { return; }

// Simulate focus via mouse.
dispatchMousedownEvent(document);
buttonElement.focus();
Expand All @@ -99,8 +91,6 @@ describe('FocusOriginMonitor', () => {
}));

it('should detect programmatic focus', async(() => {
if (platform.FIREFOX) { return; }

// Programmatically focus.
buttonElement.focus();
fixture.detectChanges();
Expand All @@ -118,8 +108,6 @@ describe('FocusOriginMonitor', () => {
}));

it('focusVia keyboard should simulate keyboard focus', async(() => {
if (platform.FIREFOX) { return; }

focusOriginMonitor.focusVia(buttonElement, buttonRenderer, 'keyboard');
fixture.detectChanges();

Expand All @@ -136,8 +124,6 @@ describe('FocusOriginMonitor', () => {
}));

it('focusVia mouse should simulate mouse focus', async(() => {
if (platform.FIREFOX) { return; }

focusOriginMonitor.focusVia(buttonElement, buttonRenderer, 'mouse');
fixture.detectChanges();

Expand All @@ -154,8 +140,6 @@ describe('FocusOriginMonitor', () => {
}));

it('focusVia program should simulate programmatic focus', async(() => {
if (platform.FIREFOX) { return; }

focusOriginMonitor.focusVia(buttonElement, buttonRenderer, 'program');
fixture.detectChanges();

Expand All @@ -176,11 +160,10 @@ describe('FocusOriginMonitor', () => {
describe('cdkFocusClasses', () => {
let fixture: ComponentFixture<ButtonWithFocusClasses>;
let buttonElement: HTMLElement;
let platform: Platform;

beforeEach(async(() => {
TestBed.configureTestingModule({
imports: [StyleModule, PlatformModule],
imports: [StyleModule],
declarations: [
ButtonWithFocusClasses,
],
Expand All @@ -189,21 +172,26 @@ describe('cdkFocusClasses', () => {
TestBed.compileComponents();
}));

beforeEach(inject([Platform], (pfm: Platform) => {
beforeEach(() => {
fixture = TestBed.createComponent(ButtonWithFocusClasses);
fixture.detectChanges();

buttonElement = fixture.debugElement.query(By.css('button')).nativeElement;
platform = pfm;
}));

// On Saucelabs, browsers will run simultaneously and therefore can't focus all browser windows
// at the same time. This is problematic when testing focus states. Chrome and Firefox
// only fire FocusEvents when the window is focused. This issue also appears locally.
let _nativeButtonFocus = buttonElement.focus.bind(buttonElement);
buttonElement.focus = () => {
document.hasFocus() ? _nativeButtonFocus() : dispatchFocusEvent(buttonElement);
};
});

it('should initially not be focused', () => {
expect(buttonElement.classList.length).toBe(0, 'button should not have focus classes');
});

it('should detect focus via keyboard', async(() => {
if (platform.FIREFOX) { return; }

// Simulate focus via keyboard.
dispatchKeydownEvent(document, TAB);
buttonElement.focus();
Expand All @@ -222,8 +210,6 @@ describe('cdkFocusClasses', () => {
}));

it('should detect focus via mouse', async(() => {
if (platform.FIREFOX) { return; }

// Simulate focus via mouse.
dispatchMousedownEvent(document);
buttonElement.focus();
Expand All @@ -242,8 +228,6 @@ describe('cdkFocusClasses', () => {
}));

it('should detect programmatic focus', async(() => {
if (platform.FIREFOX) { return; }

// Programmatically focus.
buttonElement.focus();
fixture.detectChanges();
Expand Down Expand Up @@ -291,3 +275,10 @@ function dispatchKeydownEvent(element: Node, keyCode: number) {
});
element.dispatchEvent(event);
}

/** Dispatches a focus event on the specified element. */
function dispatchFocusEvent(element: Node, type = 'focus') {
let event = document.createEvent('Event');
event.initEvent(type, true, true);
element.dispatchEvent(event);
}
61 changes: 28 additions & 33 deletions test/karma-test-shim.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,34 +2,37 @@
Error.stackTraceLimit = Infinity;
jasmine.DEFAULT_TIMEOUT_INTERVAL = 3000;

__karma__.loaded = function () {
};
__karma__.loaded = function () {};

var distPath = '/base/dist/';
var baseDir = '/base/dist/';
var configFile = baseDir + '@angular/material/system-config-spec.js';
var specFiles = Object.keys(window.__karma__.files).filter(isMaterialSpecFile);

function isJsFile(path) {
return path.slice(-3) == '.js';
}
// Configure the base path for dist/
System.config({baseURL: baseDir});

function isSpecFile(path) {
return path.slice(-8) == '.spec.js';
}
// Load the spec SystemJS configuration file.
System.import(configFile)
.then(configureTestBed)
.then(runMaterialSpecs)
.then(__karma__.start, __karma__.error);

function isMaterialFile(path) {
return isJsFile(path) && path.indexOf('vendor') == -1;
}

var allSpecFiles = Object.keys(window.__karma__.files)
.filter(isSpecFile)
.filter(isMaterialFile);
/** Runs the Angular Material specs in Karma. */
function runMaterialSpecs() {
// By importing all spec files, Karma will run the tests directly.
return Promise.all(specFiles.map(function(fileName) {
return System.import(fileName);
}));
}

// Load our SystemJS configuration.
System.config({
baseURL: distPath
});
/** Whether the specified file is part of Angular Material. */
function isMaterialSpecFile(path) {
return path.slice(-8) === '.spec.js' && path.indexOf('vendor') === -1;
}

System.import(distPath + '@angular/material/system-config-spec.js').then(function() {
// Load and configure the TestComponentBuilder.
/** Configures Angular's TestBed. */
function configureTestBed() {
return Promise.all([
System.import('@angular/core/testing'),
System.import('@angular/platform-browser-dynamic/testing')
Expand All @@ -40,16 +43,8 @@ System.import(distPath + '@angular/material/system-config-spec.js').then(functio
jasmine.DEFAULT_TIMEOUT_INTERVAL = 10000;

testing.TestBed.initTestEnvironment(
testingBrowser.BrowserDynamicTestingModule,
testingBrowser.platformBrowserDynamicTesting());
testingBrowser.BrowserDynamicTestingModule,
testingBrowser.platformBrowserDynamicTesting()
);
});
}).then(function() {
// Finally, load all spec files.
// This will run the tests directly.
return Promise.all(
allSpecFiles.map(function (moduleName) {
return System.import(moduleName).then(function(module) {
return module;
});
}));
}).then(__karma__.start, __karma__.error);
}