fix: prevent default on escape key presses (#16202)

This commit is contained in:
Kristiyan Kostadinov
2019-06-05 21:24:06 +02:00
committed by Joey Perrott
parent e7b7b30885
commit 8ecfc72269
17 changed files with 204 additions and 28 deletions
+8 -3
View File
@@ -8,7 +8,7 @@
import {OverlayRef, GlobalPositionStrategy, OverlaySizeConfig} from '@angular/cdk/overlay';
import {ESCAPE} from '@angular/cdk/keycodes';
import {ESCAPE, hasModifierKey} from '@angular/cdk/keycodes';
import {Observable} from 'rxjs';
import {map, filter} from 'rxjs/operators';
import {DialogPosition} from './dialog-config';
@@ -56,8 +56,13 @@ export class DialogRef<T, R = any> {
// Close when escape keydown event occurs
_overlayRef.keydownEvents()
.pipe(filter(event => event.keyCode === ESCAPE && !this.disableClose))
.subscribe(() => this.close());
.pipe(filter(event => {
return event.keyCode === ESCAPE && !this.disableClose && !hasModifierKey(event);
}))
.subscribe(event => {
event.preventDefault();
this.close();
});
}
/** Gets an observable that emits when the overlay's backdrop has been clicked. */
+17 -2
View File
@@ -26,7 +26,7 @@ import {Directionality} from '@angular/cdk/bidi';
import {CdkDialogContainer} from './dialog-container';
import {OverlayContainer} from '@angular/cdk/overlay';
import {A, ESCAPE} from '@angular/cdk/keycodes';
import {dispatchKeyboardEvent} from '@angular/cdk/testing';
import {dispatchKeyboardEvent, createKeyboardEvent, dispatchEvent} from '@angular/cdk/testing';
import {DIALOG_DATA, Dialog, DialogModule, DialogRef} from './index';
describe('Dialog', () => {
@@ -216,11 +216,26 @@ describe('Dialog', () => {
dialog.openFromComponent(PizzaMsg, {viewContainerRef: testViewContainerRef});
viewContainerFixture.detectChanges();
dispatchKeyboardEvent(document.body, 'keydown', ESCAPE);
const event = dispatchKeyboardEvent(document.body, 'keydown', ESCAPE);
viewContainerFixture.detectChanges();
flush();
expect(overlayContainerElement.querySelector('cdk-dialog-container')).toBeNull();
expect(event.defaultPrevented).toBe(true);
}));
it('should not close a dialog via the escape key if a modifier is pressed', fakeAsync(() => {
dialog.openFromComponent(PizzaMsg, {viewContainerRef: testViewContainerRef});
viewContainerFixture.detectChanges();
const event = createKeyboardEvent('keydown', ESCAPE);
Object.defineProperty(event, 'altKey', {get: () => true});
dispatchEvent(document.body, event);
viewContainerFixture.detectChanges();
flush();
expect(overlayContainerElement.querySelector('cdk-dialog-container')).toBeTruthy();
expect(event.defaultPrevented).toBe(false);
}));
it('should close from a ViewContainerRef with OnPush change detection', fakeAsync(() => {
+16 -2
View File
@@ -2,7 +2,7 @@ import {Component, ViewChild} from '@angular/core';
import {By} from '@angular/platform-browser';
import {ComponentFixture, TestBed, async, inject, fakeAsync, tick} from '@angular/core/testing';
import {Directionality} from '@angular/cdk/bidi';
import {dispatchKeyboardEvent} from '@angular/cdk/testing';
import {dispatchKeyboardEvent, createKeyboardEvent, dispatchEvent} from '@angular/cdk/testing';
import {ESCAPE, A} from '@angular/cdk/keycodes';
import {CdkConnectedOverlay, OverlayModule, CdkOverlayOrigin} from './index';
import {OverlayContainer} from './overlay-container';
@@ -109,11 +109,25 @@ describe('Overlay directives', () => {
fixture.componentInstance.isOpen = true;
fixture.detectChanges();
dispatchKeyboardEvent(document.body, 'keydown', ESCAPE);
const event = dispatchKeyboardEvent(document.body, 'keydown', ESCAPE);
fixture.detectChanges();
expect(overlayContainerElement.textContent!.trim()).toBe('',
'Expected overlay to have been detached.');
expect(event.defaultPrevented).toBe(true);
});
it('should not close when pressing escape with a modifier', () => {
fixture.componentInstance.isOpen = true;
fixture.detectChanges();
const event = createKeyboardEvent('keydown', ESCAPE);
Object.defineProperty(event, 'altKey', {get: () => true});
dispatchEvent(document.body, event);
fixture.detectChanges();
expect(overlayContainerElement.textContent!.trim()).toBeTruthy();
expect(event.defaultPrevented).toBe(false);
});
it('should not depend on the order in which the `origin` and `open` are set', async(() => {
+3 -2
View File
@@ -8,7 +8,7 @@
import {Direction, Directionality} from '@angular/cdk/bidi';
import {coerceBooleanProperty} from '@angular/cdk/coercion';
import {ESCAPE} from '@angular/cdk/keycodes';
import {ESCAPE, hasModifierKey} from '@angular/cdk/keycodes';
import {TemplatePortal} from '@angular/cdk/portal';
import {
Directive,
@@ -275,7 +275,8 @@ export class CdkConnectedOverlay implements OnDestroy, OnChanges {
this._overlayRef.keydownEvents().subscribe((event: KeyboardEvent) => {
this.overlayKeydown.next(event);
if (event.keyCode === ESCAPE) {
if (event.keyCode === ESCAPE && !hasModifierKey(event)) {
event.preventDefault();
this._detachOverlay();
}
});
@@ -346,6 +346,24 @@ describe('MatMenu', () => {
tick(500);
expect(overlayContainerElement.textContent).toBe('');
expect(event.defaultPrevented).toBe(true);
}));
it('should not close the menu when pressing ESCAPE with a modifier', fakeAsync(() => {
const fixture = createComponent(SimpleMenu, [], [FakeIcon]);
fixture.detectChanges();
fixture.componentInstance.trigger.openMenu();
const panel = overlayContainerElement.querySelector('.mat-mdc-menu-panel')!;
const event = createKeyboardEvent('keydown', ESCAPE);
Object.defineProperty(event, 'altKey', {get: () => true});
dispatchEvent(panel, event);
fixture.detectChanges();
tick(500);
expect(overlayContainerElement.textContent).toBeTruthy();
expect(event.defaultPrevented).toBe(false);
}));
it('should open a custom menu', () => {
@@ -7,7 +7,7 @@
*/
import {Location} from '@angular/common';
import {ESCAPE} from '@angular/cdk/keycodes';
import {ESCAPE, hasModifierKey} from '@angular/cdk/keycodes';
import {OverlayRef} from '@angular/cdk/overlay';
import {merge, Observable, Subject} from 'rxjs';
import {filter, take} from 'rxjs/operators';
@@ -72,8 +72,10 @@ export class MatBottomSheetRef<T = any, R = any> {
merge(
_overlayRef.backdropClick(),
_overlayRef.keydownEvents().pipe(filter(event => event.keyCode === ESCAPE))
).subscribe(() => {
if (!this.disableClose) {
).subscribe(event => {
if (!this.disableClose &&
(event.type !== 'keydown' || !hasModifierKey(event as KeyboardEvent))) {
event.preventDefault();
this.dismiss();
}
});
+16 -2
View File
@@ -2,7 +2,7 @@ import {Directionality} from '@angular/cdk/bidi';
import {A, ESCAPE} from '@angular/cdk/keycodes';
import {OverlayContainer, ScrollStrategy} from '@angular/cdk/overlay';
import {ViewportRuler} from '@angular/cdk/scrolling';
import {dispatchKeyboardEvent} from '@angular/cdk/testing';
import {dispatchKeyboardEvent, createKeyboardEvent, dispatchEvent} from '@angular/cdk/testing';
import {Location} from '@angular/common';
import {SpyLocation} from '@angular/common/testing';
import {
@@ -155,11 +155,25 @@ describe('MatBottomSheet', () => {
it('should close a bottom sheet via the escape key', fakeAsync(() => {
bottomSheet.open(PizzaMsg, {viewContainerRef: testViewContainerRef});
dispatchKeyboardEvent(document.body, 'keydown', ESCAPE);
const event = dispatchKeyboardEvent(document.body, 'keydown', ESCAPE);
viewContainerFixture.detectChanges();
flush();
expect(overlayContainerElement.querySelector('mat-bottom-sheet-container')).toBeNull();
expect(event.defaultPrevented).toBe(true);
}));
it('should not close a bottom sheet via the escape key with a modifier', fakeAsync(() => {
bottomSheet.open(PizzaMsg, {viewContainerRef: testViewContainerRef});
const event = createKeyboardEvent('keydown', ESCAPE);
Object.defineProperty(event, 'altKey', {get: () => true});
dispatchEvent(document.body, event);
viewContainerFixture.detectChanges();
flush();
expect(overlayContainerElement.querySelector('mat-bottom-sheet-container')).toBeTruthy();
expect(event.defaultPrevented).toBe(false);
}));
it('should close when clicking on the overlay backdrop', fakeAsync(() => {
+2 -1
View File
@@ -193,11 +193,12 @@ describe('MatDatepicker', () => {
expect(testComponent.datepicker.opened).toBe(true, 'Expected datepicker to be open.');
dispatchKeyboardEvent(document.body, 'keydown', ESCAPE);
const event = dispatchKeyboardEvent(document.body, 'keydown', ESCAPE);
fixture.detectChanges();
flush();
expect(testComponent.datepicker.opened).toBe(false, 'Expected datepicker to be closed.');
expect(event.defaultPrevented).toBe(true);
}));
it('should set the proper role on the popup', fakeAsync(() => {
+7 -1
View File
@@ -459,7 +459,13 @@ export class MatDatepicker<D> implements OnDestroy, CanColor {
return event.keyCode === ESCAPE ||
(this._datepickerInput && event.altKey && event.keyCode === UP_ARROW);
}))
).subscribe(() => this.close());
).subscribe(event => {
if (event) {
event.preventDefault();
}
this.close();
});
}
/** Create the popup PositionStrategy. */
+8 -3
View File
@@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/
import {ESCAPE} from '@angular/cdk/keycodes';
import {ESCAPE, hasModifierKey} from '@angular/cdk/keycodes';
import {GlobalPositionStrategy, OverlayRef} from '@angular/cdk/overlay';
import {Location} from '@angular/common';
import {Observable, Subject} from 'rxjs';
@@ -78,8 +78,13 @@ export class MatDialogRef<T, R = any> {
});
_overlayRef.keydownEvents()
.pipe(filter(event => event.keyCode === ESCAPE && !this.disableClose))
.subscribe(() => this.close());
.pipe(filter(event => {
return event.keyCode === ESCAPE && !this.disableClose && !hasModifierKey(event);
}))
.subscribe(event => {
event.preventDefault();
this.close();
});
}
/**
+17 -2
View File
@@ -27,7 +27,7 @@ import {MatDialogContainer} from './dialog-container';
import {OverlayContainer, ScrollStrategy, Overlay} from '@angular/cdk/overlay';
import {ScrollDispatcher} from '@angular/cdk/scrolling';
import {A, ESCAPE} from '@angular/cdk/keycodes';
import {dispatchKeyboardEvent} from '@angular/cdk/testing';
import {dispatchKeyboardEvent, createKeyboardEvent} from '@angular/cdk/testing';
import {
MAT_DIALOG_DATA,
MatDialog,
@@ -240,11 +240,26 @@ describe('MatDialog', () => {
viewContainerRef: testViewContainerRef
});
dispatchKeyboardEvent(document.body, 'keydown', ESCAPE);
const event = dispatchKeyboardEvent(document.body, 'keydown', ESCAPE);
viewContainerFixture.detectChanges();
flush();
expect(overlayContainerElement.querySelector('mat-dialog-container')).toBeNull();
expect(event.defaultPrevented).toBe(true);
}));
it('should not close a dialog via the escape key with a modifier', fakeAsync(() => {
dialog.open(PizzaMsg, {
viewContainerRef: testViewContainerRef
});
const event = createKeyboardEvent('keydown', ESCAPE);
Object.defineProperty(event, 'altKey', {get: () => true});
viewContainerFixture.detectChanges();
flush();
expect(overlayContainerElement.querySelector('mat-dialog-container')).toBeTruthy();
expect(event.defaultPrevented).toBe(false);
}));
it('should close from a ViewContainerRef with OnPush change detection', fakeAsync(() => {
+18
View File
@@ -347,6 +347,24 @@ describe('MatMenu', () => {
tick(500);
expect(overlayContainerElement.textContent).toBe('');
expect(event.defaultPrevented).toBe(true);
}));
it('should not close the menu when pressing ESCAPE with a modifier', fakeAsync(() => {
const fixture = createComponent(SimpleMenu, [], [FakeIcon]);
fixture.detectChanges();
fixture.componentInstance.trigger.openMenu();
const panel = overlayContainerElement.querySelector('.mat-menu-panel')!;
const event = createKeyboardEvent('keydown', ESCAPE);
Object.defineProperty(event, 'altKey', {get: () => true});
dispatchEvent(panel, event);
fixture.detectChanges();
tick(500);
expect(overlayContainerElement.textContent).toBeTruthy();
expect(event.defaultPrevented).toBe(false);
}));
it('should open a custom menu', () => {
+4 -1
View File
@@ -266,7 +266,10 @@ export class _MatMenuBase implements AfterContentInit, MatMenuPanel<MatMenuItem>
switch (keyCode) {
case ESCAPE:
this.closed.emit('keydown');
if (!hasModifierKey(event)) {
event.preventDefault();
this.closed.emit('keydown');
}
break;
case LEFT_ARROW:
if (this.parentMenu && this.direction === 'ltr') {
+29 -2
View File
@@ -15,7 +15,7 @@ import {Direction} from '@angular/cdk/bidi';
import {A11yModule} from '@angular/cdk/a11y';
import {PlatformModule} from '@angular/cdk/platform';
import {ESCAPE} from '@angular/cdk/keycodes';
import {dispatchKeyboardEvent} from '@angular/cdk/testing';
import {dispatchKeyboardEvent, createKeyboardEvent, dispatchEvent} from '@angular/cdk/testing';
import {CdkScrollable} from '@angular/cdk/scrolling';
@@ -202,12 +202,39 @@ describe('MatDrawer', () => {
expect(testComponent.closeCount).toBe(0, 'Expected no close events.');
expect(testComponent.closeStartCount).toBe(0, 'Expected no close start events.');
dispatchKeyboardEvent(drawer.nativeElement, 'keydown', ESCAPE);
const event = dispatchKeyboardEvent(drawer.nativeElement, 'keydown', ESCAPE);
fixture.detectChanges();
flush();
expect(testComponent.closeCount).toBe(1, 'Expected one close event.');
expect(testComponent.closeStartCount).toBe(1, 'Expected one close start event.');
expect(event.defaultPrevented).toBe(true);
}));
it('should not close when pressing escape with a modifier', fakeAsync(() => {
let fixture = TestBed.createComponent(BasicTestApp);
fixture.detectChanges();
let testComponent: BasicTestApp = fixture.debugElement.componentInstance;
let drawer = fixture.debugElement.query(By.directive(MatDrawer));
drawer.componentInstance.open();
fixture.detectChanges();
tick();
expect(testComponent.closeCount).toBe(0, 'Expected no close events.');
expect(testComponent.closeStartCount).toBe(0, 'Expected no close start events.');
const event = createKeyboardEvent('keydown', ESCAPE);
Object.defineProperty(event, 'altKey', {get: () => true});
dispatchEvent(drawer.nativeElement, event);
fixture.detectChanges();
flush();
expect(testComponent.closeCount).toBe(0, 'Expected still no close events.');
expect(testComponent.closeStartCount).toBe(0, 'Expected still no close start events.');
expect(event.defaultPrevented).toBe(false);
}));
it('should fire the open event when open on init', fakeAsync(() => {
+5 -2
View File
@@ -9,7 +9,7 @@ import {AnimationEvent} from '@angular/animations';
import {FocusMonitor, FocusOrigin, FocusTrap, FocusTrapFactory} from '@angular/cdk/a11y';
import {Directionality} from '@angular/cdk/bidi';
import {coerceBooleanProperty} from '@angular/cdk/coercion';
import {ESCAPE} from '@angular/cdk/keycodes';
import {ESCAPE, hasModifierKey} from '@angular/cdk/keycodes';
import {Platform} from '@angular/cdk/platform';
import {CdkScrollable, ScrollDispatcher, ViewportRuler} from '@angular/cdk/scrolling';
import {DOCUMENT} from '@angular/common';
@@ -267,11 +267,14 @@ export class MatDrawer implements AfterContentInit, AfterContentChecked, OnDestr
*/
this._ngZone.runOutsideAngular(() => {
(fromEvent(this._elementRef.nativeElement, 'keydown') as Observable<KeyboardEvent>).pipe(
filter(event => event.keyCode === ESCAPE && !this.disableClose),
filter(event => {
return event.keyCode === ESCAPE && !this.disableClose && !hasModifierKey(event);
}),
takeUntil(this._destroyed)
).subscribe(event => this._ngZone.run(() => {
this.close();
event.stopPropagation();
event.preventDefault();
}));
});
+28
View File
@@ -27,6 +27,8 @@ import {
dispatchKeyboardEvent,
patchElementFocus,
dispatchMouseEvent,
createKeyboardEvent,
dispatchEvent,
} from '@angular/cdk/testing';
import {ESCAPE} from '@angular/cdk/keycodes';
import {FocusMonitor} from '@angular/cdk/a11y';
@@ -635,6 +637,32 @@ describe('MatTooltip', () => {
flush();
}));
it('should preventDefault when pressing ESCAPE', fakeAsync(() => {
tooltipDirective.show();
tick(0);
fixture.detectChanges();
const event = dispatchKeyboardEvent(buttonElement, 'keydown', ESCAPE);
fixture.detectChanges();
flush();
expect(event.defaultPrevented).toBe(true);
}));
it('should not preventDefault when pressing ESCAPE with a modifier', fakeAsync(() => {
tooltipDirective.show();
tick(0);
fixture.detectChanges();
const event = createKeyboardEvent('keydown', ESCAPE);
Object.defineProperty(event, 'altKey', {get: () => true});
dispatchEvent(buttonElement, event);
fixture.detectChanges();
flush();
expect(event.defaultPrevented).toBe(false);
}));
it('should not show the tooltip on progammatic focus', fakeAsync(() => {
patchElementFocus(buttonElement);
assertTooltipInstance(tooltipDirective, false);
+3 -2
View File
@@ -9,7 +9,7 @@ import {AnimationEvent} from '@angular/animations';
import {AriaDescriber, FocusMonitor} from '@angular/cdk/a11y';
import {Directionality} from '@angular/cdk/bidi';
import {coerceBooleanProperty} from '@angular/cdk/coercion';
import {ESCAPE} from '@angular/cdk/keycodes';
import {ESCAPE, hasModifierKey} from '@angular/cdk/keycodes';
import {BreakpointObserver, Breakpoints, BreakpointState} from '@angular/cdk/layout';
import {
FlexibleConnectedPositionStrategy,
@@ -331,7 +331,8 @@ export class MatTooltip implements OnDestroy, OnInit {
/** Handles the keydown events on the host element. */
_handleKeydown(e: KeyboardEvent) {
if (this._isTooltipVisible() && e.keyCode === ESCAPE) {
if (this._isTooltipVisible() && e.keyCode === ESCAPE && !hasModifierKey(e)) {
e.preventDefault();
e.stopPropagation();
this.hide(0);
}