fix(popover-edit): unable to close by pressing on enter on some screen readers (#16466)

* Fixes not being able to close the overlay by pressing Enter on the close button when using some screen readers (tested on NVDA, but I expect it to work similarly in others). The issue comes from the fact that we keep track of whether the user is pressing Enter through `keyup` and `keydown` events, however screen readers emit fake `click` events on buttons instead of `keyup` and `keydown`.
* Fixes not being able to close the overlay by clicking on the close button with a mouse, if the keyboard was used to reach the button (e.g. by tabbing to it). This issue also comes from the fact that we were keeping track of keyboard events and assuming that the user will continue using a keyboard to close the overlay.
* Gets rid of a bunch of keyboard event tracking logic.

**Disclaimer:** I had to introduce a `setTimeout` when trapping focus inside the overlay, in order to prevent the form inside the overlay from being submitted immediately when opening through a `keydown` on Enter. From what I can tell this shouldn't really have an effect on existing tests since we have other timers that are running at the same time.

Author: crisbeto

src/cdk-experimental/popover-edit/edit-ref.ts

@@ -30,15 +30,6 @@ export class EditRef<FormValue> 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<FormValue> 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.

src/cdk-experimental/popover-edit/lens-directives.ts

@@ -89,7 +89,7 @@ export class CdkEditControl<FormValue> 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<FormValue> 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<FormValue> {
   // 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();
   }
 }

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();

src/cdk-experimental/popover-edit/table-directives.ts

@@ -133,7 +133,7 @@ export class CdkEditable implements AfterViewInit, OnDestroy {
           share(),
           ).subscribe(this.editEventDispatcher.allRows);
 
-      fromEvent<KeyboardEvent>(element, 'keyup').pipe(
+      fromEvent<KeyboardEvent>(element, 'keydown').pipe(
           filter(event => event.key === 'Enter'),
           toClosest(CELL_SELECTOR),
           takeUntil(this.destroyed),
@@ -276,7 +276,14 @@ export class CdkPopoverEdit<C> 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.

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();