Skip to content

Commit fef66fe

Browse files
authored
fix(material/dialog): mat-dialog-title should work under OnPush viewContainerRef (#28329)
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. This commit updates the queue to be a signal so we don't have to think about calling `markForCheck` at the right times.
1 parent 7840cd3 commit fef66fe

File tree

6 files changed

+90
-14
lines changed

6 files changed

+90
-14
lines changed

src/cdk/dialog/dialog-container.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import {
2525
import {DOCUMENT} from '@angular/common';
2626
import {
2727
ChangeDetectionStrategy,
28+
ChangeDetectorRef,
2829
Component,
2930
ComponentRef,
3031
ElementRef,
@@ -99,6 +100,8 @@ export class CdkDialogContainer<C extends DialogConfig = DialogConfig>
99100
*/
100101
_ariaLabelledByQueue: string[] = [];
101102

103+
protected readonly _changeDetectorRef = inject(ChangeDetectorRef);
104+
102105
constructor(
103106
protected _elementRef: ElementRef,
104107
protected _focusTrapFactory: FocusTrapFactory,
@@ -118,6 +121,20 @@ export class CdkDialogContainer<C extends DialogConfig = DialogConfig>
118121
}
119122
}
120123

124+
_addAriaLabelledBy(id: string) {
125+
this._ariaLabelledByQueue.push(id);
126+
this._changeDetectorRef.markForCheck();
127+
}
128+
129+
_removeAriaLabelledBy(id: string) {
130+
const index = this._ariaLabelledByQueue.indexOf(id);
131+
132+
if (index > -1) {
133+
this._ariaLabelledByQueue.splice(index, 1);
134+
this._changeDetectorRef.markForCheck();
135+
}
136+
}
137+
121138
protected _contentAttached() {
122139
this._initializeFocusTrap();
123140
this._handleBackdropClicks();

src/material/bottom-sheet/bottom-sheet-container.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import {OverlayRef} from '@angular/cdk/overlay';
1414
import {DOCUMENT} from '@angular/common';
1515
import {
1616
ChangeDetectionStrategy,
17-
ChangeDetectorRef,
1817
Component,
1918
ElementRef,
2019
EventEmitter,
@@ -77,7 +76,6 @@ export class MatBottomSheetContainer extends CdkDialogContainer implements OnDes
7776
ngZone: NgZone,
7877
overlayRef: OverlayRef,
7978
breakpointObserver: BreakpointObserver,
80-
private _changeDetectorRef: ChangeDetectorRef,
8179
focusMonitor?: FocusMonitor,
8280
) {
8381
super(

src/material/dialog/dialog-content-directives.ts

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -120,23 +120,19 @@ export class MatDialogTitle implements OnInit, OnDestroy {
120120
Promise.resolve().then(() => {
121121
// Note: we null check the queue, because there are some internal
122122
// tests that are mocking out `MatDialogRef` incorrectly.
123-
this._dialogRef._containerInstance?._ariaLabelledByQueue?.push(this.id);
123+
this._dialogRef._containerInstance?._addAriaLabelledBy?.(this.id);
124124
});
125125
}
126126
}
127127

128128
ngOnDestroy() {
129-
// Note: we null check the queue, because there are some internal
129+
// Note: we null check because there are some internal
130130
// tests that are mocking out `MatDialogRef` incorrectly.
131-
const queue = this._dialogRef?._containerInstance?._ariaLabelledByQueue;
131+
const instance = this._dialogRef?._containerInstance;
132132

133-
if (queue) {
133+
if (instance) {
134134
Promise.resolve().then(() => {
135-
const index = queue.indexOf(this.id);
136-
137-
if (index > -1) {
138-
queue.splice(index, 1);
139-
}
135+
instance._removeAriaLabelledBy?.(this.id);
140136
});
141137
}
142138
}

src/material/dialog/dialog.spec.ts

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import {
3030
ViewContainerRef,
3131
ViewEncapsulation,
3232
forwardRef,
33+
signal,
3334
} from '@angular/core';
3435
import {
3536
ComponentFixture,
@@ -1623,6 +1624,64 @@ describe('MDC-based MatDialog', () => {
16231624
runContentElementTests();
16241625
});
16251626

1627+
it('should set the aria-labelledby attribute to the id of the title under OnPush host', fakeAsync(() => {
1628+
@Component({
1629+
standalone: true,
1630+
imports: [MatDialogTitle],
1631+
template: `@if (showTitle()) { <h1 mat-dialog-title>This is the first title</h1> }`,
1632+
})
1633+
class DialogCmp {
1634+
showTitle = signal(true);
1635+
}
1636+
1637+
@Component({
1638+
template: '',
1639+
selector: 'child',
1640+
standalone: true,
1641+
})
1642+
class Child {
1643+
dialogRef?: MatDialogRef<DialogCmp>;
1644+
1645+
constructor(
1646+
readonly viewContainerRef: ViewContainerRef,
1647+
readonly dialog: MatDialog,
1648+
) {}
1649+
1650+
open() {
1651+
this.dialogRef = this.dialog.open(DialogCmp, {viewContainerRef: this.viewContainerRef});
1652+
}
1653+
}
1654+
1655+
@Component({
1656+
standalone: true,
1657+
imports: [Child],
1658+
template: `<child></child>`,
1659+
changeDetection: ChangeDetectionStrategy.OnPush,
1660+
})
1661+
class OnPushHost {
1662+
@ViewChild(Child, {static: true}) child: Child;
1663+
}
1664+
1665+
const hostFixture = TestBed.createComponent(OnPushHost);
1666+
hostFixture.componentInstance.child.open();
1667+
hostFixture.autoDetectChanges();
1668+
flush();
1669+
1670+
const overlayContainer = TestBed.inject(OverlayContainer);
1671+
const title = overlayContainer.getContainerElement().querySelector('[mat-dialog-title]')!;
1672+
const container = overlayContainerElement.querySelector('mat-dialog-container')!;
1673+
1674+
expect(title.id).withContext('Expected title element to have an id.').toBeTruthy();
1675+
expect(container.getAttribute('aria-labelledby'))
1676+
.withContext('Expected the aria-labelledby to match the title id.')
1677+
.toBe(title.id);
1678+
1679+
hostFixture.componentInstance.child.dialogRef?.componentInstance.showTitle.set(false);
1680+
hostFixture.detectChanges();
1681+
flush();
1682+
expect(container.getAttribute('aria-labelledby')).toBe(null);
1683+
}));
1684+
16261685
function runContentElementTests() {
16271686
it('should close the dialog when clicking on the close button', fakeAsync(() => {
16281687
expect(overlayContainerElement.querySelectorAll('.mat-mdc-dialog-container').length).toBe(

tools/public_api_guard/cdk/dialog.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
import { BasePortalOutlet } from '@angular/cdk/portal';
88
import { CdkPortalOutlet } from '@angular/cdk/portal';
9+
import { ChangeDetectorRef } from '@angular/core';
910
import { ComponentFactoryResolver } from '@angular/core';
1011
import { ComponentPortal } from '@angular/cdk/portal';
1112
import { ComponentRef } from '@angular/core';
@@ -45,12 +46,16 @@ export type AutoFocusTarget = 'dialog' | 'first-tabbable' | 'first-heading';
4546
// @public
4647
export class CdkDialogContainer<C extends DialogConfig = DialogConfig> extends BasePortalOutlet implements OnDestroy {
4748
constructor(_elementRef: ElementRef, _focusTrapFactory: FocusTrapFactory, _document: any, _config: C, _interactivityChecker: InteractivityChecker, _ngZone: NgZone, _overlayRef: OverlayRef, _focusMonitor?: FocusMonitor | undefined);
49+
// (undocumented)
50+
_addAriaLabelledBy(id: string): void;
4851
_ariaLabelledByQueue: string[];
4952
attachComponentPortal<T>(portal: ComponentPortal<T>): ComponentRef<T>;
5053
// @deprecated
5154
attachDomPortal: (portal: DomPortal) => void;
5255
attachTemplatePortal<T>(portal: TemplatePortal<T>): EmbeddedViewRef<T>;
5356
protected _captureInitialFocus(): void;
57+
// (undocumented)
58+
protected readonly _changeDetectorRef: ChangeDetectorRef;
5459
_closeInteractionType: FocusOrigin | null;
5560
// (undocumented)
5661
readonly _config: C;
@@ -68,6 +73,8 @@ export class CdkDialogContainer<C extends DialogConfig = DialogConfig> extends B
6873
protected _ngZone: NgZone;
6974
_portalOutlet: CdkPortalOutlet;
7075
_recaptureFocus(): void;
76+
// (undocumented)
77+
_removeAriaLabelledBy(id: string): void;
7178
protected _trapFocus(): void;
7279
// (undocumented)
7380
static ɵcmp: i0.ɵɵComponentDeclaration<CdkDialogContainer<any>, "cdk-dialog-container", never, {}, {}, never, never, true, never>;

tools/public_api_guard/material/bottom-sheet.md

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import { AnimationEvent as AnimationEvent_2 } from '@angular/animations';
88
import { AnimationTriggerMetadata } from '@angular/animations';
99
import { BreakpointObserver } from '@angular/cdk/layout';
1010
import { CdkDialogContainer } from '@angular/cdk/dialog';
11-
import { ChangeDetectorRef } from '@angular/core';
1211
import { ComponentRef } from '@angular/core';
1312
import { ComponentType } from '@angular/cdk/portal';
1413
import { DialogConfig } from '@angular/cdk/dialog';
@@ -83,7 +82,7 @@ export class MatBottomSheetConfig<D = any> {
8382

8483
// @public
8584
export class MatBottomSheetContainer extends CdkDialogContainer implements OnDestroy {
86-
constructor(elementRef: ElementRef, focusTrapFactory: FocusTrapFactory, document: any, config: DialogConfig, checker: InteractivityChecker, ngZone: NgZone, overlayRef: OverlayRef, breakpointObserver: BreakpointObserver, _changeDetectorRef: ChangeDetectorRef, focusMonitor?: FocusMonitor);
85+
constructor(elementRef: ElementRef, focusTrapFactory: FocusTrapFactory, document: any, config: DialogConfig, checker: InteractivityChecker, ngZone: NgZone, overlayRef: OverlayRef, breakpointObserver: BreakpointObserver, focusMonitor?: FocusMonitor);
8786
_animationState: 'void' | 'visible' | 'hidden';
8887
_animationStateChanged: EventEmitter<AnimationEvent_2>;
8988
// (undocumented)
@@ -99,7 +98,7 @@ export class MatBottomSheetContainer extends CdkDialogContainer implements OnDes
9998
// (undocumented)
10099
static ɵcmp: i0.ɵɵComponentDeclaration<MatBottomSheetContainer, "mat-bottom-sheet-container", never, {}, {}, never, never, true, never>;
101100
// (undocumented)
102-
static ɵfac: i0.ɵɵFactoryDeclaration<MatBottomSheetContainer, [null, null, { optional: true; }, null, null, null, null, null, null, null]>;
101+
static ɵfac: i0.ɵɵFactoryDeclaration<MatBottomSheetContainer, [null, null, { optional: true; }, null, null, null, null, null, null]>;
103102
}
104103

105104
// @public (undocumented)

0 commit comments

Comments
 (0)