Skip to content

Commit 1fcbca7

Browse files
edusperoniedusperoni
authored
fix: emit afterClosed only after the native modal dismissal completes (#179)
afterClosed was emitted by a hardcoded 100ms fallback timer while the iOS dismissal animation (~400ms) was still running, so opening another dialog from afterClosed failed with "the modal view could not be presented". The 'closed' state is now driven by core's showModal closeCallback, which fires once the native dismissal has completed, plus a one-shot 'unloaded' listener scoped inside it so background-driven unloads can't masquerade as a close. The timer remains only as a 5s safety net. Both portal types now share the same show/dismiss path, which also makes natively-dismissed template dialogs emit afterClosed, and afterOpened is wired to shownModally (it never fired before).
1 parent 552dc99 commit 1fcbca7

5 files changed

Lines changed: 239 additions & 93 deletions

File tree

‎apps/nativescript-demo-ng/src/tests/modal-dialog.spec.ts‎

Lines changed: 2 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -2,30 +2,9 @@
22
import { Component, inject, NgModule, NO_ERRORS_SCHEMA, ViewContainerRef } from '@angular/core';
33
import { TestBed, waitForAsync } from '@angular/core/testing';
44
import { FrameService, ModalDialogParams, ModalDialogService, NativeScriptCommonModule, NSLocationStrategy, Outlet } from '@nativescript/angular';
5-
import { Application, View } from '@nativescript/core';
65

76
import { FakeFrameService } from './ns-location-strategy.spec';
8-
9-
/**
10-
* Resolves once `condition` is truthy, polling on each frame. Unlike a fixed delay this resolves
11-
* as soon as the awaited state is reached (e.g. a modal finishing its animated dismissal), with a
12-
* bounded safety timeout so a stuck condition can't hang the suite.
13-
*/
14-
function waitUntil(condition: () => boolean, timeout = 5000): Promise<void> {
15-
return new Promise((resolve, reject) => {
16-
const start = Date.now();
17-
const check = () => {
18-
if (condition()) {
19-
resolve();
20-
} else if (Date.now() - start > timeout) {
21-
reject(new Error('Timed out waiting for condition'));
22-
} else {
23-
setTimeout(check, 16);
24-
}
25-
};
26-
check();
27-
});
28-
}
7+
import { closeRemainingModals } from './test-utils.spec';
298

309
@Component({
3110
selector: 'modal-comp',
@@ -95,23 +74,7 @@ describe('modal-dialog', () => {
9574
// done()
9675
// });
9776

98-
afterEach(async () => {
99-
// Close any modal still presented (via core's global registry) and wait until it has actually
100-
// finished dismissing before the next test runs.
101-
//
102-
// Note: `closeModal()` removes the modal from `_rootModalViews` *synchronously*, before the
103-
// animated dismissal starts, so the registry being empty does NOT mean the modal is gone. On
104-
// iOS the parent keeps a `presentedViewController` until the dismiss animation completes — and
105-
// that's exactly what makes the next `showModal` fail with "already presenting" — so wait on it.
106-
const open = ((Application.getRootView()?._getRootModalViews() ?? []) as View[]).slice();
107-
// Capture parents before closing: `closeModal()` nulls `_modalParent` synchronously.
108-
const parents = open
109-
.map((modal) => (modal as { _modalParent?: View })._modalParent)
110-
.filter((parent): parent is View => !!parent);
111-
open.forEach((modal) => modal.closeModal());
112-
const isPresenting = (parent: View) => !!(parent as { viewController?: { presentedViewController?: unknown } }).viewController?.presentedViewController;
113-
await waitUntil(() => parents.every((parent) => !isPresenting(parent))).catch(() => undefined);
114-
});
77+
afterEach(() => closeRemainingModals());
11578

11679
it('showModal does not throws when there is no viewContainer provided', waitForAsync(async () => {
11780
const fixture = TestBed.createComponent(FailComponent);

‎apps/nativescript-demo-ng/src/tests/native-dialog.spec.ts‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
import { Component, inject, NO_ERRORS_SCHEMA, TemplateRef, ViewChild, ViewContainerRef } from '@angular/core';
2+
import { TestBed } from '@angular/core/testing';
3+
import { FrameService, NativeDialogRef, NativeDialogService, NativeScriptCommonModule, NSLocationStrategy } from '@nativescript/angular';
4+
import { View } from '@nativescript/core';
5+
import { firstValueFrom } from 'rxjs';
6+
7+
import { FakeFrameService } from './ns-location-strategy.spec';
8+
import { closeRemainingModals, isPresentingModally, topRootModalView } from './test-utils.spec';
9+
10+
@Component({
11+
selector: 'dialog-content-comp',
12+
template: `<GridLayout><Label text="dialog content"></Label></GridLayout>`,
13+
imports: [NativeScriptCommonModule],
14+
schemas: [NO_ERRORS_SCHEMA],
15+
})
16+
export class DialogContentComponent {
17+
ref = inject(NativeDialogRef<DialogContentComponent>);
18+
}
19+
20+
@Component({
21+
selector: 'dialog-host-comp',
22+
template: `<GridLayout>
23+
<Label text="dialog host"></Label>
24+
<ng-template #dialogTemplate><Label text="template dialog content"></Label></ng-template>
25+
</GridLayout>`,
26+
imports: [NativeScriptCommonModule],
27+
schemas: [NO_ERRORS_SCHEMA],
28+
})
29+
export class DialogHostComponent {
30+
dialog = inject(NativeDialogService);
31+
vcRef = inject(ViewContainerRef);
32+
@ViewChild('dialogTemplate', { static: true }) dialogTemplate: TemplateRef<unknown>;
33+
}
34+
35+
describe('native-dialog', () => {
36+
beforeEach(() => {
37+
return TestBed.configureTestingModule({
38+
imports: [DialogHostComponent, DialogContentComponent, NativeScriptCommonModule],
39+
providers: [{ provide: FrameService, useValue: new FakeFrameService() }, NSLocationStrategy],
40+
}).compileComponents();
41+
});
42+
43+
afterEach(() => closeRemainingModals());
44+
45+
async function createHost(): Promise<DialogHostComponent> {
46+
const fixture = TestBed.createComponent(DialogHostComponent);
47+
fixture.detectChanges();
48+
await fixture.whenRenderingDone();
49+
return fixture.componentRef.instance;
50+
}
51+
52+
it(
53+
'afterOpened emits once the modal is fully presented',
54+
async () => {
55+
const host = await createHost();
56+
const ref = host.dialog.open(DialogContentComponent, { viewContainerRef: host.vcRef });
57+
await firstValueFrom(ref.afterOpened());
58+
const closed = firstValueFrom(ref.afterClosed());
59+
ref.close();
60+
await closed;
61+
},
62+
10000,
63+
);
64+
65+
it(
66+
'afterClosed emits only after the native dismissal completes, so a new dialog can open immediately',
67+
async () => {
68+
const host = await createHost();
69+
const hostView: View = host.vcRef.element.nativeElement;
70+
71+
const firstRef = host.dialog.open(DialogContentComponent, { viewContainerRef: host.vcRef });
72+
await firstValueFrom(firstRef.afterOpened());
73+
74+
const events: string[] = [];
75+
firstRef.beforeClosed().subscribe(() => events.push('beforeClosed'));
76+
firstRef.afterClosed().subscribe(() => events.push('afterClosed'));
77+
78+
const closed = firstValueFrom(firstRef.afterClosed());
79+
firstRef.close('first result');
80+
expect(await closed).toEqual('first result');
81+
expect(events).toEqual(['beforeClosed', 'afterClosed']);
82+
// On iOS a premature afterClosed would leave the parent still presenting the old
83+
// view controller, which is what makes the next showModal fail.
84+
expect(isPresentingModally(hostView)).toBe(false);
85+
86+
const secondRef = host.dialog.open(DialogContentComponent, { viewContainerRef: host.vcRef });
87+
await firstValueFrom(secondRef.afterOpened());
88+
const secondClosed = firstValueFrom(secondRef.afterClosed());
89+
secondRef.close();
90+
await secondClosed;
91+
},
92+
15000,
93+
);
94+
95+
it(
96+
'afterClosed emits when a component dialog is dismissed natively',
97+
async () => {
98+
const host = await createHost();
99+
const ref = host.dialog.open(DialogContentComponent, { viewContainerRef: host.vcRef });
100+
await firstValueFrom(ref.afterOpened());
101+
102+
const closed = firstValueFrom(ref.afterClosed());
103+
// Dismiss through core, like the user swiping down or pressing back.
104+
topRootModalView().closeModal();
105+
await closed;
106+
},
107+
10000,
108+
);
109+
110+
it(
111+
'afterClosed emits when a template dialog is dismissed natively',
112+
async () => {
113+
const host = await createHost();
114+
const ref = host.dialog.open(host.dialogTemplate, { viewContainerRef: host.vcRef });
115+
await firstValueFrom(ref.afterOpened());
116+
117+
const closed = firstValueFrom(ref.afterClosed());
118+
topRootModalView().closeModal();
119+
await closed;
120+
},
121+
10000,
122+
);
123+
});

‎apps/nativescript-demo-ng/src/tests/test-utils.spec.ts‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { Application } from '@nativescript/core';
12
import { View } from '@nativescript/core/ui/core/view';
23
import { TextBase } from '@nativescript/core/ui/text-base';
34
import { Device } from '@nativescript/core/platform';
@@ -37,6 +38,67 @@ export function dumpView(view: View, verbose: boolean = false): string {
3738
return output.join('');
3839
}
3940

41+
/**
42+
* Resolves once `condition` is truthy, polling on each frame. Unlike a fixed delay this resolves
43+
* as soon as the awaited state is reached (e.g. a modal finishing its animated dismissal), with a
44+
* bounded safety timeout so a stuck condition can't hang the suite.
45+
*/
46+
export function waitUntil(condition: () => boolean, timeout = 5000): Promise<void> {
47+
return new Promise((resolve, reject) => {
48+
const start = Date.now();
49+
const check = () => {
50+
if (condition()) {
51+
resolve();
52+
} else if (Date.now() - start > timeout) {
53+
reject(new Error('Timed out waiting for condition'));
54+
} else {
55+
setTimeout(check, 16);
56+
}
57+
};
58+
check();
59+
});
60+
}
61+
62+
/**
63+
* Returns true while any ancestor view controller of `view` is still presenting modally.
64+
* Only meaningful on iOS; on Android there is no `viewController` and this returns false.
65+
*/
66+
export function isPresentingModally(view: View): boolean {
67+
let current = view;
68+
while (current) {
69+
if ((current as { viewController?: { presentedViewController?: unknown } }).viewController?.presentedViewController) {
70+
return true;
71+
}
72+
current = current.parent as View;
73+
}
74+
return false;
75+
}
76+
77+
/** The most recently presented modal view still tracked in core's global registry. */
78+
export function topRootModalView(): View | undefined {
79+
const modals = ((Application.getRootView()?._getRootModalViews() ?? []) as View[]).slice();
80+
return modals[modals.length - 1];
81+
}
82+
83+
/**
84+
* Close any modal still presented (via core's global registry) and wait until it has actually
85+
* finished dismissing before the next test runs.
86+
*
87+
* Note: `closeModal()` removes the modal from `_rootModalViews` *synchronously*, before the
88+
* animated dismissal starts, so the registry being empty does NOT mean the modal is gone. On
89+
* iOS the parent keeps a `presentedViewController` until the dismiss animation completes — and
90+
* that's exactly what makes the next `showModal` fail with "already presenting" — so wait on it.
91+
*/
92+
export async function closeRemainingModals(): Promise<void> {
93+
const open = ((Application.getRootView()?._getRootModalViews() ?? []) as View[]).slice();
94+
// Capture parents before closing: `closeModal()` nulls `_modalParent` synchronously.
95+
const parents = open
96+
.map((modal) => (modal as { _modalParent?: View })._modalParent)
97+
.filter((parent): parent is View => !!parent);
98+
open.forEach((modal) => modal.closeModal());
99+
await waitUntil(() => parents.every((parent) => !isPresentingModally(parent))).catch(() => undefined);
100+
}
101+
40102
export function createDevice(os: string): typeof Device {
41103
return {
42104
os: os,

‎packages/angular/src/lib/cdk/dialog/dialog-ref.ts‎

Lines changed: 15 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,14 @@ import { NativeModalRef } from './native-modal-ref';
55
// Counter for unique dialog ids.
66
let uniqueId = 0;
77

8+
/**
9+
* Safety-net delay before `afterClosed` is forced when the native dismissal never reports
10+
* completion (e.g. the parent view is destroyed mid-animation). Must exceed the longest
11+
* modal dismiss animation, including custom transitions, so it can't preempt a normal
12+
* close — the 'closed' state emitted after the native dismissal is the real trigger.
13+
*/
14+
const CLOSE_FALLBACK_TIMEOUT = 5000;
15+
816
/** Possible states of the lifecycle of a dialog. */
917
export const enum NativeDialogState {
1018
OPEN,
@@ -31,7 +39,7 @@ export class NativeDialogRef<T, R = any> {
3139
/** Result to be passed to afterClosed. */
3240
private _result: R | undefined;
3341

34-
/** Handle to the timeout that's running as a fallback in case the exit animation doesn't fire. */
42+
/** Handle to the safety-net timeout in case the native dismissal never reports completion. */
3543
private _closeFallbackTimeout: any;
3644

3745
/** Current state of the dialog. */
@@ -82,7 +90,6 @@ export class NativeDialogRef<T, R = any> {
8290
close(dialogResult?: R): void {
8391
this._result = dialogResult;
8492

85-
// Transition the backdrop in parallel to the dialog.
8693
this._nativeModalRef.stateChanged
8794
.pipe(
8895
filter((event) => event.state === 'closing'),
@@ -92,22 +99,12 @@ export class NativeDialogRef<T, R = any> {
9299
this._beforeClosed.next(dialogResult);
93100
this._beforeClosed.complete();
94101
this._nativeModalRef.dispose();
95-
// this._overlayRef.detachBackdrop();
96-
97-
// The logic that disposes of the overlay depends on the exit animation completing, however
98-
// it isn't guaranteed if the parent view is destroyed while it's running. Add a fallback
99-
// timeout which will clean everything up if the animation hasn't fired within the specified
100-
// amount of time plus 100ms. We don't need to run this outside the NgZone, because for the
101-
// vast majority of cases the timeout will have been cleared before it has the chance to fire.
102-
this._closeFallbackTimeout = setTimeout(
103-
() => {
104-
this._finishDialogClose();
105-
this._afterClosed.next(this._result);
106-
this._afterClosed.complete();
107-
},
108-
//event.totalTime + 100);
109-
100
110-
);
102+
103+
this._closeFallbackTimeout = setTimeout(() => {
104+
this._finishDialogClose();
105+
this._afterClosed.next(this._result);
106+
this._afterClosed.complete();
107+
}, CLOSE_FALLBACK_TIMEOUT);
111108
});
112109

113110
this._state = NativeDialogState.CLOSING;

0 commit comments

Comments
 (0)