Skip to content

Commit 0871d88

Browse files
crisbetojosephperrott
crisbeto
authored and
josephperrott
committed
fix: prevent default on escape key presses (#16202)
1 parent 711c8c3 commit 0871d88

File tree

17 files changed

+204
-28
lines changed

17 files changed

+204
-28
lines changed

‎src/cdk-experimental/dialog/dialog-ref.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99

1010
import {OverlayRef, GlobalPositionStrategy, OverlaySizeConfig} from '@angular/cdk/overlay';
11-
import {ESCAPE} from '@angular/cdk/keycodes';
11+
import {ESCAPE, hasModifierKey} from '@angular/cdk/keycodes';
1212
import {Observable} from 'rxjs';
1313
import {map, filter} from 'rxjs/operators';
1414
import {DialogPosition} from './dialog-config';
@@ -56,8 +56,13 @@ export class DialogRef<T, R = any> {
5656

5757
// Close when escape keydown event occurs
5858
_overlayRef.keydownEvents()
59-
.pipe(filter(event => event.keyCode === ESCAPE && !this.disableClose))
60-
.subscribe(() => this.close());
59+
.pipe(filter(event => {
60+
return event.keyCode === ESCAPE && !this.disableClose && !hasModifierKey(event);
61+
}))
62+
.subscribe(event => {
63+
event.preventDefault();
64+
this.close();
65+
});
6166
}
6267

6368
/** Gets an observable that emits when the overlay's backdrop has been clicked. */

‎src/cdk-experimental/dialog/dialog.spec.ts

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import {Directionality} from '@angular/cdk/bidi';
2626
import {CdkDialogContainer} from './dialog-container';
2727
import {OverlayContainer} from '@angular/cdk/overlay';
2828
import {A, ESCAPE} from '@angular/cdk/keycodes';
29-
import {dispatchKeyboardEvent} from '@angular/cdk/testing';
29+
import {dispatchKeyboardEvent, createKeyboardEvent, dispatchEvent} from '@angular/cdk/testing';
3030
import {DIALOG_DATA, Dialog, DialogModule, DialogRef} from './index';
3131

3232
describe('Dialog', () => {
@@ -216,11 +216,26 @@ describe('Dialog', () => {
216216
dialog.openFromComponent(PizzaMsg, {viewContainerRef: testViewContainerRef});
217217

218218
viewContainerFixture.detectChanges();
219-
dispatchKeyboardEvent(document.body, 'keydown', ESCAPE);
219+
const event = dispatchKeyboardEvent(document.body, 'keydown', ESCAPE);
220220
viewContainerFixture.detectChanges();
221221
flush();
222222

223223
expect(overlayContainerElement.querySelector('cdk-dialog-container')).toBeNull();
224+
expect(event.defaultPrevented).toBe(true);
225+
}));
226+
227+
it('should not close a dialog via the escape key if a modifier is pressed', fakeAsync(() => {
228+
dialog.openFromComponent(PizzaMsg, {viewContainerRef: testViewContainerRef});
229+
230+
viewContainerFixture.detectChanges();
231+
const event = createKeyboardEvent('keydown', ESCAPE);
232+
Object.defineProperty(event, 'altKey', {get: () => true});
233+
dispatchEvent(document.body, event);
234+
viewContainerFixture.detectChanges();
235+
flush();
236+
237+
expect(overlayContainerElement.querySelector('cdk-dialog-container')).toBeTruthy();
238+
expect(event.defaultPrevented).toBe(false);
224239
}));
225240

226241
it('should close from a ViewContainerRef with OnPush change detection', fakeAsync(() => {

‎src/cdk/overlay/overlay-directives.spec.ts

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import {Component, ViewChild} from '@angular/core';
22
import {By} from '@angular/platform-browser';
33
import {ComponentFixture, TestBed, async, inject, fakeAsync, tick} from '@angular/core/testing';
44
import {Directionality} from '@angular/cdk/bidi';
5-
import {dispatchKeyboardEvent} from '@angular/cdk/testing';
5+
import {dispatchKeyboardEvent, createKeyboardEvent, dispatchEvent} from '@angular/cdk/testing';
66
import {ESCAPE, A} from '@angular/cdk/keycodes';
77
import {CdkConnectedOverlay, OverlayModule, CdkOverlayOrigin} from './index';
88
import {OverlayContainer} from './overlay-container';
@@ -109,11 +109,25 @@ describe('Overlay directives', () => {
109109
fixture.componentInstance.isOpen = true;
110110
fixture.detectChanges();
111111

112-
dispatchKeyboardEvent(document.body, 'keydown', ESCAPE);
112+
const event = dispatchKeyboardEvent(document.body, 'keydown', ESCAPE);
113113
fixture.detectChanges();
114114

115115
expect(overlayContainerElement.textContent!.trim()).toBe('',
116116
'Expected overlay to have been detached.');
117+
expect(event.defaultPrevented).toBe(true);
118+
});
119+
120+
it('should not close when pressing escape with a modifier', () => {
121+
fixture.componentInstance.isOpen = true;
122+
fixture.detectChanges();
123+
124+
const event = createKeyboardEvent('keydown', ESCAPE);
125+
Object.defineProperty(event, 'altKey', {get: () => true});
126+
dispatchEvent(document.body, event);
127+
fixture.detectChanges();
128+
129+
expect(overlayContainerElement.textContent!.trim()).toBeTruthy();
130+
expect(event.defaultPrevented).toBe(false);
117131
});
118132

119133
it('should not depend on the order in which the `origin` and `open` are set', async(() => {

‎src/cdk/overlay/overlay-directives.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
import {Direction, Directionality} from '@angular/cdk/bidi';
1010
import {coerceBooleanProperty} from '@angular/cdk/coercion';
11-
import {ESCAPE} from '@angular/cdk/keycodes';
11+
import {ESCAPE, hasModifierKey} from '@angular/cdk/keycodes';
1212
import {TemplatePortal} from '@angular/cdk/portal';
1313
import {
1414
Directive,
@@ -275,7 +275,8 @@ export class CdkConnectedOverlay implements OnDestroy, OnChanges {
275275
this._overlayRef.keydownEvents().subscribe((event: KeyboardEvent) => {
276276
this.overlayKeydown.next(event);
277277

278-
if (event.keyCode === ESCAPE) {
278+
if (event.keyCode === ESCAPE && !hasModifierKey(event)) {
279+
event.preventDefault();
279280
this._detachOverlay();
280281
}
281282
});

‎src/material-experimental/mdc-menu/menu.spec.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -346,6 +346,24 @@ describe('MatMenu', () => {
346346
tick(500);
347347

348348
expect(overlayContainerElement.textContent).toBe('');
349+
expect(event.defaultPrevented).toBe(true);
350+
}));
351+
352+
it('should not close the menu when pressing ESCAPE with a modifier', fakeAsync(() => {
353+
const fixture = createComponent(SimpleMenu, [], [FakeIcon]);
354+
fixture.detectChanges();
355+
fixture.componentInstance.trigger.openMenu();
356+
357+
const panel = overlayContainerElement.querySelector('.mat-mdc-menu-panel')!;
358+
const event = createKeyboardEvent('keydown', ESCAPE);
359+
360+
Object.defineProperty(event, 'altKey', {get: () => true});
361+
dispatchEvent(panel, event);
362+
fixture.detectChanges();
363+
tick(500);
364+
365+
expect(overlayContainerElement.textContent).toBeTruthy();
366+
expect(event.defaultPrevented).toBe(false);
349367
}));
350368

351369
it('should open a custom menu', () => {

‎src/material/bottom-sheet/bottom-sheet-ref.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*/
88

99
import {Location} from '@angular/common';
10-
import {ESCAPE} from '@angular/cdk/keycodes';
10+
import {ESCAPE, hasModifierKey} from '@angular/cdk/keycodes';
1111
import {OverlayRef} from '@angular/cdk/overlay';
1212
import {merge, Observable, Subject} from 'rxjs';
1313
import {filter, take} from 'rxjs/operators';
@@ -72,8 +72,10 @@ export class MatBottomSheetRef<T = any, R = any> {
7272
merge(
7373
_overlayRef.backdropClick(),
7474
_overlayRef.keydownEvents().pipe(filter(event => event.keyCode === ESCAPE))
75-
).subscribe(() => {
76-
if (!this.disableClose) {
75+
).subscribe(event => {
76+
if (!this.disableClose &&
77+
(event.type !== 'keydown' || !hasModifierKey(event as KeyboardEvent))) {
78+
event.preventDefault();
7779
this.dismiss();
7880
}
7981
});

‎src/material/bottom-sheet/bottom-sheet.spec.ts

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import {Directionality} from '@angular/cdk/bidi';
22
import {A, ESCAPE} from '@angular/cdk/keycodes';
33
import {OverlayContainer, ScrollStrategy} from '@angular/cdk/overlay';
44
import {ViewportRuler} from '@angular/cdk/scrolling';
5-
import {dispatchKeyboardEvent} from '@angular/cdk/testing';
5+
import {dispatchKeyboardEvent, createKeyboardEvent, dispatchEvent} from '@angular/cdk/testing';
66
import {Location} from '@angular/common';
77
import {SpyLocation} from '@angular/common/testing';
88
import {
@@ -155,11 +155,25 @@ describe('MatBottomSheet', () => {
155155
it('should close a bottom sheet via the escape key', fakeAsync(() => {
156156
bottomSheet.open(PizzaMsg, {viewContainerRef: testViewContainerRef});
157157

158-
dispatchKeyboardEvent(document.body, 'keydown', ESCAPE);
158+
const event = dispatchKeyboardEvent(document.body, 'keydown', ESCAPE);
159159
viewContainerFixture.detectChanges();
160160
flush();
161161

162162
expect(overlayContainerElement.querySelector('mat-bottom-sheet-container')).toBeNull();
163+
expect(event.defaultPrevented).toBe(true);
164+
}));
165+
166+
it('should not close a bottom sheet via the escape key with a modifier', fakeAsync(() => {
167+
bottomSheet.open(PizzaMsg, {viewContainerRef: testViewContainerRef});
168+
169+
const event = createKeyboardEvent('keydown', ESCAPE);
170+
Object.defineProperty(event, 'altKey', {get: () => true});
171+
dispatchEvent(document.body, event);
172+
viewContainerFixture.detectChanges();
173+
flush();
174+
175+
expect(overlayContainerElement.querySelector('mat-bottom-sheet-container')).toBeTruthy();
176+
expect(event.defaultPrevented).toBe(false);
163177
}));
164178

165179
it('should close when clicking on the overlay backdrop', fakeAsync(() => {

‎src/material/datepicker/datepicker.spec.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,11 +193,12 @@ describe('MatDatepicker', () => {
193193

194194
expect(testComponent.datepicker.opened).toBe(true, 'Expected datepicker to be open.');
195195

196-
dispatchKeyboardEvent(document.body, 'keydown', ESCAPE);
196+
const event = dispatchKeyboardEvent(document.body, 'keydown', ESCAPE);
197197
fixture.detectChanges();
198198
flush();
199199

200200
expect(testComponent.datepicker.opened).toBe(false, 'Expected datepicker to be closed.');
201+
expect(event.defaultPrevented).toBe(true);
201202
}));
202203

203204
it('should set the proper role on the popup', fakeAsync(() => {

‎src/material/datepicker/datepicker.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -459,7 +459,13 @@ export class MatDatepicker<D> implements OnDestroy, CanColor {
459459
return event.keyCode === ESCAPE ||
460460
(this._datepickerInput && event.altKey && event.keyCode === UP_ARROW);
461461
}))
462-
).subscribe(() => this.close());
462+
).subscribe(event => {
463+
if (event) {
464+
event.preventDefault();
465+
}
466+
467+
this.close();
468+
});
463469
}
464470

465471
/** Create the popup PositionStrategy. */

‎src/material/dialog/dialog-ref.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {ESCAPE} from '@angular/cdk/keycodes';
9+
import {ESCAPE, hasModifierKey} from '@angular/cdk/keycodes';
1010
import {GlobalPositionStrategy, OverlayRef} from '@angular/cdk/overlay';
1111
import {Location} from '@angular/common';
1212
import {Observable, Subject} from 'rxjs';
@@ -78,8 +78,13 @@ export class MatDialogRef<T, R = any> {
7878
});
7979

8080
_overlayRef.keydownEvents()
81-
.pipe(filter(event => event.keyCode === ESCAPE && !this.disableClose))
82-
.subscribe(() => this.close());
81+
.pipe(filter(event => {
82+
return event.keyCode === ESCAPE && !this.disableClose && !hasModifierKey(event);
83+
}))
84+
.subscribe(event => {
85+
event.preventDefault();
86+
this.close();
87+
});
8388
}
8489

8590
/**

‎src/material/dialog/dialog.spec.ts

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ import {MatDialogContainer} from './dialog-container';
2727
import {OverlayContainer, ScrollStrategy, Overlay} from '@angular/cdk/overlay';
2828
import {ScrollDispatcher} from '@angular/cdk/scrolling';
2929
import {A, ESCAPE} from '@angular/cdk/keycodes';
30-
import {dispatchKeyboardEvent} from '@angular/cdk/testing';
30+
import {dispatchKeyboardEvent, createKeyboardEvent} from '@angular/cdk/testing';
3131
import {
3232
MAT_DIALOG_DATA,
3333
MatDialog,
@@ -240,11 +240,26 @@ describe('MatDialog', () => {
240240
viewContainerRef: testViewContainerRef
241241
});
242242

243-
dispatchKeyboardEvent(document.body, 'keydown', ESCAPE);
243+
const event = dispatchKeyboardEvent(document.body, 'keydown', ESCAPE);
244244
viewContainerFixture.detectChanges();
245245
flush();
246246

247247
expect(overlayContainerElement.querySelector('mat-dialog-container')).toBeNull();
248+
expect(event.defaultPrevented).toBe(true);
249+
}));
250+
251+
it('should not close a dialog via the escape key with a modifier', fakeAsync(() => {
252+
dialog.open(PizzaMsg, {
253+
viewContainerRef: testViewContainerRef
254+
});
255+
256+
const event = createKeyboardEvent('keydown', ESCAPE);
257+
Object.defineProperty(event, 'altKey', {get: () => true});
258+
viewContainerFixture.detectChanges();
259+
flush();
260+
261+
expect(overlayContainerElement.querySelector('mat-dialog-container')).toBeTruthy();
262+
expect(event.defaultPrevented).toBe(false);
248263
}));
249264

250265
it('should close from a ViewContainerRef with OnPush change detection', fakeAsync(() => {

‎src/material/menu/menu.spec.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,24 @@ describe('MatMenu', () => {
347347
tick(500);
348348

349349
expect(overlayContainerElement.textContent).toBe('');
350+
expect(event.defaultPrevented).toBe(true);
351+
}));
352+
353+
it('should not close the menu when pressing ESCAPE with a modifier', fakeAsync(() => {
354+
const fixture = createComponent(SimpleMenu, [], [FakeIcon]);
355+
fixture.detectChanges();
356+
fixture.componentInstance.trigger.openMenu();
357+
358+
const panel = overlayContainerElement.querySelector('.mat-menu-panel')!;
359+
const event = createKeyboardEvent('keydown', ESCAPE);
360+
361+
Object.defineProperty(event, 'altKey', {get: () => true});
362+
dispatchEvent(panel, event);
363+
fixture.detectChanges();
364+
tick(500);
365+
366+
expect(overlayContainerElement.textContent).toBeTruthy();
367+
expect(event.defaultPrevented).toBe(false);
350368
}));
351369

352370
it('should open a custom menu', () => {

‎src/material/menu/menu.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,10 @@ export class _MatMenuBase implements AfterContentInit, MatMenuPanel<MatMenuItem>
266266

267267
switch (keyCode) {
268268
case ESCAPE:
269-
this.closed.emit('keydown');
269+
if (!hasModifierKey(event)) {
270+
event.preventDefault();
271+
this.closed.emit('keydown');
272+
}
270273
break;
271274
case LEFT_ARROW:
272275
if (this.parentMenu && this.direction === 'ltr') {

‎src/material/sidenav/drawer.spec.ts

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import {Direction} from '@angular/cdk/bidi';
1515
import {A11yModule} from '@angular/cdk/a11y';
1616
import {PlatformModule} from '@angular/cdk/platform';
1717
import {ESCAPE} from '@angular/cdk/keycodes';
18-
import {dispatchKeyboardEvent} from '@angular/cdk/testing';
18+
import {dispatchKeyboardEvent, createKeyboardEvent, dispatchEvent} from '@angular/cdk/testing';
1919
import {CdkScrollable} from '@angular/cdk/scrolling';
2020

2121

@@ -202,12 +202,39 @@ describe('MatDrawer', () => {
202202
expect(testComponent.closeCount).toBe(0, 'Expected no close events.');
203203
expect(testComponent.closeStartCount).toBe(0, 'Expected no close start events.');
204204

205-
dispatchKeyboardEvent(drawer.nativeElement, 'keydown', ESCAPE);
205+
const event = dispatchKeyboardEvent(drawer.nativeElement, 'keydown', ESCAPE);
206206
fixture.detectChanges();
207207
flush();
208208

209209
expect(testComponent.closeCount).toBe(1, 'Expected one close event.');
210210
expect(testComponent.closeStartCount).toBe(1, 'Expected one close start event.');
211+
expect(event.defaultPrevented).toBe(true);
212+
}));
213+
214+
it('should not close when pressing escape with a modifier', fakeAsync(() => {
215+
let fixture = TestBed.createComponent(BasicTestApp);
216+
217+
fixture.detectChanges();
218+
219+
let testComponent: BasicTestApp = fixture.debugElement.componentInstance;
220+
let drawer = fixture.debugElement.query(By.directive(MatDrawer));
221+
222+
drawer.componentInstance.open();
223+
fixture.detectChanges();
224+
tick();
225+
226+
expect(testComponent.closeCount).toBe(0, 'Expected no close events.');
227+
expect(testComponent.closeStartCount).toBe(0, 'Expected no close start events.');
228+
229+
const event = createKeyboardEvent('keydown', ESCAPE);
230+
Object.defineProperty(event, 'altKey', {get: () => true});
231+
dispatchEvent(drawer.nativeElement, event);
232+
fixture.detectChanges();
233+
flush();
234+
235+
expect(testComponent.closeCount).toBe(0, 'Expected still no close events.');
236+
expect(testComponent.closeStartCount).toBe(0, 'Expected still no close start events.');
237+
expect(event.defaultPrevented).toBe(false);
211238
}));
212239

213240
it('should fire the open event when open on init', fakeAsync(() => {

0 commit comments

Comments
 (0)