Skip to content

Commit

Permalink
fix(dialog): focus trap not attached if autoFocus is disabled (#19251)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
crisbeto authored May 8, 2020
1 parent 703b1da commit 5bb81e8
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 16 deletions.
30 changes: 14 additions & 16 deletions src/material/dialog/dialog-container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ export class MatDialogContainer extends BasePortalOutlet {
throwMatDialogContentAlreadyAttachedError();
}

this._savePreviouslyFocusedElement();
this._setupFocusTrap();
return this._portalOutlet.attachComponentPortal(portal);
}

Expand All @@ -129,7 +129,7 @@ export class MatDialogContainer extends BasePortalOutlet {
throwMatDialogContentAlreadyAttachedError();
}

this._savePreviouslyFocusedElement();
this._setupFocusTrap();
return this._portalOutlet.attachTemplatePortal(portal);
}

Expand All @@ -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();
Expand All @@ -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:
Expand Down Expand Up @@ -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;

Expand All @@ -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') {
Expand Down
13 changes: 13 additions & 0 deletions src/material/dialog/dialog.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down

0 comments on commit 5bb81e8

Please sign in to comment.