Skip to content

Commit 442ec30

Browse files
crisbetommalerba
authored andcommitted
fix(cdk-experimental/dialog): don't move focus if it was moved during close animation (#17320)
Along the same lines as #17300. Restores focus only if focus is inside the dialog at the time the exit animation has finished. This avoids overriding focus that was moved while the dialog is animating away.
1 parent 037baff commit 442ec30

File tree

2 files changed

+56
-2
lines changed

2 files changed

+56
-2
lines changed

src/cdk-experimental/dialog/dialog-container.ts

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,8 @@ export function throwDialogContentAlreadyAttachedError() {
7373
},
7474
})
7575
export class CdkDialogContainer extends BasePortalOutlet implements OnDestroy {
76+
private _document: Document;
77+
7678
/** State of the dialog animation. */
7779
_state: 'void' | 'enter' | 'exit' = 'enter';
7880

@@ -120,11 +122,13 @@ export class CdkDialogContainer extends BasePortalOutlet implements OnDestroy {
120122
private _elementRef: ElementRef<HTMLElement>,
121123
private _focusTrapFactory: FocusTrapFactory,
122124
private _changeDetectorRef: ChangeDetectorRef,
123-
@Optional() @Inject(DOCUMENT) private _document: any,
125+
@Optional() @Inject(DOCUMENT) _document: any,
124126
/** The dialog configuration. */
125127
public _config: DialogConfig) {
126128
super();
127129

130+
this._document = _document;
131+
128132
// We use a Subject with a distinctUntilChanged, rather than a callback attached to .done,
129133
// because some browsers fire the done event twice and we don't want to emit duplicate events.
130134
// See: https://github.com/angular/angular/issues/24084
@@ -248,7 +252,17 @@ export class CdkDialogContainer extends BasePortalOutlet implements OnDestroy {
248252
const toFocus = this._elementFocusedBeforeDialogWasOpened;
249253
// We need the extra check, because IE can set the `activeElement` to null in some cases.
250254
if (toFocus && typeof toFocus.focus === 'function') {
251-
toFocus.focus();
255+
const activeElement = this._document.activeElement;
256+
const element = this._elementRef.nativeElement;
257+
258+
// Make sure that focus is still inside the dialog or is on the body (usually because a
259+
// non-focusable element like the backdrop was clicked) before moving it. It's possible that
260+
// the consumer moved it themselves before the animation was done, in which case we shouldn't
261+
// do anything.
262+
if (!activeElement || activeElement === this._document.body || activeElement === element ||
263+
element.contains(activeElement)) {
264+
toFocus.focus();
265+
}
252266
}
253267
}
254268
}

src/cdk-experimental/dialog/dialog.spec.ts

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -910,6 +910,46 @@ describe('Dialog', () => {
910910
document.body.removeChild(button);
911911
}));
912912

913+
it('should not move focus if it was moved outside the dialog while animating', fakeAsync(() => {
914+
// Create a element that has focus before the dialog is opened.
915+
const button = document.createElement('button');
916+
const otherButton = document.createElement('button');
917+
const body = document.body;
918+
button.id = 'dialog-trigger';
919+
otherButton.id = 'other-button';
920+
body.appendChild(button);
921+
body.appendChild(otherButton);
922+
button.focus();
923+
924+
const dialogRef = dialog.openFromComponent(PizzaMsg, {
925+
viewContainerRef: testViewContainerRef
926+
});
927+
928+
flushMicrotasks();
929+
viewContainerFixture.detectChanges();
930+
flushMicrotasks();
931+
932+
expect(document.activeElement!.id)
933+
.not.toBe('dialog-trigger', 'Expected the focus to change when dialog was opened.');
934+
935+
// Start the closing sequence and move focus out of dialog.
936+
dialogRef.close();
937+
otherButton.focus();
938+
939+
expect(document.activeElement!.id)
940+
.toBe('other-button', 'Expected focus to be on the alternate button.');
941+
942+
flushMicrotasks();
943+
viewContainerFixture.detectChanges();
944+
flush();
945+
946+
expect(document.activeElement!.id)
947+
.toBe('other-button', 'Expected focus to stay on the alternate button.');
948+
949+
body.removeChild(button);
950+
body.removeChild(otherButton);
951+
}));
952+
913953
it('should allow the consumer to shift focus in afterClosed', fakeAsync(() => {
914954
// Create a element that has focus before the dialog is opened.
915955
let button = document.createElement('button');

0 commit comments

Comments
 (0)