Skip to content

Commit

Permalink
fix(material/dialog): mat-dialog-title should work under OnPush `…
Browse files Browse the repository at this point in the history
…viewContainerRef`

The `mat-dialog-title` directive updates state in a microtask and should
call `ChangeDetectorRef.markForCheck`. Failing to do this will cause the
component tree to not be checked if it lives under an `OnPush` component
that has not otherwise been marked for check.
  • Loading branch information
atscott committed Dec 26, 2023
1 parent 5bbad71 commit cd0f028
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 0 deletions.
4 changes: 4 additions & 0 deletions src/material/dialog/dialog-content-directives.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@
*/

import {
ChangeDetectorRef,
Directive,
ElementRef,
inject,
Input,
OnChanges,
OnDestroy,
Expand Down Expand Up @@ -102,6 +104,7 @@ export class MatDialogClose implements OnInit, OnChanges {
})
export class MatDialogTitle implements OnInit, OnDestroy {
@Input() id: string = `mat-mdc-dialog-title-${dialogElementUid++}`;
private readonly changeDetectorRef = inject(ChangeDetectorRef);

constructor(
// The dialog title directive is always used in combination with a `MatDialogRef`.
Expand All @@ -121,6 +124,7 @@ export class MatDialogTitle implements OnInit, OnDestroy {
// Note: we null check the queue, because there are some internal
// tests that are mocking out `MatDialogRef` incorrectly.
this._dialogRef._containerInstance?._ariaLabelledByQueue?.push(this.id);
this.changeDetectorRef.markForCheck();
});
}
}
Expand Down
50 changes: 50 additions & 0 deletions src/material/dialog/dialog.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ import {
MatDialogRef,
MAT_DIALOG_DATA,
MAT_DIALOG_DEFAULT_OPTIONS,
MatDialogTitle,
} from './index';
import {CLOSE_ANIMATION_DURATION, OPEN_ANIMATION_DURATION} from './dialog-container';

Expand Down Expand Up @@ -1617,6 +1618,55 @@ describe('MDC-based MatDialog', () => {
runContentElementTests();
});

it('should set the aria-labelledby attribute to the id of the title under OnPush host', fakeAsync(() => {
@Component({
standalone: true,
imports: [MatDialogTitle],
template: `<h1 mat-dialog-title>This is the first title</h1>`,
})
class DialogCmp {}

@Component({
template: '',
selector: 'child',
standalone: true,
})
class Child {
constructor(
readonly viewContainerRef: ViewContainerRef,
readonly dialog: MatDialog,
) {}

open() {
this.dialog.open(DialogCmp, {viewContainerRef: this.viewContainerRef});
}
}

@Component({
standalone: true,
imports: [Child],
template: `<child></child>`,
changeDetection: ChangeDetectionStrategy.OnPush,
})
class OnPushHost {
@ViewChild(Child, {static: true}) child: Child;
}

const hostFixture = TestBed.createComponent(OnPushHost);
hostFixture.componentInstance.child.open();
hostFixture.autoDetectChanges();
flush();

const overlayContainer = TestBed.inject(OverlayContainer);
const title = overlayContainer.getContainerElement().querySelector('[mat-dialog-title]')!;
const container = overlayContainerElement.querySelector('mat-dialog-container')!;

expect(title.id).withContext('Expected title element to have an id.').toBeTruthy();
expect(container.getAttribute('aria-labelledby'))
.withContext('Expected the aria-labelledby to match the title id.')
.toBe(title.id);
}));

function runContentElementTests() {
it('should close the dialog when clicking on the close button', fakeAsync(() => {
expect(overlayContainerElement.querySelectorAll('.mat-mdc-dialog-container').length).toBe(
Expand Down

0 comments on commit cd0f028

Please sign in to comment.