iframe-proxy

edusperoni · 2026-06-17T16:28:43Z

PR Checklist

The PR title follows our guidelines: https://github.com/NativeScript/NativeScript/blob/master/CONTRIBUTING.md#commit-messages.
There is an issue for the bug/feature this PR is for. To avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it.
You have signed the CLA.
All existing tests are passing: https://github.com/NativeScript/angular/blob/master/DevelopmentWorkflow.md#running-the-tests
Tests for the changes are included.

What is the current behavior?

When NativeScript silently failed to present a modal (e.g. on iOS when another modal is already being presented), the dialog services swallowed the failure: the returned dialog ref/promise looked successful, the modal navigation stack stayed incremented (blocking further navigation), and the attached views/detached loaders leaked on the ApplicationRef.
The ported cdk/portal primitives (Portal, ComponentPortal, TemplatePortal, CdkPortalOutlet, NativeScriptDomPortalOutlet) were based on an older revision of @angular/components. They lacked projectableNodes/bindings/directives on ComponentPortal, the embedded-view injector on TemplatePortal, ngModuleRef/EnvironmentInjector resolution, and the ApplicationRef.viewCount guard before detaching views.
The codebase still relied on the deprecated ComponentFactoryResolver / resolveComponentFactory API in the portal outlet, detached loader, dialog config, legacy modal dialogs, and the page router outlet.

What is the new behavior?

Failed dialog opens now throw. Both NativeModalRef (new dialog API) and the legacy ModalDialogService detect a failed showModal via a new didModalOpen helper. Instead of silently failing, they roll back the modal navigation state, dispose the portal outlet / detached loader so nothing leaks, and throw a descriptive error ("Failed to open dialog: the modal view could not be presented...").
Updated the CDK-inspired portal/dialog implementation to track the latest @angular/components:
- ComponentPortal now supports projectableNodes, bindings, and directives; TemplatePortal accepts an injector.
- CdkPortal/CdkPortalOutlet migrated to inject()-based DI and the @Input getter/setter form.
- NativeScriptDomPortalOutlet resolves ngModuleRef/EnvironmentInjector, forwards the new portal options, guards detachView with viewCount, and tracks the attached portal.
- NativeDialogConfig gained bindings, which are passed through to the rendered component; template dialogs now receive the dialog injector.
Completely removed ComponentFactoryResolver from the repo, migrating every usage to createComponent / ViewContainerRef.createComponent. PageRouterOutlet.activateWith now matches Angular's current RouterOutletContract signature (EnvironmentInjector).

Fixes/Implements/Closes #[Issue Number].

BREAKING CHANGES:

A failed dialog/modal open now throws instead of failing silently. Code that opens dialogs (NativeDialog.open, ModalDialogService.showModal, and direct NativeModalRef usage) may now throw if NativeScript cannot present the modal (commonly: trying to open a dialog while another is already being presented). Previously this returned/resolved without error.
The deprecated ComponentFactoryResolver-based public API surface was dropped:
- NativeScriptDomPortalOutlet constructor no longer accepts a ComponentFactoryResolver argument.
  - Before: new NativeScriptDomPortalOutlet(view, resolver, appRef, injector)
  - After: new NativeScriptDomPortalOutlet(view, appRef, injector)
- generateDetachedLoader(resolver, injector, viewContainerRef?) lost its first resolver parameter.
  - Before: generateDetachedLoader(resolver, injector, vcRef)
  - After: generateDetachedLoader(injector, vcRef)
- The resolver option was removed from generateNativeScriptView(...)'s options object.
- NativeDialogConfig.componentFactoryResolver was removed.

Migration steps:

Wrap dialog/modal opens that may legitimately fail (e.g. opening while another modal is active) in a try/catch (or .catch() on the returned promise) and handle the error, instead of relying on the previous silent no-op.
Remove any ComponentFactoryResolver you were passing to the APIs above — Angular resolves components without it. Drop the resolver argument/option and the componentFactoryResolver config field; no replacement is required.

Summary by CodeRabbit

Release Notes

New Features
- Dialog configuration now supports a new bindings option for injecting dependencies into component-based dialogs
Bug Fixes
- Modal dialog presentation now includes automatic failure detection with recovery to prevent navigation issues
Refactor
- Modernized internal component instantiation to use Angular's current best practices
- Removed deprecated dependencies from dialog and component creation systems

coderabbitai · 2026-06-17T16:31:12Z

coderabbitai

🧹 Nitpick comments (1)

packages/angular/src/lib/legacy/directives/dialogs.ts (1)

177-192: 💤 Low value

Consider removing unused detachedLoaderRef parameter.

The detachedLoaderRef parameter is declared at line 120 but never assigned in _showDialog, so it will always be undefined when passed to _handleFailedOpen. While the optional chaining (?.) operators handle this safely, the parameter is misleading and the detectChanges() call on line 189 will never execute.

♻️ Suggested simplification

-  private _handleFailedOpen(modalParams: ModalDialogParams, portalOutlet?: NativeScriptDomPortalOutlet, detachedLoaderRef?: ComponentRef<DetachedLoader>): never {
+  private _handleFailedOpen(modalParams: ModalDialogParams, portalOutlet?: NativeScriptDomPortalOutlet): never {
     const index = this.openedModalParams?.indexOf(modalParams) ?? -1;
     if (index > -1) {
       this.openedModalParams.splice(index, 1);
     }
     this.location?._closeModalNavigation();
     portalOutlet?.dispose();
-    detachedLoaderRef?.instance.detectChanges();
-    detachedLoaderRef?.destroy();
     throw new Error('Failed to open dialog: the modal view could not be presented. This usually happens when another modal is already being presented.');
   }

And update the call site at line 172:

-        this._handleFailedOpen(modalParams, portalOutlet, detachedLoaderRef);
+        this._handleFailedOpen(modalParams, portalOutlet);

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/angular/src/lib/legacy/directives/dialogs.ts` around lines 177 -
192, The detachedLoaderRef parameter in the _handleFailedOpen method is never
assigned a value in _showDialog and will always be undefined, making it
misleading and causing the detectChanges() call to never execute. Remove the
detachedLoaderRef parameter from the _handleFailedOpen method signature, remove
the associated method calls (detachedLoaderRef?.instance.detectChanges() and
detachedLoaderRef?.destroy()) that operate on it, and update the call site where
_handleFailedOpen is invoked (around line 172) to no longer pass this parameter.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@packages/angular/src/lib/legacy/directives/dialogs.ts`:
- Around line 177-192: The detachedLoaderRef parameter in the _handleFailedOpen
method is never assigned a value in _showDialog and will always be undefined,
making it misleading and causing the detectChanges() call to never execute.
Remove the detachedLoaderRef parameter from the _handleFailedOpen method
signature, remove the associated method calls
(detachedLoaderRef?.instance.detectChanges() and detachedLoaderRef?.destroy())
that operate on it, and update the call site where _handleFailedOpen is invoked
(around line 172) to no longer pass this parameter.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 26643213-ff61-453d-ba1b-3a1132172b50

📥 Commits

Reviewing files that changed from the base of the PR and between e0dab64 and 9aacc9f.

📒 Files selected for processing (12)

apps/nativescript-demo-ng/src/tests/detached-utils-tests.spec.ts
packages/angular/src/lib/cdk/detached-loader.ts
packages/angular/src/lib/cdk/dialog/dialog-config.ts
packages/angular/src/lib/cdk/dialog/dialog-services.ts
packages/angular/src/lib/cdk/dialog/native-modal-ref.ts
packages/angular/src/lib/cdk/portal/common.ts
packages/angular/src/lib/cdk/portal/nsdom-portal-outlet.ts
packages/angular/src/lib/cdk/portal/portal-directives.ts
packages/angular/src/lib/detached-loader-utils.ts
packages/angular/src/lib/legacy/directives/dialogs.ts
packages/angular/src/lib/legacy/router/page-router-outlet.ts
packages/angular/src/lib/utils/general.ts

…ed usages

coderabbitai Bot reviewed Jun 17, 2026

View reviewed changes

edusperoni added 5 commits June 17, 2026 23:07

feat: throw error on failed to open dialog and refactor some deprecat…

44601f0

…ed usages

feat: update cdk-inspired components and drop ComponentFactoryResolver

611e4fe

chore: remove unused detachedLoaderRef

bf1389e

fix: correctly identify if modal has opened or not

4383dc8

test: properly handle race conditions in modal tests

b83e315

edusperoni force-pushed the feat/native-dialog-revamp branch from e308fd0 to b83e315 Compare June 18, 2026 02:07

Sunbelt Computer Software

PL/B Language Development and Support

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

feat: modernize CDK portal/dialog internals and drop ComponentFactoryResolver#171

feat: modernize CDK portal/dialog internals and drop ComponentFactoryResolver#171
edusperoni wants to merge 5 commits into
mainfrom
feat/native-dialog-revamp

edusperoni commented Jun 17, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 17, 2026 •

edited

Loading

Review skipped

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Sunbelt Computer Software

PL/B Language Development and Support

Uh oh!

Uh oh!

Conversation

edusperoni commented Jun 17, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Checklist

What is the current behavior?

What is the new behavior?

Summary by CodeRabbit

Release Notes

Uh oh!

coderabbitai Bot commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

edusperoni commented Jun 17, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 17, 2026 •

edited

Loading