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. This commit updates the
queue to be a signal so we don't have to think about calling
`markForCheck` at the right times.
  • Loading branch information
atscott committed Dec 26, 2023
1 parent a8b8e62 commit 1c3f4bd
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 8 deletions.
18 changes: 18 additions & 0 deletions src/cdk/dialog/dialog-container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
import {DOCUMENT} from '@angular/common';
import {
ChangeDetectionStrategy,
ChangeDetectorRef,
Component,
ComponentRef,
ElementRef,
Expand All @@ -35,6 +36,7 @@ import {
Optional,
ViewChild,
ViewEncapsulation,
inject,
} from '@angular/core';
import {DialogConfig} from './dialog-config';

Expand Down Expand Up @@ -97,6 +99,8 @@ export class CdkDialogContainer<C extends DialogConfig = DialogConfig>
*/
_ariaLabelledByQueue: string[] = [];

private readonly _changeDetectorRef = inject(ChangeDetectorRef);

constructor(
protected _elementRef: ElementRef,
protected _focusTrapFactory: FocusTrapFactory,
Expand All @@ -116,6 +120,20 @@ export class CdkDialogContainer<C extends DialogConfig = DialogConfig>
}
}

_addAriaLabelledBy(id: string) {
this._ariaLabelledByQueue.push(id);
this._changeDetectorRef.markForCheck();
}

_removeAriaLabelledBy(id: string) {
const index = this._ariaLabelledByQueue.indexOf(id);

if (index > -1) {
this._ariaLabelledByQueue.splice(index, 1);
this._changeDetectorRef.markForCheck();
}
}

protected _contentAttached() {
this._initializeFocusTrap();
this._handleBackdropClicks();
Expand Down
12 changes: 4 additions & 8 deletions src/material/dialog/dialog-content-directives.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,23 +120,19 @@ export class MatDialogTitle implements OnInit, OnDestroy {
Promise.resolve().then(() => {
// 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._dialogRef._containerInstance?._addAriaLabelledBy(this.id);
});
}
}

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

if (queue) {
if (instance) {
Promise.resolve().then(() => {
const index = queue.indexOf(this.id);

if (index > -1) {
queue.splice(index, 1);
}
instance._removeAriaLabelledBy(this.id);
});
}
}
Expand Down
59 changes: 59 additions & 0 deletions src/material/dialog/dialog.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
ViewContainerRef,
ViewEncapsulation,
forwardRef,
signal,
} from '@angular/core';
import {
ComponentFixture,
Expand Down Expand Up @@ -1623,6 +1624,64 @@ 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: `@if (showTitle()) { <h1 mat-dialog-title>This is the first title</h1> }`,
})
class DialogCmp {
showTitle = signal(true);
}

@Component({
template: '',
selector: 'child',
standalone: true,
})
class Child {
dialogRef?: MatDialogRef<DialogCmp>;

constructor(
readonly viewContainerRef: ViewContainerRef,
readonly dialog: MatDialog,
) {}

open() {
this.dialogRef = 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);

hostFixture.componentInstance.child.dialogRef?.componentInstance.showTitle.set(false);
hostFixture.detectChanges();
flush();
expect(container.getAttribute('aria-labelledby')).toBe(null);
}));

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
4 changes: 4 additions & 0 deletions tools/public_api_guard/cdk/dialog.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ export type AutoFocusTarget = 'dialog' | 'first-tabbable' | 'first-heading';
// @public
export class CdkDialogContainer<C extends DialogConfig = DialogConfig> extends BasePortalOutlet implements OnDestroy {
constructor(_elementRef: ElementRef, _focusTrapFactory: FocusTrapFactory, _document: any, _config: C, _interactivityChecker: InteractivityChecker, _ngZone: NgZone, _overlayRef: OverlayRef, _focusMonitor?: FocusMonitor | undefined);
// (undocumented)
_addAriaLabelledBy(id: string): void;
_ariaLabelledByQueue: string[];
attachComponentPortal<T>(portal: ComponentPortal<T>): ComponentRef<T>;
// @deprecated
Expand All @@ -68,6 +70,8 @@ export class CdkDialogContainer<C extends DialogConfig = DialogConfig> extends B
protected _ngZone: NgZone;
_portalOutlet: CdkPortalOutlet;
_recaptureFocus(): void;
// (undocumented)
_removeAriaLabelledBy(id: string): void;
protected _trapFocus(): void;
// (undocumented)
static ɵcmp: i0.ɵɵComponentDeclaration<CdkDialogContainer<any>, "cdk-dialog-container", never, {}, {}, never, never, true, never>;
Expand Down

0 comments on commit 1c3f4bd

Please sign in to comment.