Skip to content

refactor(material/dialog): switch to CDK dialog internally #24857

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/cdk/dialog/dialog-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,12 @@ export class DialogConfig<D = unknown, R = unknown, C extends BasePortalOutlet =
*/
closeOnNavigation?: boolean = true;

/**
* Whether the dialog should close when the dialog service is destroyed. This is useful if
* another service is wrapping the dialog and is managing the destruction instead.
*/
closeOnDestroy?: boolean = true;

/** Alternate `ComponentFactoryResolver` to use when resolving the associated component. */
componentFactoryResolver?: ComponentFactoryResolver;

Expand Down
29 changes: 21 additions & 8 deletions src/cdk/dialog/dialog-container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import {
} from '@angular/cdk/portal';
import {DOCUMENT} from '@angular/common';
import {
AfterViewInit,
ChangeDetectionStrategy,
Component,
ComponentRef,
Expand Down Expand Up @@ -67,7 +66,7 @@ export function throwDialogContentAlreadyAttachedError() {
})
export class CdkDialogContainer<C extends DialogConfig = DialogConfig>
extends BasePortalOutlet
implements AfterViewInit, OnDestroy
implements OnDestroy
{
protected _document: Document;

Expand Down Expand Up @@ -105,7 +104,7 @@ export class CdkDialogContainer<C extends DialogConfig = DialogConfig>
this._document = _document;
}

ngAfterViewInit() {
protected _contentAttached() {
this._initializeFocusTrap();
this._handleBackdropClicks();
this._captureInitialFocus();
Expand All @@ -132,7 +131,9 @@ export class CdkDialogContainer<C extends DialogConfig = DialogConfig>
throwDialogContentAlreadyAttachedError();
}

return this._portalOutlet.attachComponentPortal(portal);
const result = this._portalOutlet.attachComponentPortal(portal);
this._contentAttached();
return result;
}

/**
Expand All @@ -144,7 +145,9 @@ export class CdkDialogContainer<C extends DialogConfig = DialogConfig>
throwDialogContentAlreadyAttachedError();
}

return this._portalOutlet.attachTemplatePortal(portal);
const result = this._portalOutlet.attachTemplatePortal(portal);
this._contentAttached();
return result;
}

/**
Expand All @@ -158,9 +161,19 @@ export class CdkDialogContainer<C extends DialogConfig = DialogConfig>
throwDialogContentAlreadyAttachedError();
}

return this._portalOutlet.attachDomPortal(portal);
const result = this._portalOutlet.attachDomPortal(portal);
this._contentAttached();
return result;
};

// TODO(crisbeto): this shouldn't be exposed, but there are internal references to it.
/** Captures focus if it isn't already inside the dialog. */
_recaptureFocus() {
if (!this._containsFocus()) {
this._trapFocus();
}
}

/**
* Focuses the provided element. If the element is not focusable, it will add a tabIndex
* attribute to forcefully focus it. The attribute is removed after focus is moved.
Expand Down Expand Up @@ -316,8 +329,8 @@ export class CdkDialogContainer<C extends DialogConfig = DialogConfig>
// Clicking on the backdrop will move focus out of dialog.
// Recapture it if closing via the backdrop is disabled.
this._overlayRef.backdropClick().subscribe(() => {
if (this._config.disableClose && !this._containsFocus()) {
this._trapFocus();
if (this._config.disableClose) {
this._recaptureFocus();
}
});
}
Expand Down
56 changes: 36 additions & 20 deletions src/cdk/dialog/dialog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ export class Dialog implements OnDestroy {
}

(this.openDialogs as DialogRef<R, C>[]).push(dialogRef);
dialogRef.closed.subscribe(() => this._removeOpenDialog(dialogRef));
dialogRef.closed.subscribe(() => this._removeOpenDialog(dialogRef, true));
this.afterOpened.next(dialogRef);

return dialogRef;
Expand All @@ -148,7 +148,7 @@ export class Dialog implements OnDestroy {
* Closes all of the currently-open dialogs.
*/
closeAll(): void {
this._closeDialogs(this.openDialogs);
reverseForEach(this.openDialogs, dialog => dialog.close());
}

/**
Expand All @@ -160,11 +160,24 @@ export class Dialog implements OnDestroy {
}

ngOnDestroy() {
// Only close the dialogs at this level on destroy
// since the parent service may still be active.
this._closeDialogs(this._openDialogsAtThisLevel);
// Make one pass over all the dialogs that need to be untracked, but should not be closed. We
// want to stop tracking the open dialog even if it hasn't been closed, because the tracking
// determines when `aria-hidden` is removed from elements outside the dialog.
reverseForEach(this._openDialogsAtThisLevel, dialog => {
// Check for `false` specifically since we want `undefined` to be interpreted as `true`.
if (dialog.config.closeOnDestroy === false) {
this._removeOpenDialog(dialog, false);
}
});

// Make a second pass and close the remaining dialogs. We do this second pass in order to
// correctly dispatch the `afterAllClosed` event in case we have a mixed array of dialogs
// that should be closed and dialogs that should not.
reverseForEach(this._openDialogsAtThisLevel, dialog => dialog.close());

this._afterAllClosedAtThisLevel.complete();
this._afterOpenedAtThisLevel.complete();
this._openDialogsAtThisLevel = [];
}

/**
Expand Down Expand Up @@ -326,8 +339,9 @@ export class Dialog implements OnDestroy {
/**
* Removes a dialog from the array of open dialogs.
* @param dialogRef Dialog to be removed.
* @param emitEvent Whether to emit an event if this is the last dialog.
*/
private _removeOpenDialog<R, C>(dialogRef: DialogRef<R, C>) {
private _removeOpenDialog<R, C>(dialogRef: DialogRef<R, C>, emitEvent: boolean) {
const index = this.openDialogs.indexOf(dialogRef);

if (index > -1) {
Expand All @@ -345,7 +359,10 @@ export class Dialog implements OnDestroy {
});

this._ariaHiddenElements.clear();
this._getAfterAllClosed().next();

if (emitEvent) {
this._getAfterAllClosed().next();
}
}
}
}
Expand Down Expand Up @@ -374,21 +391,20 @@ export class Dialog implements OnDestroy {
}
}

/** Closes all of the dialogs in an array. */
private _closeDialogs(dialogs: readonly DialogRef<unknown>[]) {
let i = dialogs.length;

while (i--) {
// The `_openDialogs` property isn't updated after close until the rxjs subscription
// runs on the next microtask, in addition to modifying the array as we're going
// through it. We loop through all of them and call close without assuming that
// they'll be removed from the list instantaneously.
dialogs[i].close();
}
}

private _getAfterAllClosed(): Subject<void> {
const parent = this._parentDialog;
return parent ? parent._getAfterAllClosed() : this._afterAllClosedAtThisLevel;
}
}

/**
* Executes a callback against all elements in an array while iterating in reverse.
* Useful if the array is being modified as it is being iterated.
*/
function reverseForEach<T>(items: T[] | readonly T[], callback: (current: T) => void) {
let i = items.length;

while (i--) {
callback(items[i]);
}
}
1 change: 1 addition & 0 deletions src/material-experimental/mdc-dialog/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ ng_test_library(
":mdc-dialog",
"//src/cdk/a11y",
"//src/cdk/bidi",
"//src/cdk/dialog",
"//src/cdk/keycodes",
"//src/cdk/overlay",
"//src/cdk/platform",
Expand Down
24 changes: 13 additions & 11 deletions src/material-experimental/mdc-dialog/dialog-container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@
*/

import {FocusMonitor, FocusTrapFactory, InteractivityChecker} from '@angular/cdk/a11y';
import {OverlayRef} from '@angular/cdk/overlay';
import {DOCUMENT} from '@angular/common';
import {
ChangeDetectionStrategy,
ChangeDetectorRef,
Component,
ElementRef,
Inject,
Expand Down Expand Up @@ -38,8 +38,8 @@ import {cssClasses, numbers} from '@material/dialog';
host: {
'class': 'mat-mdc-dialog-container mdc-dialog',
'tabindex': '-1',
'aria-modal': 'true',
'[id]': '_id',
'[attr.aria-modal]': '_config.ariaModal',
'[id]': '_config.id',
'[attr.role]': '_config.role',
'[attr.aria-labelledby]': '_config.ariaLabel ? null : _ariaLabelledBy',
'[attr.aria-label]': '_config.ariaLabel',
Expand Down Expand Up @@ -67,30 +67,31 @@ export class MatDialogContainer extends _MatDialogContainerBase implements OnDes
constructor(
elementRef: ElementRef,
focusTrapFactory: FocusTrapFactory,
changeDetectorRef: ChangeDetectorRef,
@Optional() @Inject(DOCUMENT) document: any,
config: MatDialogConfig,
dialogConfig: MatDialogConfig,
checker: InteractivityChecker,
ngZone: NgZone,
overlayRef: OverlayRef,
@Optional() @Inject(ANIMATION_MODULE_TYPE) private _animationMode?: string,
focusMonitor?: FocusMonitor,
) {
super(
elementRef,
focusTrapFactory,
changeDetectorRef,
document,
config,
dialogConfig,
checker,
ngZone,
overlayRef,
focusMonitor,
);
}

override _initializeWithAttachedContent() {
protected override _contentAttached(): void {
// Delegate to the original dialog-container initialization (i.e. saving the
// previous element, setting up the focus trap and moving focus to the container).
super._initializeWithAttachedContent();
super._contentAttached();

// Note: Usually we would be able to use the MDC dialog foundation here to handle
// the dialog animation for us, but there are a few reasons why we just leverage
// their styles and not use the runtime foundation code:
Expand All @@ -103,7 +104,9 @@ export class MatDialogContainer extends _MatDialogContainerBase implements OnDes
this._startOpenAnimation();
}

ngOnDestroy() {
override ngOnDestroy() {
super.ngOnDestroy();

if (this._animationTimer !== null) {
clearTimeout(this._animationTimer);
}
Expand Down Expand Up @@ -177,7 +180,6 @@ export class MatDialogContainer extends _MatDialogContainerBase implements OnDes
*/
private _finishDialogClose = () => {
this._clearAnimationClasses();
this._restoreFocus();
this._animationStateChanged.emit({state: 'closed', totalTime: this._closeAnimationDuration});
};

Expand Down
15 changes: 1 addition & 14 deletions src/material-experimental/mdc-dialog/dialog-ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,22 +6,9 @@
* found in the LICENSE file at https://angular.io/license
*/

import {OverlayRef} from '@angular/cdk/overlay';
import {MatDialogRef as NonMdcDialogRef} from '@angular/material/dialog';
import {MatDialogContainer} from './dialog-container';

// Counter for unique dialog ids.
let uniqueId = 0;

/**
* Reference to a dialog opened via the MatDialog service.
*/
export class MatDialogRef<T, R = any> extends NonMdcDialogRef<T, R> {
constructor(
overlayRef: OverlayRef,
containerInstance: MatDialogContainer,
id: string = `mat-mdc-dialog-${uniqueId++}`,
) {
super(overlayRef, containerInstance, id);
}
}
export class MatDialogRef<T, R = any> extends NonMdcDialogRef<T, R> {}
4 changes: 4 additions & 0 deletions src/material-experimental/mdc-dialog/dialog.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1325,6 +1325,7 @@ describe('MDC-based MatDialog', () => {

tick(500);
viewContainerFixture.detectChanges();
flushMicrotasks();
expect(lastFocusOrigin!).withContext('Expected the trigger button to be blurred').toBeNull();

dispatchKeyboardEvent(document.body, 'keydown', ESCAPE);
Expand Down Expand Up @@ -1359,6 +1360,7 @@ describe('MDC-based MatDialog', () => {

tick(500);
viewContainerFixture.detectChanges();
flushMicrotasks();
expect(lastFocusOrigin!).withContext('Expected the trigger button to be blurred').toBeNull();

const backdrop = overlayContainerElement.querySelector(
Expand Down Expand Up @@ -1395,6 +1397,7 @@ describe('MDC-based MatDialog', () => {

tick(500);
viewContainerFixture.detectChanges();
flushMicrotasks();
expect(lastFocusOrigin!).withContext('Expected the trigger button to be blurred').toBeNull();

const closeButton = overlayContainerElement.querySelector(
Expand Down Expand Up @@ -1434,6 +1437,7 @@ describe('MDC-based MatDialog', () => {

tick(500);
viewContainerFixture.detectChanges();
flushMicrotasks();
expect(lastFocusOrigin!).withContext('Expected the trigger button to be blurred').toBeNull();

const closeButton = overlayContainerElement.querySelector(
Expand Down
17 changes: 15 additions & 2 deletions src/material-experimental/mdc-dialog/dialog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,18 @@

import {Overlay, OverlayContainer, ScrollStrategy} from '@angular/cdk/overlay';
import {Location} from '@angular/common';
import {Inject, Injectable, InjectionToken, Injector, Optional, SkipSelf} from '@angular/core';
import {
ANIMATION_MODULE_TYPE,
Inject,
Injectable,
InjectionToken,
Injector,
Optional,
SkipSelf,
} from '@angular/core';
import {_MatDialogBase, MatDialogConfig} from '@angular/material/dialog';
import {MatDialogContainer} from './dialog-container';
import {MatDialogRef} from './dialog-ref';
import {ANIMATION_MODULE_TYPE} from '@angular/platform-browser/animations';

/** Injection token that can be used to access the data that was passed in to a dialog. */
export const MAT_DIALOG_DATA = new InjectionToken<any>('MatMdcDialogData');
Expand Down Expand Up @@ -57,6 +64,10 @@ export class MatDialog extends _MatDialogBase<MatDialogContainer> {
@Optional() @Inject(MAT_DIALOG_DEFAULT_OPTIONS) defaultOptions: MatDialogConfig,
@Inject(MAT_DIALOG_SCROLL_STRATEGY) scrollStrategy: any,
@Optional() @SkipSelf() parentDialog: MatDialog,
/**
* @deprecated No longer used. To be removed.
* @breaking-change 15.0.0
*/
overlayContainer: OverlayContainer,
/**
* @deprecated No longer used. To be removed.
Expand All @@ -78,5 +89,7 @@ export class MatDialog extends _MatDialogBase<MatDialogContainer> {
MAT_DIALOG_DATA,
animationMode,
);

this._idPrefix = 'mat-mdc-dialog-';
}
}
3 changes: 2 additions & 1 deletion src/material-experimental/mdc-dialog/module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {DialogModule} from '@angular/cdk/dialog';
import {OverlayModule} from '@angular/cdk/overlay';
import {PortalModule} from '@angular/cdk/portal';
import {NgModule} from '@angular/core';
Expand All @@ -20,7 +21,7 @@ import {
} from './dialog-content-directives';

@NgModule({
imports: [OverlayModule, PortalModule, MatCommonModule],
imports: [DialogModule, OverlayModule, PortalModule, MatCommonModule],
exports: [
MatDialogContainer,
MatDialogClose,
Expand Down
Loading