Skip to content

Commit a08dc55

Browse files
crisbetoandrewseguin
authored andcommitted
fix(dialog): escape key not working once element loses focus (#3082)
Fixes issue in the dialog that prevented the user from being able to close via the escape key, if any of the elements inside the dialog loses focus. Fixes #3009.
1 parent 1547440 commit a08dc55

File tree

5 files changed

+47
-25
lines changed

5 files changed

+47
-25
lines changed

e2e/components/dialog/dialog.e2e.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,16 @@ describe('dialog', () => {
3535
});
3636
});
3737

38+
it('should close by pressing escape when the first tabbable element has lost focus', () => {
39+
element(by.id('default')).click();
40+
41+
waitForDialog().then(() => {
42+
clickElementAtPoint('md-dialog-container', { x: 0, y: 0 });
43+
pressKeys(Key.ESCAPE);
44+
expectToExist('md-dialog-container', false);
45+
});
46+
});
47+
3848
it('should close by clicking on the "close" button', () => {
3949
element(by.id('default')).click();
4050

src/lib/dialog/dialog-container.ts

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ import 'rxjs/add/operator/first';
2727
host: {
2828
'[class.mat-dialog-container]': 'true',
2929
'[attr.role]': 'dialogConfig?.role',
30-
'(keydown.escape)': 'handleEscapeKey()',
3130
},
3231
encapsulation: ViewEncapsulation.None,
3332
})
@@ -93,16 +92,6 @@ export class MdDialogContainer extends BasePortalHost implements OnDestroy {
9392
});
9493
}
9594

96-
/**
97-
* Handles the user pressing the Escape key.
98-
* @docs-private
99-
*/
100-
handleEscapeKey() {
101-
if (!this.dialogConfig.disableClose) {
102-
this.dialogRef.close();
103-
}
104-
}
105-
10695
ngOnDestroy() {
10796
// When the dialog is destroyed, return focus to the element that originally had it before
10897
// the dialog was opened. Wait for the DOM to finish settling before changing the focus so

src/lib/dialog/dialog-ref.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import {OverlayRef} from '../core';
2+
import {MdDialogConfig} from './dialog-config';
23
import {Observable} from 'rxjs/Observable';
34
import {Subject} from 'rxjs/Subject';
45

@@ -17,7 +18,7 @@ export class MdDialogRef<T> {
1718
/** Subject for notifying the user that the dialog has finished closing. */
1819
private _afterClosed: Subject<any> = new Subject();
1920

20-
constructor(private _overlayRef: OverlayRef) { }
21+
constructor(private _overlayRef: OverlayRef, public config: MdDialogConfig) { }
2122

2223
/**
2324
* Close the dialog.

src/lib/dialog/dialog.spec.ts

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,12 @@ import {NgModule,
1515
Injector,
1616
Inject,
1717
} from '@angular/core';
18-
import {By} from '@angular/platform-browser';
1918
import {MdDialogModule} from './index';
2019
import {MdDialog} from './dialog';
2120
import {OverlayContainer} from '../core';
2221
import {MdDialogRef} from './dialog-ref';
23-
import {MdDialogContainer} from './dialog-container';
2422
import {MD_DIALOG_DATA} from './dialog-injector';
23+
import {ESCAPE} from '../core/keyboard/keycodes';
2524

2625

2726
describe('MdDialog', () => {
@@ -136,11 +135,7 @@ describe('MdDialog', () => {
136135

137136
viewContainerFixture.detectChanges();
138137

139-
let dialogContainer: MdDialogContainer =
140-
viewContainerFixture.debugElement.query(By.directive(MdDialogContainer)).componentInstance;
141-
142-
// Fake the user pressing the escape key by calling the handler directly.
143-
dialogContainer.handleEscapeKey();
138+
dispatchKeydownEvent(document, ESCAPE);
144139

145140
expect(overlayContainerElement.querySelector('md-dialog-container')).toBeNull();
146141
});
@@ -324,11 +319,7 @@ describe('MdDialog', () => {
324319

325320
viewContainerFixture.detectChanges();
326321

327-
let dialogContainer: MdDialogContainer = viewContainerFixture.debugElement.query(
328-
By.directive(MdDialogContainer)).componentInstance;
329-
330-
// Fake the user pressing the escape key by calling the handler directly.
331-
dialogContainer.handleEscapeKey();
322+
dispatchKeydownEvent(document, ESCAPE);
332323

333324
expect(overlayContainerElement.querySelector('md-dialog-container')).toBeTruthy();
334325
});
@@ -565,3 +556,15 @@ const TEST_DIRECTIVES = [
565556
],
566557
})
567558
class DialogTestModule { }
559+
560+
561+
// TODO(crisbeto): switch to using function from common testing utils once #2943 is merged.
562+
function dispatchKeydownEvent(element: Node, keyCode: number) {
563+
let event: any = document.createEvent('KeyboardEvent');
564+
(event.initKeyEvent || event.initKeyboardEvent).bind(event)(
565+
'keydown', true, true, window, 0, 0, 0, 0, 0, keyCode);
566+
Object.defineProperty(event, 'keyCode', {
567+
get: function() { return keyCode; }
568+
});
569+
element.dispatchEvent(event);
570+
}

src/lib/dialog/dialog.ts

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import {Observable} from 'rxjs/Observable';
33
import {Subject} from 'rxjs/Subject';
44
import {Overlay, OverlayRef, ComponentType, OverlayState, ComponentPortal} from '../core';
55
import {extendObject} from '../core/util/object-extend';
6+
import {ESCAPE} from '../core/keyboard/keycodes';
67
import {DialogInjector} from './dialog-injector';
78
import {MdDialogConfig} from './dialog-config';
89
import {MdDialogRef} from './dialog-ref';
@@ -22,6 +23,7 @@ export class MdDialog {
2223
private _openDialogsAtThisLevel: MdDialogRef<any>[] = [];
2324
private _afterAllClosedAtThisLevel = new Subject<void>();
2425
private _afterOpenAtThisLevel = new Subject<MdDialogRef<any>>();
26+
private _boundKeydown = this._handleKeydown.bind(this);
2527

2628
/** Keeps track of the currently-open dialogs. */
2729
get _openDialogs(): MdDialogRef<any>[] {
@@ -65,6 +67,10 @@ export class MdDialog {
6567
let dialogRef =
6668
this._attachDialogContent(componentOrTemplateRef, dialogContainer, overlayRef, config);
6769

70+
if (!this._openDialogs.length && !this._parentDialog) {
71+
document.addEventListener('keydown', this._boundKeydown);
72+
}
73+
6874
this._openDialogs.push(dialogRef);
6975
dialogRef.afterClosed().subscribe(() => this._removeOpenDialog(dialogRef));
7076
this._afterOpen.next(dialogRef);
@@ -129,7 +135,7 @@ export class MdDialog {
129135
config?: MdDialogConfig): MdDialogRef<T> {
130136
// Create a reference to the dialog we're creating in order to give the user a handle
131137
// to modify and close it.
132-
let dialogRef = <MdDialogRef<T>> new MdDialogRef(overlayRef);
138+
let dialogRef = <MdDialogRef<T>> new MdDialogRef(overlayRef, config);
133139

134140
if (!config.disableClose) {
135141
// When the dialog backdrop is clicked, we want to close it.
@@ -199,9 +205,22 @@ export class MdDialog {
199205
// no open dialogs are left, call next on afterAllClosed Subject
200206
if (!this._openDialogs.length) {
201207
this._afterAllClosed.next();
208+
document.removeEventListener('keydown', this._boundKeydown);
202209
}
203210
}
204211
}
212+
213+
/**
214+
* Handles global key presses while there are open dialogs. Closes the
215+
* top dialog when the user presses escape.
216+
*/
217+
private _handleKeydown(event: KeyboardEvent): void {
218+
let topDialog = this._openDialogs[this._openDialogs.length - 1];
219+
220+
if (event.keyCode === ESCAPE && topDialog && !topDialog.config.disableClose) {
221+
topDialog.close();
222+
}
223+
}
205224
}
206225

207226
/**

0 commit comments

Comments
 (0)