Skip to content

fix(material/dialog): content directives picking up wrong ref for stacked mixed type dialogs #21579

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(material/dialog): content directives picking up wrong ref for stacked mixed type dialogs #21579

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
38 changes: 20 additions & 18 deletions src/material-experimental/mdc-dialog/dialog-content-directives.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@
import {
Directive,
ElementRef,
Inject,
Input,
OnChanges,
OnInit,
Optional,
SimpleChanges,
} from '@angular/core';
import {_closeDialogVia} from '@angular/material/dialog';
Expand Down Expand Up @@ -47,23 +47,23 @@ export class MatDialogClose implements OnInit, OnChanges {

@Input('matDialogClose') _matDialogClose: any;

/** Reference to the dialog that the close button is placed inside of. */
dialogRef: MatDialogRef<any>;

constructor(
// The dialog title directive is always used in combination with a `MatDialogRef`.
// tslint:disable-next-line: lightweight-tokens
@Optional() public dialogRef: MatDialogRef<any>,
/**
* @deprecated `_dialogRef` parameter to be removed.
* @breaking-change 12.0.0
*/
@Inject(ElementRef) _dialogRef: any,
private _elementRef: ElementRef<HTMLElement>,
private _dialog: MatDialog,
) {}

ngOnInit() {
if (!this.dialogRef) {
// When this directive is included in a dialog via TemplateRef (rather than being
// in a Component), the DialogRef isn't available via injection because embedded
// views cannot be given a custom injector. Instead, we look up the DialogRef by
// ID. This must occur in `onInit`, as the ID binding for the dialog container won't
// be resolved at constructor time.
this.dialogRef = getClosestDialog(this._elementRef, this._dialog.openDialogs)!;
}
// Always resolve the closest dialog using the DOM, rather than DI, because DI won't work for
// `TemplateRef`-based dialogs and it may give us wrong results for stacked ones (see #21554).
this.dialogRef = getClosestDialog(this._elementRef, this._dialog.openDialogs)!;
}

ngOnChanges(changes: SimpleChanges) {
Expand Down Expand Up @@ -101,18 +101,20 @@ export class MatDialogClose implements OnInit, OnChanges {
export class MatDialogTitle implements OnInit {
@Input() id: string = `mat-mdc-dialog-title-${dialogElementUid++}`;

private _dialogRef: MatDialogRef<unknown>;

constructor(
// The dialog title directive is always used in combination with a `MatDialogRef`.
// tslint:disable-next-line: lightweight-tokens
@Optional() private _dialogRef: MatDialogRef<any>,
/**
* @deprecated `_dialogRef` parameter to be removed.
* @breaking-change 12.0.0
*/
@Inject(ElementRef) _dialogRef: any,
private _elementRef: ElementRef<HTMLElement>,
private _dialog: MatDialog,
) {}

ngOnInit() {
if (!this._dialogRef) {
this._dialogRef = getClosestDialog(this._elementRef, this._dialog.openDialogs)!;
}
this._dialogRef = getClosestDialog(this._elementRef, this._dialog.openDialogs)!;

if (this._dialogRef) {
Promise.resolve().then(() => {
Expand Down
39 changes: 39 additions & 0 deletions src/material-experimental/mdc-dialog/dialog.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ describe('MDC-based MatDialog', () => {
DialogWithoutFocusableElements,
DirectiveWithViewContainer,
ComponentWithContentElementTemplateRef,
MixedTypeStackedDialog,
],
providers: [
{provide: Location, useClass: SpyLocation},
Expand Down Expand Up @@ -764,6 +765,25 @@ describe('MDC-based MatDialog', () => {
},
));

it('should close the correct dialog when stacked and using a template from another dialog', fakeAsync(() => {
const dialogRef = dialog.open(MixedTypeStackedDialog);
viewContainerFixture.detectChanges();

dialogRef.componentInstance.open();
viewContainerFixture.detectChanges();

expect(overlayContainerElement.textContent).toContain('Bottom');
expect(overlayContainerElement.textContent).toContain('Top');

(overlayContainerElement.querySelector('.close') as HTMLButtonElement).click();
flushMicrotasks();
viewContainerFixture.detectChanges();
tick(500);

expect(overlayContainerElement.textContent).toContain('Bottom');
expect(overlayContainerElement.textContent).not.toContain('Top');
}));

describe('passing in data', () => {
it('should be able to pass in data', () => {
let config = {data: {stringParam: 'hello', dateParam: new Date()}};
Expand Down Expand Up @@ -2127,3 +2147,22 @@ class DialogWithoutFocusableElements {}
encapsulation: ViewEncapsulation.ShadowDom,
})
class ShadowDomComponent {}

@Component({
template: `
Bottom
<ng-template>
Top
<button class="close" mat-dialog-close>Close</button>
</ng-template>
`,
})
class MixedTypeStackedDialog {
@ViewChild(TemplateRef) template: TemplateRef<any>;

constructor(private _dialog: MatDialog) {}

open() {
this._dialog.open(this.template);
}
}
43 changes: 22 additions & 21 deletions src/material/dialog/dialog-content-directives.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ import {
Input,
OnChanges,
OnInit,
Optional,
SimpleChanges,
ElementRef,
Inject,
} from '@angular/core';
import {MatDialog} from './dialog';
import {_closeDialogVia, MatDialogRef} from './dialog-ref';
Expand Down Expand Up @@ -45,28 +45,27 @@ export class MatDialogClose implements OnInit, OnChanges {

@Input('matDialogClose') _matDialogClose: any;

/**
* Reference to the containing dialog.
* @deprecated `dialogRef` property to become private.
* @breaking-change 13.0.0
*/
dialogRef: MatDialogRef<any>;

constructor(
/**
* Reference to the containing dialog.
* @deprecated `dialogRef` property to become private.
* @breaking-change 13.0.0
* @deprecated `_dialogRef` parameter to be removed.
* @breaking-change 12.0.0
*/
// The dialog title directive is always used in combination with a `MatDialogRef`.
// tslint:disable-next-line: lightweight-tokens
@Optional() public dialogRef: MatDialogRef<any>,
@Inject(ElementRef) _dialogRef: any,
private _elementRef: ElementRef<HTMLElement>,
private _dialog: MatDialog,
) {}

ngOnInit() {
if (!this.dialogRef) {
// When this directive is included in a dialog via TemplateRef (rather than being
// in a Component), the DialogRef isn't available via injection because embedded
// views cannot be given a custom injector. Instead, we look up the DialogRef by
// ID. This must occur in `onInit`, as the ID binding for the dialog container won't
// be resolved at constructor time.
this.dialogRef = getClosestDialog(this._elementRef, this._dialog.openDialogs)!;
}
// Always resolve the closest dialog using the DOM, rather than DI, because DI won't work for
// `TemplateRef`-based dialogs and it may give us wrong results for stacked ones (see #21554).
this.dialogRef = getClosestDialog(this._elementRef, this._dialog.openDialogs)!;
}

ngOnChanges(changes: SimpleChanges) {
Expand Down Expand Up @@ -105,18 +104,20 @@ export class MatDialogTitle implements OnInit {
/** Unique id for the dialog title. If none is supplied, it will be auto-generated. */
@Input() id: string = `mat-dialog-title-${dialogElementUid++}`;

private _dialogRef: MatDialogRef<unknown>;

constructor(
// The dialog title directive is always used in combination with a `MatDialogRef`.
// tslint:disable-next-line: lightweight-tokens
@Optional() private _dialogRef: MatDialogRef<any>,
/**
* @deprecated `_dialogRef` parameter to be removed.
* @breaking-change 12.0.0
*/
@Inject(ElementRef) _dialogRef: any,
private _elementRef: ElementRef<HTMLElement>,
private _dialog: MatDialog,
) {}

ngOnInit() {
if (!this._dialogRef) {
this._dialogRef = getClosestDialog(this._elementRef, this._dialog.openDialogs)!;
}
this._dialogRef = getClosestDialog(this._elementRef, this._dialog.openDialogs)!;

if (this._dialogRef) {
Promise.resolve().then(() => {
Expand Down
39 changes: 39 additions & 0 deletions src/material/dialog/dialog.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ describe('MatDialog', () => {
DialogWithoutFocusableElements,
DirectiveWithViewContainer,
ComponentWithContentElementTemplateRef,
MixedTypeStackedDialog,
],
providers: [
{provide: Location, useClass: SpyLocation},
Expand Down Expand Up @@ -824,6 +825,25 @@ describe('MatDialog', () => {
},
));

it('should close the correct dialog when stacked and using a template from another dialog', fakeAsync(() => {
const dialogRef = dialog.open(MixedTypeStackedDialog);
viewContainerFixture.detectChanges();

dialogRef.componentInstance.open();
viewContainerFixture.detectChanges();

expect(overlayContainerElement.textContent).toContain('Bottom');
expect(overlayContainerElement.textContent).toContain('Top');

(overlayContainerElement.querySelector('.close') as HTMLButtonElement).click();
flushMicrotasks();
viewContainerFixture.detectChanges();
tick(500);

expect(overlayContainerElement.textContent).toContain('Bottom');
expect(overlayContainerElement.textContent).not.toContain('Top');
}));

describe('passing in data', () => {
it('should be able to pass in data', () => {
const config = {
Expand Down Expand Up @@ -2174,3 +2194,22 @@ class DialogWithoutFocusableElements {}
encapsulation: ViewEncapsulation.ShadowDom,
})
class ShadowDomComponent {}

@Component({
template: `
Bottom
<ng-template>
Top
<button class="close" mat-dialog-close>Close</button>
</ng-template>
`,
})
class MixedTypeStackedDialog {
@ViewChild(TemplateRef) template: TemplateRef<any>;

constructor(private _dialog: MatDialog) {}

open() {
this._dialog.open(this.template);
}
}
9 changes: 5 additions & 4 deletions tools/public_api_guard/material/dialog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ export abstract class _MatDialogBase<C extends _MatDialogContainerBase> implemen
// @public
export class MatDialogClose implements OnInit, OnChanges {
constructor(
dialogRef: MatDialogRef<any>, _elementRef: ElementRef<HTMLElement>, _dialog: MatDialog);
_dialogRef: any, _elementRef: ElementRef<HTMLElement>, _dialog: MatDialog);
ariaLabel: string;
// @deprecated
dialogRef: MatDialogRef<any>;
Expand All @@ -149,7 +149,7 @@ export class MatDialogClose implements OnInit, OnChanges {
// (undocumented)
static ɵdir: i0.ɵɵDirectiveDeclaration<MatDialogClose, "[mat-dialog-close], [matDialogClose]", ["matDialogClose"], { "ariaLabel": "aria-label"; "type": "type"; "dialogResult": "mat-dialog-close"; "_matDialogClose": "matDialogClose"; }, {}, never>;
// (undocumented)
static ɵfac: i0.ɵɵFactoryDeclaration<MatDialogClose, [{ optional: true; }, null, null]>;
static ɵfac: i0.ɵɵFactoryDeclaration<MatDialogClose, never>;
}

// @public
Expand Down Expand Up @@ -277,14 +277,15 @@ export const enum MatDialogState {

// @public
export class MatDialogTitle implements OnInit {
constructor(_dialogRef: MatDialogRef<any>, _elementRef: ElementRef<HTMLElement>, _dialog: MatDialog);
constructor(
_dialogRef: any, _elementRef: ElementRef<HTMLElement>, _dialog: MatDialog);
id: string;
// (undocumented)
ngOnInit(): void;
// (undocumented)
static ɵdir: i0.ɵɵDirectiveDeclaration<MatDialogTitle, "[mat-dialog-title], [matDialogTitle]", ["matDialogTitle"], { "id": "id"; }, {}, never>;
// (undocumented)
static ɵfac: i0.ɵɵFactoryDeclaration<MatDialogTitle, [{ optional: true; }, null, null]>;
static ɵfac: i0.ɵɵFactoryDeclaration<MatDialogTitle, never>;
}

// @public
Expand Down