From 5bb81e84becbb0ebf729ac1f2b86b9114acd2962 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Fri, 8 May 2020 21:10:56 +0200 Subject: [PATCH] fix(dialog): focus trap not attached if autoFocus is disabled (#19251) In #18826 some code was moved around, and as a result, the focus trap was being created lazily only when we interact with its API. The problem is that this means that focus trapping will be disabled if `autoFocus` was turned off. Fixes #19246. --- src/material/dialog/dialog-container.ts | 30 ++++++++++++------------- src/material/dialog/dialog.spec.ts | 13 +++++++++++ 2 files changed, 27 insertions(+), 16 deletions(-) diff --git a/src/material/dialog/dialog-container.ts b/src/material/dialog/dialog-container.ts index a4af074ad51c..a13c984533ba 100644 --- a/src/material/dialog/dialog-container.ts +++ b/src/material/dialog/dialog-container.ts @@ -116,7 +116,7 @@ export class MatDialogContainer extends BasePortalOutlet { throwMatDialogContentAlreadyAttachedError(); } - this._savePreviouslyFocusedElement(); + this._setupFocusTrap(); return this._portalOutlet.attachComponentPortal(portal); } @@ -129,7 +129,7 @@ export class MatDialogContainer extends BasePortalOutlet { throwMatDialogContentAlreadyAttachedError(); } - this._savePreviouslyFocusedElement(); + this._setupFocusTrap(); return this._portalOutlet.attachTemplatePortal(portal); } @@ -144,14 +144,14 @@ export class MatDialogContainer extends BasePortalOutlet { throwMatDialogContentAlreadyAttachedError(); } - this._savePreviouslyFocusedElement(); + this._setupFocusTrap(); return this._portalOutlet.attachDomPortal(portal); } /** Moves focus back into the dialog if it was moved out. */ _recaptureFocus() { if (!this._containsFocus()) { - const focusWasTrapped = this._getFocusTrap().focusInitialElement(); + const focusWasTrapped = this._focusTrap.focusInitialElement(); if (!focusWasTrapped) { this._elementRef.nativeElement.focus(); @@ -165,7 +165,7 @@ export class MatDialogContainer extends BasePortalOutlet { // ready in instances where change detection has to run first. To deal with this, we simply // wait for the microtask queue to be empty. if (this._config.autoFocus) { - this._getFocusTrap().focusInitialElementWhenReady(); + this._focusTrap.focusInitialElementWhenReady(); } else if (!this._containsFocus()) { // Otherwise ensure that focus is on the dialog container. It's possible that a different // component tried to move focus while the open animation was running. See: @@ -200,8 +200,15 @@ export class MatDialogContainer extends BasePortalOutlet { } } - /** Saves a reference to the element that was focused before the dialog was opened. */ - private _savePreviouslyFocusedElement() { + /** + * Sets up the focus trand and saves a reference to the + * element that was focused before the dialog was opened. + */ + private _setupFocusTrap() { + if (!this._focusTrap) { + this._focusTrap = this._focusTrapFactory.create(this._elementRef.nativeElement); + } + if (this._document) { this._elementFocusedBeforeDialogWasOpened = this._document.activeElement as HTMLElement; @@ -222,15 +229,6 @@ export class MatDialogContainer extends BasePortalOutlet { return element === activeElement || element.contains(activeElement); } - /** Gets the focus trap associated with the dialog. */ - private _getFocusTrap() { - if (!this._focusTrap) { - this._focusTrap = this._focusTrapFactory.create(this._elementRef.nativeElement); - } - - return this._focusTrap; - } - /** Callback, invoked whenever an animation on the host completes. */ _onAnimationDone(event: AnimationEvent) { if (event.toState === 'enter') { diff --git a/src/material/dialog/dialog.spec.ts b/src/material/dialog/dialog.spec.ts index 6a326dd1174c..fa4a2b948ad5 100644 --- a/src/material/dialog/dialog.spec.ts +++ b/src/material/dialog/dialog.spec.ts @@ -1075,6 +1075,19 @@ describe('MatDialog', () => { expect(document.activeElement!.tagName).not.toBe('INPUT'); })); + it('should attach the focus trap even if automatic focus is disabled', fakeAsync(() => { + dialog.open(PizzaMsg, { + viewContainerRef: testViewContainerRef, + autoFocus: false + }); + + viewContainerFixture.detectChanges(); + flushMicrotasks(); + + expect(overlayContainerElement.querySelectorAll('.cdk-focus-trap-anchor').length) + .toBeGreaterThan(0); + })); + it('should re-focus trigger element when dialog closes', fakeAsync(() => { // Create a element that has focus before the dialog is opened. let button = document.createElement('button');