diff --git a/src/cdk-experimental/popover-edit/edit-ref.ts b/src/cdk-experimental/popover-edit/edit-ref.ts index 9cd0edc03cd2..baf77a3c6c4e 100644 --- a/src/cdk-experimental/popover-edit/edit-ref.ts +++ b/src/cdk-experimental/popover-edit/edit-ref.ts @@ -30,15 +30,6 @@ export class EditRef implements OnDestroy { /** The value to set the form back to on revert. */ private _revertFormValue: FormValue; - /** - * The flags are used to track whether a keyboard enter press is in progress at the same time - * as other events that would cause the edit lens to close. We must track this so that the - * Enter keyup event does not fire after we close as it would cause the edit to immediately - * reopen. - */ - private _enterPressed = false; - private _closePending = false; - constructor( @Self() private readonly _form: ControlContainer, private readonly _editEventDispatcher: EditEventDispatcher, @@ -88,34 +79,6 @@ export class EditRef implements OnDestroy { this._blurredSubject.next(); } - /** - * Closes the edit if the enter key is not down. - * Otherwise, sets _closePending to true so that the edit will close on the - * next enter keyup. - */ - closeAfterEnterKeypress(): void { - // If the enter key is currently pressed, delay closing the popup so that - // the keyUp event does not cause it to immediately reopen. - if (this._enterPressed) { - this._closePending = true; - } else { - this.close(); - } - } - - /** - * Called on Enter keyup/keydown. - * Closes the edit if pending. Otherwise just updates _enterPressed. - */ - trackEnterPressForClose(pressed: boolean): void { - if (this._closePending) { - this.close(); - return; - } - - this._enterPressed = pressed; - } - /** * Resets the form value to the specified value or the previously set * revert value. diff --git a/src/cdk-experimental/popover-edit/lens-directives.ts b/src/cdk-experimental/popover-edit/lens-directives.ts index d5128287cc26..8a5aed6f171f 100644 --- a/src/cdk-experimental/popover-edit/lens-directives.ts +++ b/src/cdk-experimental/popover-edit/lens-directives.ts @@ -89,7 +89,7 @@ export class CdkEditControl implements OnDestroy, OnInit { if (this.ignoreSubmitUnlessValid && !this.editRef.isValid()) { return; } this.editRef.updateRevertValue(); - this.editRef.closeAfterEnterKeypress(); + this.editRef.close(); } /** Called on Escape keyup. Closes the edit. */ @@ -128,21 +128,9 @@ export class CdkEditControl implements OnDestroy, OnInit { // to avoid double event listeners, we need to use `HostListener`. Once Ivy is the default, we // can move this back into `host`. // tslint:disable:no-host-decorator-in-concrete - @HostListener('keydown') - _handleKeydown() { - this.editRef.trackEnterPressForClose(true); - } - - // In Ivy the `host` metadata will be merged, whereas in ViewEngine it is overridden. In order - // to avoid double event listeners, we need to use `HostListener`. Once Ivy is the default, we - // can move this back into `host`. - // tslint:disable:no-host-decorator-in-concrete - @HostListener('keyup', ['$event']) - _handleKeyup(event: KeyboardEvent) { - // TODO(crisbeto): should use cdk/keycodes once the tests are reworked to use cdk/testing. - if (event.key === 'Enter') { - this.editRef.trackEnterPressForClose(false); - } else if (event.key === 'Escape') { + @HostListener('keydown', ['$event']) + _handleKeydown(event: KeyboardEvent) { + if (event.key === 'Escape') { this.close(); } } @@ -204,6 +192,8 @@ export class CdkEditClose { // tslint:disable:no-host-decorator-in-concrete @HostListener('click') closeEdit(): void { - this.editRef.closeAfterEnterKeypress(); + // Note that we use `click` here, rather than a keyboard event, because some screen readers + // will emit a fake click event instead of an enter keyboard event on buttons. + this.editRef.close(); } } diff --git a/src/cdk-experimental/popover-edit/popover-edit.spec.ts b/src/cdk-experimental/popover-edit/popover-edit.spec.ts index 51d15b78bd60..6a9d2a65b127 100644 --- a/src/cdk-experimental/popover-edit/popover-edit.spec.ts +++ b/src/cdk-experimental/popover-edit/popover-edit.spec.ts @@ -120,7 +120,7 @@ abstract class BaseTestComponent { openLens(rowIndex = 0, cellIndex = 1) { this.focusEditCell(rowIndex, cellIndex); this.getEditCell(rowIndex, cellIndex) - .dispatchEvent(new KeyboardEvent('keyup', {bubbles: true, key: 'Enter'})); + .dispatchEvent(new KeyboardEvent('keydown', {bubbles: true, key: 'Enter'})); flush(); } @@ -772,7 +772,7 @@ cdkPopoverEditTabOut`, fakeAsync(() => { component.openLens(); component.getNameInput()!.dispatchEvent( - new KeyboardEvent('keyup', {bubbles: true, key: 'Escape'})); + new KeyboardEvent('keydown', {bubbles: true, key: 'Escape'})); expect(component.lensIsOpen()).toBe(false); clearLeftoverTimers(); diff --git a/src/cdk-experimental/popover-edit/table-directives.ts b/src/cdk-experimental/popover-edit/table-directives.ts index b619accd679b..79f1f0043fc7 100644 --- a/src/cdk-experimental/popover-edit/table-directives.ts +++ b/src/cdk-experimental/popover-edit/table-directives.ts @@ -133,7 +133,7 @@ export class CdkEditable implements AfterViewInit, OnDestroy { share(), ).subscribe(this.editEventDispatcher.allRows); - fromEvent(element, 'keyup').pipe( + fromEvent(element, 'keydown').pipe( filter(event => event.key === 'Enter'), toClosest(CELL_SELECTOR), takeUntil(this.destroyed), @@ -276,7 +276,14 @@ export class CdkPopoverEdit implements AfterViewInit, OnDestroy { this.template!, this.viewContainerRef, {$implicit: this.context})); - this.focusTrap!.focusInitialElement(); + + // We have to defer trapping focus, because doing so too early can cause the form inside + // the overlay to be submitted immediately if it was opened on an Enter keydown event. + this.services.ngZone.runOutsideAngular(() => { + setTimeout(() => { + this.focusTrap!.focusInitialElement(); + }); + }); // Update the size of the popup initially and on subsequent changes to // scroll position and viewport size. diff --git a/src/material-experimental/popover-edit/popover-edit.spec.ts b/src/material-experimental/popover-edit/popover-edit.spec.ts index 50a455eca878..9a5e24bb66d7 100644 --- a/src/material-experimental/popover-edit/popover-edit.spec.ts +++ b/src/material-experimental/popover-edit/popover-edit.spec.ts @@ -117,7 +117,7 @@ abstract class BaseTestComponent { openLens(rowIndex = 0, cellIndex = 1) { this.focusEditCell(rowIndex, cellIndex); this.getEditCell(rowIndex, cellIndex) - .dispatchEvent(new KeyboardEvent('keyup', {bubbles: true, key: 'Enter'})); + .dispatchEvent(new KeyboardEvent('keydown', {bubbles: true, key: 'Enter'})); flush(); } @@ -691,7 +691,7 @@ matPopoverEditTabOut`, fakeAsync(() => { component.openLens(); component.getInput()!.dispatchEvent( - new KeyboardEvent('keyup', {bubbles: true, key: 'Escape'})); + new KeyboardEvent('keydown', {bubbles: true, key: 'Escape'})); expect(component.lensIsOpen()).toBe(false); clearLeftoverTimers();