From e4435a609316cc18b1b8ec88de290630e5ed6a78 Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Mon, 24 Aug 2026 12:52:05 -0300 Subject: [PATCH] fix: emit afterClosed only after the native modal dismissal completes 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). --- .../src/tests/modal-dialog.spec.ts | 41 +----- .../src/tests/native-dialog.spec.ts | 123 ++++++++++++++++++ .../src/tests/test-utils.spec.ts | 62 +++++++++ .../angular/src/lib/cdk/dialog/dialog-ref.ts | 33 +++-- .../src/lib/cdk/dialog/native-modal-ref.ts | 73 ++++++----- 5 files changed, 239 insertions(+), 93 deletions(-) create mode 100644 apps/nativescript-demo-ng/src/tests/native-dialog.spec.ts diff --git a/apps/nativescript-demo-ng/src/tests/modal-dialog.spec.ts b/apps/nativescript-demo-ng/src/tests/modal-dialog.spec.ts index 117710b9..ce4f11a6 100644 --- a/apps/nativescript-demo-ng/src/tests/modal-dialog.spec.ts +++ b/apps/nativescript-demo-ng/src/tests/modal-dialog.spec.ts @@ -2,30 +2,9 @@ import { Component, inject, NgModule, NO_ERRORS_SCHEMA, ViewContainerRef } from '@angular/core'; import { TestBed, waitForAsync } from '@angular/core/testing'; import { FrameService, ModalDialogParams, ModalDialogService, NativeScriptCommonModule, NSLocationStrategy, Outlet } from '@nativescript/angular'; -import { Application, View } from '@nativescript/core'; import { FakeFrameService } from './ns-location-strategy.spec'; - -/** - * Resolves once `condition` is truthy, polling on each frame. Unlike a fixed delay this resolves - * as soon as the awaited state is reached (e.g. a modal finishing its animated dismissal), with a - * bounded safety timeout so a stuck condition can't hang the suite. - */ -function waitUntil(condition: () => boolean, timeout = 5000): Promise { - return new Promise((resolve, reject) => { - const start = Date.now(); - const check = () => { - if (condition()) { - resolve(); - } else if (Date.now() - start > timeout) { - reject(new Error('Timed out waiting for condition')); - } else { - setTimeout(check, 16); - } - }; - check(); - }); -} +import { closeRemainingModals } from './test-utils.spec'; @Component({ selector: 'modal-comp', @@ -95,23 +74,7 @@ describe('modal-dialog', () => { // done() // }); - afterEach(async () => { - // Close any modal still presented (via core's global registry) and wait until it has actually - // finished dismissing before the next test runs. - // - // Note: `closeModal()` removes the modal from `_rootModalViews` *synchronously*, before the - // animated dismissal starts, so the registry being empty does NOT mean the modal is gone. On - // iOS the parent keeps a `presentedViewController` until the dismiss animation completes — and - // that's exactly what makes the next `showModal` fail with "already presenting" — so wait on it. - const open = ((Application.getRootView()?._getRootModalViews() ?? []) as View[]).slice(); - // Capture parents before closing: `closeModal()` nulls `_modalParent` synchronously. - const parents = open - .map((modal) => (modal as { _modalParent?: View })._modalParent) - .filter((parent): parent is View => !!parent); - open.forEach((modal) => modal.closeModal()); - const isPresenting = (parent: View) => !!(parent as { viewController?: { presentedViewController?: unknown } }).viewController?.presentedViewController; - await waitUntil(() => parents.every((parent) => !isPresenting(parent))).catch(() => undefined); - }); + afterEach(() => closeRemainingModals()); it('showModal does not throws when there is no viewContainer provided', waitForAsync(async () => { const fixture = TestBed.createComponent(FailComponent); diff --git a/apps/nativescript-demo-ng/src/tests/native-dialog.spec.ts b/apps/nativescript-demo-ng/src/tests/native-dialog.spec.ts new file mode 100644 index 00000000..d092b22c --- /dev/null +++ b/apps/nativescript-demo-ng/src/tests/native-dialog.spec.ts @@ -0,0 +1,123 @@ +import { Component, inject, NO_ERRORS_SCHEMA, TemplateRef, ViewChild, ViewContainerRef } from '@angular/core'; +import { TestBed } from '@angular/core/testing'; +import { FrameService, NativeDialogRef, NativeDialogService, NativeScriptCommonModule, NSLocationStrategy } from '@nativescript/angular'; +import { View } from '@nativescript/core'; +import { firstValueFrom } from 'rxjs'; + +import { FakeFrameService } from './ns-location-strategy.spec'; +import { closeRemainingModals, isPresentingModally, topRootModalView } from './test-utils.spec'; + +@Component({ + selector: 'dialog-content-comp', + template: ``, + imports: [NativeScriptCommonModule], + schemas: [NO_ERRORS_SCHEMA], +}) +export class DialogContentComponent { + ref = inject(NativeDialogRef); +} + +@Component({ + selector: 'dialog-host-comp', + template: ` + + + `, + imports: [NativeScriptCommonModule], + schemas: [NO_ERRORS_SCHEMA], +}) +export class DialogHostComponent { + dialog = inject(NativeDialogService); + vcRef = inject(ViewContainerRef); + @ViewChild('dialogTemplate', { static: true }) dialogTemplate: TemplateRef; +} + +describe('native-dialog', () => { + beforeEach(() => { + return TestBed.configureTestingModule({ + imports: [DialogHostComponent, DialogContentComponent, NativeScriptCommonModule], + providers: [{ provide: FrameService, useValue: new FakeFrameService() }, NSLocationStrategy], + }).compileComponents(); + }); + + afterEach(() => closeRemainingModals()); + + async function createHost(): Promise { + const fixture = TestBed.createComponent(DialogHostComponent); + fixture.detectChanges(); + await fixture.whenRenderingDone(); + return fixture.componentRef.instance; + } + + it( + 'afterOpened emits once the modal is fully presented', + async () => { + const host = await createHost(); + const ref = host.dialog.open(DialogContentComponent, { viewContainerRef: host.vcRef }); + await firstValueFrom(ref.afterOpened()); + const closed = firstValueFrom(ref.afterClosed()); + ref.close(); + await closed; + }, + 10000, + ); + + it( + 'afterClosed emits only after the native dismissal completes, so a new dialog can open immediately', + async () => { + const host = await createHost(); + const hostView: View = host.vcRef.element.nativeElement; + + const firstRef = host.dialog.open(DialogContentComponent, { viewContainerRef: host.vcRef }); + await firstValueFrom(firstRef.afterOpened()); + + const events: string[] = []; + firstRef.beforeClosed().subscribe(() => events.push('beforeClosed')); + firstRef.afterClosed().subscribe(() => events.push('afterClosed')); + + const closed = firstValueFrom(firstRef.afterClosed()); + firstRef.close('first result'); + expect(await closed).toEqual('first result'); + expect(events).toEqual(['beforeClosed', 'afterClosed']); + // On iOS a premature afterClosed would leave the parent still presenting the old + // view controller, which is what makes the next showModal fail. + expect(isPresentingModally(hostView)).toBe(false); + + const secondRef = host.dialog.open(DialogContentComponent, { viewContainerRef: host.vcRef }); + await firstValueFrom(secondRef.afterOpened()); + const secondClosed = firstValueFrom(secondRef.afterClosed()); + secondRef.close(); + await secondClosed; + }, + 15000, + ); + + it( + 'afterClosed emits when a component dialog is dismissed natively', + async () => { + const host = await createHost(); + const ref = host.dialog.open(DialogContentComponent, { viewContainerRef: host.vcRef }); + await firstValueFrom(ref.afterOpened()); + + const closed = firstValueFrom(ref.afterClosed()); + // Dismiss through core, like the user swiping down or pressing back. + topRootModalView().closeModal(); + await closed; + }, + 10000, + ); + + it( + 'afterClosed emits when a template dialog is dismissed natively', + async () => { + const host = await createHost(); + const ref = host.dialog.open(host.dialogTemplate, { viewContainerRef: host.vcRef }); + await firstValueFrom(ref.afterOpened()); + + const closed = firstValueFrom(ref.afterClosed()); + topRootModalView().closeModal(); + await closed; + }, + 10000, + ); +}); diff --git a/apps/nativescript-demo-ng/src/tests/test-utils.spec.ts b/apps/nativescript-demo-ng/src/tests/test-utils.spec.ts index d2009185..a4de7e32 100644 --- a/apps/nativescript-demo-ng/src/tests/test-utils.spec.ts +++ b/apps/nativescript-demo-ng/src/tests/test-utils.spec.ts @@ -1,3 +1,4 @@ +import { Application } from '@nativescript/core'; import { View } from '@nativescript/core/ui/core/view'; import { TextBase } from '@nativescript/core/ui/text-base'; import { Device } from '@nativescript/core/platform'; @@ -37,6 +38,67 @@ export function dumpView(view: View, verbose: boolean = false): string { return output.join(''); } +/** + * Resolves once `condition` is truthy, polling on each frame. Unlike a fixed delay this resolves + * as soon as the awaited state is reached (e.g. a modal finishing its animated dismissal), with a + * bounded safety timeout so a stuck condition can't hang the suite. + */ +export function waitUntil(condition: () => boolean, timeout = 5000): Promise { + return new Promise((resolve, reject) => { + const start = Date.now(); + const check = () => { + if (condition()) { + resolve(); + } else if (Date.now() - start > timeout) { + reject(new Error('Timed out waiting for condition')); + } else { + setTimeout(check, 16); + } + }; + check(); + }); +} + +/** + * Returns true while any ancestor view controller of `view` is still presenting modally. + * Only meaningful on iOS; on Android there is no `viewController` and this returns false. + */ +export function isPresentingModally(view: View): boolean { + let current = view; + while (current) { + if ((current as { viewController?: { presentedViewController?: unknown } }).viewController?.presentedViewController) { + return true; + } + current = current.parent as View; + } + return false; +} + +/** The most recently presented modal view still tracked in core's global registry. */ +export function topRootModalView(): View | undefined { + const modals = ((Application.getRootView()?._getRootModalViews() ?? []) as View[]).slice(); + return modals[modals.length - 1]; +} + +/** + * Close any modal still presented (via core's global registry) and wait until it has actually + * finished dismissing before the next test runs. + * + * Note: `closeModal()` removes the modal from `_rootModalViews` *synchronously*, before the + * animated dismissal starts, so the registry being empty does NOT mean the modal is gone. On + * iOS the parent keeps a `presentedViewController` until the dismiss animation completes — and + * that's exactly what makes the next `showModal` fail with "already presenting" — so wait on it. + */ +export async function closeRemainingModals(): Promise { + const open = ((Application.getRootView()?._getRootModalViews() ?? []) as View[]).slice(); + // Capture parents before closing: `closeModal()` nulls `_modalParent` synchronously. + const parents = open + .map((modal) => (modal as { _modalParent?: View })._modalParent) + .filter((parent): parent is View => !!parent); + open.forEach((modal) => modal.closeModal()); + await waitUntil(() => parents.every((parent) => !isPresentingModally(parent))).catch(() => undefined); +} + export function createDevice(os: string): typeof Device { return { os: os, diff --git a/packages/angular/src/lib/cdk/dialog/dialog-ref.ts b/packages/angular/src/lib/cdk/dialog/dialog-ref.ts index 79c4ab04..1310ea6e 100644 --- a/packages/angular/src/lib/cdk/dialog/dialog-ref.ts +++ b/packages/angular/src/lib/cdk/dialog/dialog-ref.ts @@ -5,6 +5,14 @@ import { NativeModalRef } from './native-modal-ref'; // Counter for unique dialog ids. let uniqueId = 0; +/** + * Safety-net delay before `afterClosed` is forced when the native dismissal never reports + * completion (e.g. the parent view is destroyed mid-animation). Must exceed the longest + * modal dismiss animation, including custom transitions, so it can't preempt a normal + * close — the 'closed' state emitted after the native dismissal is the real trigger. + */ +const CLOSE_FALLBACK_TIMEOUT = 5000; + /** Possible states of the lifecycle of a dialog. */ export const enum NativeDialogState { OPEN, @@ -31,7 +39,7 @@ export class NativeDialogRef { /** Result to be passed to afterClosed. */ private _result: R | undefined; - /** Handle to the timeout that's running as a fallback in case the exit animation doesn't fire. */ + /** Handle to the safety-net timeout in case the native dismissal never reports completion. */ private _closeFallbackTimeout: any; /** Current state of the dialog. */ @@ -82,7 +90,6 @@ export class NativeDialogRef { close(dialogResult?: R): void { this._result = dialogResult; - // Transition the backdrop in parallel to the dialog. this._nativeModalRef.stateChanged .pipe( filter((event) => event.state === 'closing'), @@ -92,22 +99,12 @@ export class NativeDialogRef { this._beforeClosed.next(dialogResult); this._beforeClosed.complete(); this._nativeModalRef.dispose(); - // this._overlayRef.detachBackdrop(); - - // The logic that disposes of the overlay depends on the exit animation completing, however - // it isn't guaranteed if the parent view is destroyed while it's running. Add a fallback - // timeout which will clean everything up if the animation hasn't fired within the specified - // amount of time plus 100ms. We don't need to run this outside the NgZone, because for the - // vast majority of cases the timeout will have been cleared before it has the chance to fire. - this._closeFallbackTimeout = setTimeout( - () => { - this._finishDialogClose(); - this._afterClosed.next(this._result); - this._afterClosed.complete(); - }, - //event.totalTime + 100); - 100 - ); + + this._closeFallbackTimeout = setTimeout(() => { + this._finishDialogClose(); + this._afterClosed.next(this._result); + this._afterClosed.complete(); + }, CLOSE_FALLBACK_TIMEOUT); }); this._state = NativeDialogState.CLOSING; diff --git a/packages/angular/src/lib/cdk/dialog/native-modal-ref.ts b/packages/angular/src/lib/cdk/dialog/native-modal-ref.ts index e2a8f6de..3852d237 100644 --- a/packages/angular/src/lib/cdk/dialog/native-modal-ref.ts +++ b/packages/angular/src/lib/cdk/dialog/native-modal-ref.ts @@ -1,7 +1,6 @@ import { ApplicationRef, ComponentRef, createComponent, EmbeddedViewRef, Injector, Optional, ViewContainerRef } from '@angular/core'; import { Application, ContentView, Frame, View } from '@nativescript/core'; -import { fromEvent, Subject } from 'rxjs'; -import { take } from 'rxjs/operators'; +import { Subject } from 'rxjs'; import { AppHostAsyncView, AppHostView } from '../../app-host-view'; import { NSLocationStrategy } from '../../legacy/router/ns-location-strategy'; import { didModalOpen, once } from '../../utils/general'; @@ -49,20 +48,11 @@ export class NativeModalRef { } this.parentView = parentView; - this._closeCallback = once(async () => { + this._closeCallback = once(() => { this.stateChanged.next({ state: 'closing' }); if (!this._isDismissed) { this.modalViewRef.firstNativeLikeView?.closeModal(); } - await this.location?._closeModalNavigation(); - // this.detachedLoaderRef?.destroy(); - if (this.modalViewRef?.firstNativeLikeView.isLoaded) { - fromEvent(this.modalViewRef.firstNativeLikeView, 'unloaded') - .pipe(take(1)) - .subscribe(() => this.stateChanged.next({ state: 'closed' })); - } else { - this.stateChanged.next({ state: 'closed' }); - } }); } @@ -93,21 +83,7 @@ export class NativeModalRef { // if we don't detach the view from its parent, ios gets mad this.modalViewRef.detachNativeLikeView(); - const userOptions = this._config.nativeOptions || {}; - const modalView = this.modalViewRef.firstNativeLikeView; - this.parentView.showModal(modalView, { - context: null, - ...userOptions, - closeCallback: async () => { - await this.location?._closeModalNavigation(); - this.onDismiss.next(); - this.onDismiss.complete(); - }, - cancelable: !this._config.disableClose, - }); - if (!didModalOpen(this.parentView, modalView)) { - this._handleFailedOpen(); - } + this._showModal(this.modalViewRef.firstNativeLikeView); // if (this.modalView !== templateRef.rootNodes[0]) { // componentRef.location.nativeElement._ngDialogRoot = this.modalView; // } @@ -129,27 +105,52 @@ export class NativeModalRef { // if we don't detach the view from its parent, ios gets mad this.modalViewRef.detachNativeLikeView(); + this._showModal(this.modalViewRef.firstNativeLikeView); + return componentRef; + } + + _startExitAnimation() { + this._closeCallback(); + } + + private _showModal(modalView: View): void { + modalView.once(View.shownModallyEvent, () => this.stateChanged.next({ state: 'opened' })); const userOptions = this._config.nativeOptions || {}; - const modalView = this.modalViewRef.firstNativeLikeView; this.parentView.showModal(modalView, { context: null, ...userOptions, - closeCallback: async () => { - this._isDismissed = true; - this._closeCallback(); // close callback can only be called once, so we call it here to setup the exit events - this.onDismiss.next(); - this.onDismiss.complete(); - }, + closeCallback: () => this._onDismissed(), cancelable: !this._config.disableClose, }); if (!didModalOpen(this.parentView, modalView)) { this._handleFailedOpen(); } - return componentRef; } - _startExitAnimation() { + /** + * Runs when core reports the native dismissal as complete (the `showModal` closeCallback — + * on iOS the `dismissViewControllerAnimated` completion handler). The `unloaded` listener + * must not be attached before this point: a view also unloads for unrelated reasons (e.g. + * the app going to the background), which would signal 'closed' while the modal is still + * presented and make the next `showModal` fail. + */ + private async _onDismissed(): Promise { + this._isDismissed = true; + // Emits 'closing' when the dismissal was native-initiated (back button, swipe-down) + // and NativeDialogRef.close() was never called. this._closeCallback(); + + // Core tears the view down right after this callback returns, so the listener has to be + // attached synchronously, before any await. + const modalView = this.modalViewRef?.firstNativeLikeView; + const whenUnloaded = modalView?.isLoaded ? new Promise((resolve) => modalView.once(View.unloadedEvent, () => resolve())) : Promise.resolve(); + + await this.location?._closeModalNavigation(); + this.onDismiss.next(); + this.onDismiss.complete(); + + await whenUnloaded; + this.stateChanged.next({ state: 'closed' }); } /**