Skip to content

Commit

Permalink
delay 1 more macrotask
Browse files Browse the repository at this point in the history
  • Loading branch information
@mmalerba
mmalerba committed May 1, 2024
1 parent a4e15fa commit a5dd5ed
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 57 deletions.
28 changes: 16 additions & 12 deletions src/cdk/dialog/dialog-container.ts
View file
Expand Up @@ -270,21 +270,25 @@ export class CdkDialogContainer<C extends DialogConfig = DialogConfig>
break;
case true:
case 'first-tabbable':
let timeoutRef: {};
// TODO(mmalerba): Once hybrid mode is enabled in g3, we can change this to just
// `afterNextRender`.
// `afterNextRender(() => setTimeout(() =>`.
new Promise<void>(r => {
afterNextRender(r, {injector: this._injector});
timeoutRef = setTimeout(r);
}).then(() => {
clearTimeout(timeoutRef as number);
const focusedSuccessfully = this._focusTrap?.focusInitialElement();
// If we weren't able to find a focusable element in the dialog, then focus the dialog
// container instead.
if (!focusedSuccessfully) {
this._focusDialogContainer();
}
});
setTimeout(r);
}).then(() =>
this._ngZone.runOutsideAngular(() =>
// Wait for one more setTimeout in case there are subsequent microtask change detection
// cycles (e.g. from ngModel).
setTimeout(() => {
const focusedSuccessfully = this._focusTrap?.focusInitialElement();
// If we weren't able to find a focusable element in the dialog, then focus the
// dialog container instead.
if (!focusedSuccessfully) {
this._focusDialogContainer();
}
}),
),
);
break;
case 'first-heading':
this._focusByCssSelector('h1, h2, h3, h4, h5, h6, [role="heading"]');
Expand Down
62 changes: 32 additions & 30 deletions src/cdk/dialog/dialog.spec.ts
View file
@@ -1,37 +1,37 @@
import {Directionality} from '@angular/cdk/bidi';
import {A, ESCAPE} from '@angular/cdk/keycodes';
import {Overlay, OverlayContainer, ScrollDispatcher} from '@angular/cdk/overlay';
import {_supportsShadowDom} from '@angular/cdk/platform';
import {
ComponentFixture,
fakeAsync,
flushMicrotasks,
TestBed,
tick,
flush,
} from '@angular/core/testing';
createKeyboardEvent,
dispatchEvent,
dispatchKeyboardEvent,
} from '@angular/cdk/testing/private';
import {Location} from '@angular/common';
import {SpyLocation} from '@angular/common/testing';
import {
ChangeDetectionStrategy,
Component,
ComponentRef,
Directive,
inject,
Inject,
InjectionToken,
Injector,
TemplateRef,
ViewChild,
ViewContainerRef,
ViewEncapsulation,
inject,
} from '@angular/core';
import {By} from '@angular/platform-browser';
import {Location} from '@angular/common';
import {SpyLocation} from '@angular/common/testing';
import {Directionality} from '@angular/cdk/bidi';
import {Overlay, OverlayContainer, ScrollDispatcher} from '@angular/cdk/overlay';
import {A, ESCAPE} from '@angular/cdk/keycodes';
import {_supportsShadowDom} from '@angular/cdk/platform';
import {
dispatchKeyboardEvent,
createKeyboardEvent,
dispatchEvent,
} from '@angular/cdk/testing/private';
ComponentFixture,
TestBed,
fakeAsync,
flush,
flushMicrotasks,
tick,
} from '@angular/core/testing';
import {By} from '@angular/platform-browser';
import {Subject} from 'rxjs';
import {DIALOG_DATA, Dialog, DialogModule, DialogRef} from './index';

Expand Down Expand Up @@ -282,17 +282,20 @@ describe('Dialog', () => {
const spy = jasmine.createSpy('backdropClick spy');
dialogRef.backdropClick.subscribe(spy);
viewContainerFixture.detectChanges();
flush();

const backdrop = overlayContainerElement.querySelector('.cdk-overlay-backdrop') as HTMLElement;

backdrop.click();
viewContainerFixture.detectChanges();
flush();
expect(spy).toHaveBeenCalledTimes(1);

// Additional clicks after the dialog has closed should not be emitted
dialogRef.disableClose = false;
backdrop.click();
viewContainerFixture.detectChanges();
flush();
expect(spy).toHaveBeenCalledTimes(1);
}));

Expand All @@ -303,6 +306,7 @@ describe('Dialog', () => {
dialogRef.keydownEvents.subscribe(spy);

viewContainerFixture.detectChanges();
flush();

let backdrop = overlayContainerElement.querySelector('.cdk-overlay-backdrop') as HTMLElement;
let container = overlayContainerElement.querySelector('cdk-dialog-container') as HTMLElement;
Expand Down Expand Up @@ -757,9 +761,11 @@ describe('Dialog', () => {
fakeAsync(() => {
const templateInjectFixture = TestBed.createComponent(TemplateInjectorParentComponent);
templateInjectFixture.detectChanges();
flush();

dialog.open(templateInjectFixture.componentInstance.templateRef);
templateInjectFixture.detectChanges();
flush();

expect(templateInjectFixture.componentInstance.innerComponentValue).toBe(
'hello from parent component',
Expand Down Expand Up @@ -840,7 +846,7 @@ describe('Dialog', () => {
});

viewContainerFixture.detectChanges();
flushMicrotasks();
flush();

expect(document.activeElement!.tagName)
.withContext('Expected first tabbable element (input) in the dialog to be focused.')
Expand Down Expand Up @@ -910,17 +916,15 @@ describe('Dialog', () => {

let dialogRef = dialog.open(PizzaMsg, {viewContainerRef: testViewContainerRef});

flushMicrotasks();
viewContainerFixture.detectChanges();
flushMicrotasks();
flush();

expect(document.activeElement!.id).not.toBe(
'dialog-trigger',
'Expected the focus to change when dialog was opened.',
);

dialogRef.close();
flushMicrotasks();
viewContainerFixture.detectChanges();
flush();

Expand All @@ -939,18 +943,18 @@ describe('Dialog', () => {
viewContainerFixture.destroy();
const fixture = TestBed.createComponent(ShadowDomComponent);
fixture.detectChanges();
flush();
const button = fixture.debugElement.query(By.css('button'))!.nativeElement;

button.focus();

const dialogRef = dialog.open(PizzaMsg);
flushMicrotasks();
fixture.detectChanges();
flushMicrotasks();
flush();

const spy = spyOn(button, 'focus').and.callThrough();
dialogRef.close();
flushMicrotasks();
flush();
fixture.detectChanges();
tick(500);

Expand Down Expand Up @@ -994,7 +998,7 @@ describe('Dialog', () => {
dialog.open(DialogWithoutFocusableElements);

viewContainerFixture.detectChanges();
flushMicrotasks();
flush();

expect(document.activeElement!.tagName.toLowerCase())
.withContext('Expected dialog container to be focused.')
Expand All @@ -1013,14 +1017,12 @@ describe('Dialog', () => {
restoreFocus: false,
});

flushMicrotasks();
viewContainerFixture.detectChanges();
flushMicrotasks();
flush();

expect(document.activeElement!.id).not.toBe('dialog-trigger');

dialogRef.close();
flushMicrotasks();
viewContainerFixture.detectChanges();
flush();

Expand Down
6 changes: 3 additions & 3 deletions src/material/bottom-sheet/bottom-sheet.spec.ts
View file
Expand Up @@ -4,9 +4,9 @@ import {OverlayContainer, ScrollStrategy} from '@angular/cdk/overlay';
import {_supportsShadowDom} from '@angular/cdk/platform';
import {ViewportRuler} from '@angular/cdk/scrolling';
import {
dispatchKeyboardEvent,
createKeyboardEvent,
dispatchEvent,
dispatchKeyboardEvent,
} from '@angular/cdk/testing/private';
import {Location} from '@angular/common';
import {SpyLocation} from '@angular/common/testing';
Expand All @@ -23,11 +23,11 @@ import {
} from '@angular/core';
import {
ComponentFixture,
TestBed,
fakeAsync,
flush,
flushMicrotasks,
inject,
TestBed,
tick,
} from '@angular/core/testing';
import {By} from '@angular/platform-browser';
Expand Down Expand Up @@ -640,7 +640,7 @@ describe('MatBottomSheet', () => {
});

viewContainerFixture.detectChanges();
flushMicrotasks();
flush();

expect(document.activeElement!.tagName)
.withContext('Expected first tabbable element (input) in the dialog to be focused.')
Expand Down
17 changes: 5 additions & 12 deletions src/material/dialog/dialog.spec.ts
View file
Expand Up @@ -1276,9 +1276,8 @@ describe('MDC-based MatDialog', () => {

let dialogRef = dialog.open(PizzaMsg, {viewContainerRef: testViewContainerRef});

flushMicrotasks();
viewContainerFixture.detectChanges();
flushMicrotasks();
flush();

expect(document.activeElement!.id).not.toBe(
'dialog-trigger',
Expand All @@ -1291,7 +1290,6 @@ describe('MDC-based MatDialog', () => {
'Expected the focus not to have changed before the animation finishes.',
);

flushMicrotasks();
viewContainerFixture.detectChanges();
tick(500);

Expand All @@ -1310,18 +1308,17 @@ describe('MDC-based MatDialog', () => {
viewContainerFixture.destroy();
const fixture = TestBed.createComponent(ShadowDomComponent);
fixture.detectChanges();
flush();
const button = fixture.debugElement.query(By.css('button'))!.nativeElement;

button.focus();

const dialogRef = dialog.open(PizzaMsg);
flushMicrotasks();
fixture.detectChanges();
flushMicrotasks();
flush();

const spy = spyOn(button, 'focus').and.callThrough();
dialogRef.close();
flushMicrotasks();
fixture.detectChanges();
tick(500);

Expand Down Expand Up @@ -1539,17 +1536,15 @@ describe('MDC-based MatDialog', () => {
restoreFocus: false,
});

flushMicrotasks();
viewContainerFixture.detectChanges();
flushMicrotasks();
flush();

expect(document.activeElement!.id).not.toBe(
'dialog-trigger',
'Expected the focus to change when dialog was opened.',
);

dialogRef.close();
flushMicrotasks();
viewContainerFixture.detectChanges();
tick(500);

Expand All @@ -1574,9 +1569,8 @@ describe('MDC-based MatDialog', () => {

const dialogRef = dialog.open(PizzaMsg, {viewContainerRef: testViewContainerRef});

flushMicrotasks();
viewContainerFixture.detectChanges();
flushMicrotasks();
flush();

expect(document.activeElement!.id).not.toBe(
'dialog-trigger',
Expand All @@ -1591,7 +1585,6 @@ describe('MDC-based MatDialog', () => {
.withContext('Expected focus to be on the alternate button.')
.toBe('other-button');

flushMicrotasks();
viewContainerFixture.detectChanges();
flush();

Expand Down

0 comments on commit a5dd5ed

Please sign in to comment.