From 2d93c6d7fd6f79c72357c703cea12f4b7fe60e31 Mon Sep 17 00:00:00 2001 From: Steven Xu Date: Tue, 4 Jun 2019 09:42:09 -0700 Subject: [PATCH] fix(overlay): stop using capture phase for overlay keyboard handling (#16019) OverlayKeyboardDispatcher is a document-level keyboard listener that dispatches events to the topmost overlay that's listening for them. It does it on capture as of e30852aa in order to avoid missing events whose propagation have been stopped. However, there are legitimate use cases for stopping propagation and not wanting the overlay to hear about it. For example, grid navigation based on the ARIA best practice allows for enter to trap focus inside a grid cell and escape to leave it and restore focus to the grid cell. https://www.w3.org/TR/wai-aria-practices-1.1/#gridNav_inside In these cases, we must be able to call stopPropagation and prevent the escape keypress event from reaching the overlay. Fixes #9928. BREAKING CHANGE: OverlayKeyboardDispatcher (used in dialog and overlay and available in the CDK) now listens on bubbling/propagation phase instead of capture phase. This means that overlay keydown handlers are now called after any applicable handlers inside of an overlay, and it respects any stopPropagation() calls from inside the overlay. --- .../overlay/keyboard/overlay-keyboard-dispatcher.spec.ts | 8 ++++---- src/cdk/overlay/keyboard/overlay-keyboard-dispatcher.ts | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/cdk/overlay/keyboard/overlay-keyboard-dispatcher.spec.ts b/src/cdk/overlay/keyboard/overlay-keyboard-dispatcher.spec.ts index aca87a42395..24c615b8223 100644 --- a/src/cdk/overlay/keyboard/overlay-keyboard-dispatcher.spec.ts +++ b/src/cdk/overlay/keyboard/overlay-keyboard-dispatcher.spec.ts @@ -69,7 +69,7 @@ describe('OverlayKeyboardDispatcher', () => { expect(overlayTwoSpy).toHaveBeenCalled(); }); - it('should dispatch keyboard events when propagation is stopped', () => { + it('should not dispatch keyboard events when propagation is stopped', () => { const overlayRef = overlay.create(); const spy = jasmine.createSpy('keyboard event spy'); const button = document.createElement('button'); @@ -81,7 +81,7 @@ describe('OverlayKeyboardDispatcher', () => { keyboardDispatcher.add(overlayRef); dispatchKeyboardEvent(button, 'keydown', ESCAPE); - expect(spy).toHaveBeenCalled(); + expect(spy).not.toHaveBeenCalled(); button.parentNode!.removeChild(button); }); @@ -137,10 +137,10 @@ describe('OverlayKeyboardDispatcher', () => { spyOn(body, 'removeEventListener'); keyboardDispatcher.add(overlayRef); - expect(body.addEventListener).toHaveBeenCalledWith('keydown', jasmine.any(Function), true); + expect(body.addEventListener).toHaveBeenCalledWith('keydown', jasmine.any(Function)); overlayRef.dispose(); - expect(body.removeEventListener).toHaveBeenCalledWith('keydown', jasmine.any(Function), true); + expect(body.removeEventListener).toHaveBeenCalledWith('keydown', jasmine.any(Function)); }); it('should skip overlays that do not have keydown event subscriptions', () => { diff --git a/src/cdk/overlay/keyboard/overlay-keyboard-dispatcher.ts b/src/cdk/overlay/keyboard/overlay-keyboard-dispatcher.ts index 9a6783b73cf..e4571e72e6c 100644 --- a/src/cdk/overlay/keyboard/overlay-keyboard-dispatcher.ts +++ b/src/cdk/overlay/keyboard/overlay-keyboard-dispatcher.ts @@ -47,7 +47,7 @@ export class OverlayKeyboardDispatcher implements OnDestroy { // Lazily start dispatcher once first overlay is added if (!this._isAttached) { - this._document.body.addEventListener('keydown', this._keydownListener, true); + this._document.body.addEventListener('keydown', this._keydownListener); this._isAttached = true; } @@ -71,7 +71,7 @@ export class OverlayKeyboardDispatcher implements OnDestroy { /** Detaches the global keyboard event listener. */ private _detach() { if (this._isAttached) { - this._document.body.removeEventListener('keydown', this._keydownListener, true); + this._document.body.removeEventListener('keydown', this._keydownListener); this._isAttached = false; } }