From d04010c534ffbbbf78d33527db03e0c6da78427b Mon Sep 17 00:00:00 2001 From: Dimitris - Rafail Katsampas Date: Sat, 1 Mar 2025 16:24:04 +0200 Subject: [PATCH 01/23] fix(core): Better handling for nested frames --- packages/core/ui/core/view-base/index.ts | 14 ++++++--- packages/core/ui/frame/frame-common.ts | 7 ----- packages/core/ui/frame/index.android.ts | 40 +++++++++++++++++------- packages/core/ui/frame/index.ios.ts | 8 +++++ 4 files changed, 45 insertions(+), 24 deletions(-) diff --git a/packages/core/ui/core/view-base/index.ts b/packages/core/ui/core/view-base/index.ts index 39f0668a44..05d0fa5caf 100644 --- a/packages/core/ui/core/view-base/index.ts +++ b/packages/core/ui/core/view-base/index.ts @@ -686,6 +686,14 @@ export abstract class ViewBase extends Observable implements ViewBaseDefinition this[name] = WrappedValue.unwrap(value); } + public loadChildren(): void { + this.eachChild((child) => { + this.loadView(child); + + return true; + }); + } + @profile public onLoaded() { this.setFlag(Flags.superOnLoadedCalled, true); @@ -697,11 +705,7 @@ export abstract class ViewBase extends Observable implements ViewBaseDefinition this._cssState.onLoaded(); this._resumeNativeUpdates(SuspendType.Loaded); - this.eachChild((child) => { - this.loadView(child); - - return true; - }); + this.loadChildren(); this._emit('loaded'); } diff --git a/packages/core/ui/frame/frame-common.ts b/packages/core/ui/frame/frame-common.ts index 3a42c2979d..78f92fcbe4 100644 --- a/packages/core/ui/frame/frame-common.ts +++ b/packages/core/ui/frame/frame-common.ts @@ -119,13 +119,6 @@ export class FrameBase extends CustomLayoutView { throw new Error(`Frame should not have a view. Use 'defaultPage' property instead.`); } - @profile - public onLoaded() { - super.onLoaded(); - - this._processNextNavigationEntry(); - } - public canGoBack(): boolean { let backstack = this._backStack.length; let previousForwardNotInBackstack = false; diff --git a/packages/core/ui/frame/index.android.ts b/packages/core/ui/frame/index.android.ts index 1bf7569a14..4541a83155 100644 --- a/packages/core/ui/frame/index.android.ts +++ b/packages/core/ui/frame/index.android.ts @@ -241,23 +241,33 @@ export class Frame extends FrameBase { this.backgroundColor = this._originalBackground; this._originalBackground = null; } + + super.onLoaded(); + } + + public override loadChildren(): void { + // Process navigation entry after loading view but before loading children views, otherwise the navigation entries of nested frames will be processed first + // and needed fragments won't have been attached yet + this._processNextNavigationEntry(); + + // TODO: Check if this is still needed since there have been new improvements regarding fragment restoration + // there's a bug with nested frames where sometimes the nested fragment is not recreated at all + // so we manually check on loaded event if the fragment is not recreated and recreate it this._frameCreateTimeout = setTimeout(() => { - // there's a bug with nested frames where sometimes the nested fragment is not recreated at all - // so we manually check on loaded event if the fragment is not recreated and recreate it const currentEntry = this._currentEntry || this._executingContext?.entry; - if (currentEntry) { - if (!currentEntry.fragment) { - const manager = this._getFragmentManager(); - const transaction = manager.beginTransaction(); - currentEntry.fragment = this.createFragment(currentEntry, currentEntry.fragmentTag); - _updateTransitions(currentEntry); - transaction.replace(this.containerViewId, currentEntry.fragment, currentEntry.fragmentTag); - transaction.commitAllowingStateLoss(); - } + if (currentEntry && !currentEntry.fragment) { + const manager = this._getFragmentManager(); + const transaction = manager.beginTransaction(); + currentEntry.fragment = this.createFragment(currentEntry, currentEntry.fragmentTag); + _updateTransitions(currentEntry); + transaction.replace(this.containerViewId, currentEntry.fragment, currentEntry.fragmentTag); + transaction.commitAllowingStateLoss(); } + + this._frameCreateTimeout = null; }, 0); - super.onLoaded(); + super.loadChildren(); } onUnloaded() { @@ -529,11 +539,14 @@ export class Frame extends FrameBase { public disposeNativeView() { const listener = getAttachListener(); + this.nativeViewProtected.removeOnAttachStateChangeListener(listener); this.nativeViewProtected[ownerSymbol] = null; this._tearDownPending = !!this._executingContext; + const current = this._currentEntry; const executingEntry = this._executingContext ? this._executingContext.entry : null; + this.backStack.forEach((entry) => { // Don't destroy current and executing entries or UI will look blank. // We will do it in setCurrent. @@ -546,6 +559,9 @@ export class Frame extends FrameBase { this._disposeBackstackEntry(current); } + // There are cases transition state is still cached even during disposal as setCurrent may not necessarily be called to clean it up + this._cachedTransitionState = null; + this._android.rootViewGroup = null; this._removeFromFrameStack(); super.disposeNativeView(); diff --git a/packages/core/ui/frame/index.ios.ts b/packages/core/ui/frame/index.ios.ts index 33787b214c..dc18546f0a 100644 --- a/packages/core/ui/frame/index.ios.ts +++ b/packages/core/ui/frame/index.ios.ts @@ -75,6 +75,14 @@ export class Frame extends FrameBase { return this._ios; } + public override loadChildren(): void { + // Process navigation entry after loading view but before loading children views, otherwise the navigation entries of nested frames will be processed first + // and needed fragments won't have been attached yet + this._processNextNavigationEntry(); + + super.loadChildren(); + } + public setCurrent(entry: BackstackEntry, navigationType: NavigationType): void { const current = this._currentEntry; const currentEntryChanged = current !== entry; From 66817174f70f6a678095aa04ee30d037ebdc4a76 Mon Sep 17 00:00:00 2001 From: Dimitris - Rafail Katsampas Date: Sat, 1 Mar 2025 16:57:54 +0200 Subject: [PATCH 02/23] test: Updated tab-view automated tests --- apps/automated/src/ui/tab-view/tab-view-root-tests.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/automated/src/ui/tab-view/tab-view-root-tests.ts b/apps/automated/src/ui/tab-view/tab-view-root-tests.ts index 8d5129afcd..8b91bd7e80 100644 --- a/apps/automated/src/ui/tab-view/tab-view-root-tests.ts +++ b/apps/automated/src/ui/tab-view/tab-view-root-tests.ts @@ -133,7 +133,7 @@ export function test_offset_zero_should_raise_same_events() { waitUntilNavigatedToMaxTimeout([items[0].page], () => Application.resetRootView(entry)); - const expectedEventsRaisedAfterTabCreated = [['Tab0 Frame0 loaded', 'Tab0 Frame0 Page0 navigatingTo', 'Tab0 Frame0 Page0 loaded', 'Tab0 Frame0 Page0 navigatedTo'], [], []]; + const expectedEventsRaisedAfterTabCreated = [['Tab0 Frame0 Page0 navigatingTo', 'Tab0 Frame0 loaded', 'Tab0 Frame0 Page0 loaded', 'Tab0 Frame0 Page0 navigatedTo'], [], []]; TKUnit.assertDeepEqual(actualEventsRaised, expectedEventsRaisedAfterTabCreated); @@ -199,14 +199,14 @@ export function test_android_default_offset_should_preload_1_tab_on_each_side() waitUntilNavigatedToMaxTimeout([items[0].page, items[1].page], () => Application.resetRootView(entry)); - const expectedEventsRaisedAfterTabCreated = [['Tab0 Frame0 loaded', 'Tab0 Frame0 Page0 navigatingTo', 'Tab0 Frame0 Page0 loaded', 'Tab0 Frame0 Page0 navigatedTo'], ['Tab1 Frame1 loaded', 'Tab1 Frame1 Page1 navigatingTo', 'Tab1 Frame1 Page1 loaded', 'Tab1 Frame1 Page1 navigatedTo'], []]; + const expectedEventsRaisedAfterTabCreated = [['Tab0 Frame0 Page0 navigatingTo', 'Tab0 Frame0 loaded', 'Tab0 Frame0 Page0 loaded', 'Tab0 Frame0 Page0 navigatedTo'], ['Tab1 Frame1 Page1 navigatingTo', 'Tab1 Frame1 loaded', 'Tab1 Frame1 Page1 loaded', 'Tab1 Frame1 Page1 navigatedTo'], []]; TKUnit.assertDeepEqual(actualEventsRaised, expectedEventsRaisedAfterTabCreated); resetActualEventsRaised(); waitUntilNavigatedToMaxTimeout([items[2].page], () => (tabView.selectedIndex = 2)); - const expectedEventsRaisedAfterSelectThirdTab = [['Tab0 Frame0 Page0 unloaded', 'Tab0 Frame0 unloaded'], [], ['Tab2 Frame2 loaded', 'Tab2 Frame2 Page2 navigatingTo', 'Tab2 Frame2 Page2 loaded', 'Tab2 Frame2 Page2 navigatedTo']]; + const expectedEventsRaisedAfterSelectThirdTab = [['Tab0 Frame0 Page0 unloaded', 'Tab0 Frame0 unloaded'], [], ['Tab2 Frame2 Page2 navigatingTo', 'Tab2 Frame2 loaded', 'Tab2 Frame2 Page2 loaded', 'Tab2 Frame2 Page2 navigatedTo']]; TKUnit.assertDeepEqual(actualEventsRaised, expectedEventsRaisedAfterSelectThirdTab); From 1edaae1d256f1efb3a0cef72917fa2709dbea73c Mon Sep 17 00:00:00 2001 From: Dimitris - Rafail Katsampas Date: Sat, 1 Mar 2025 17:02:44 +0200 Subject: [PATCH 03/23] chore: Renamed new load children method --- packages/core/ui/core/view-base/index.ts | 4 ++-- packages/core/ui/frame/index.android.ts | 4 ++-- packages/core/ui/frame/index.ios.ts | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/core/ui/core/view-base/index.ts b/packages/core/ui/core/view-base/index.ts index 05d0fa5caf..46215bbacb 100644 --- a/packages/core/ui/core/view-base/index.ts +++ b/packages/core/ui/core/view-base/index.ts @@ -686,7 +686,7 @@ export abstract class ViewBase extends Observable implements ViewBaseDefinition this[name] = WrappedValue.unwrap(value); } - public loadChildren(): void { + public loadSubviews(): void { this.eachChild((child) => { this.loadView(child); @@ -705,7 +705,7 @@ export abstract class ViewBase extends Observable implements ViewBaseDefinition this._cssState.onLoaded(); this._resumeNativeUpdates(SuspendType.Loaded); - this.loadChildren(); + this.loadSubviews(); this._emit('loaded'); } diff --git a/packages/core/ui/frame/index.android.ts b/packages/core/ui/frame/index.android.ts index 4541a83155..cd55537736 100644 --- a/packages/core/ui/frame/index.android.ts +++ b/packages/core/ui/frame/index.android.ts @@ -245,7 +245,7 @@ export class Frame extends FrameBase { super.onLoaded(); } - public override loadChildren(): void { + public override loadSubviews(): void { // Process navigation entry after loading view but before loading children views, otherwise the navigation entries of nested frames will be processed first // and needed fragments won't have been attached yet this._processNextNavigationEntry(); @@ -267,7 +267,7 @@ export class Frame extends FrameBase { this._frameCreateTimeout = null; }, 0); - super.loadChildren(); + super.loadSubviews(); } onUnloaded() { diff --git a/packages/core/ui/frame/index.ios.ts b/packages/core/ui/frame/index.ios.ts index dc18546f0a..623da3b092 100644 --- a/packages/core/ui/frame/index.ios.ts +++ b/packages/core/ui/frame/index.ios.ts @@ -75,12 +75,12 @@ export class Frame extends FrameBase { return this._ios; } - public override loadChildren(): void { + public override loadSubviews(): void { // Process navigation entry after loading view but before loading children views, otherwise the navigation entries of nested frames will be processed first // and needed fragments won't have been attached yet this._processNextNavigationEntry(); - super.loadChildren(); + super.loadSubviews(); } public setCurrent(entry: BackstackEntry, navigationType: NavigationType): void { From f8991e6519336c950a7fe213e057ac1f27b8d8c3 Mon Sep 17 00:00:00 2001 From: Dimitris - Rafail Katsampas Date: Mon, 3 Mar 2025 13:56:03 +0200 Subject: [PATCH 04/23] chore: Reverted test changes --- apps/automated/src/ui/tab-view/tab-view-root-tests.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/automated/src/ui/tab-view/tab-view-root-tests.ts b/apps/automated/src/ui/tab-view/tab-view-root-tests.ts index 8b91bd7e80..8d5129afcd 100644 --- a/apps/automated/src/ui/tab-view/tab-view-root-tests.ts +++ b/apps/automated/src/ui/tab-view/tab-view-root-tests.ts @@ -133,7 +133,7 @@ export function test_offset_zero_should_raise_same_events() { waitUntilNavigatedToMaxTimeout([items[0].page], () => Application.resetRootView(entry)); - const expectedEventsRaisedAfterTabCreated = [['Tab0 Frame0 Page0 navigatingTo', 'Tab0 Frame0 loaded', 'Tab0 Frame0 Page0 loaded', 'Tab0 Frame0 Page0 navigatedTo'], [], []]; + const expectedEventsRaisedAfterTabCreated = [['Tab0 Frame0 loaded', 'Tab0 Frame0 Page0 navigatingTo', 'Tab0 Frame0 Page0 loaded', 'Tab0 Frame0 Page0 navigatedTo'], [], []]; TKUnit.assertDeepEqual(actualEventsRaised, expectedEventsRaisedAfterTabCreated); @@ -199,14 +199,14 @@ export function test_android_default_offset_should_preload_1_tab_on_each_side() waitUntilNavigatedToMaxTimeout([items[0].page, items[1].page], () => Application.resetRootView(entry)); - const expectedEventsRaisedAfterTabCreated = [['Tab0 Frame0 Page0 navigatingTo', 'Tab0 Frame0 loaded', 'Tab0 Frame0 Page0 loaded', 'Tab0 Frame0 Page0 navigatedTo'], ['Tab1 Frame1 Page1 navigatingTo', 'Tab1 Frame1 loaded', 'Tab1 Frame1 Page1 loaded', 'Tab1 Frame1 Page1 navigatedTo'], []]; + const expectedEventsRaisedAfterTabCreated = [['Tab0 Frame0 loaded', 'Tab0 Frame0 Page0 navigatingTo', 'Tab0 Frame0 Page0 loaded', 'Tab0 Frame0 Page0 navigatedTo'], ['Tab1 Frame1 loaded', 'Tab1 Frame1 Page1 navigatingTo', 'Tab1 Frame1 Page1 loaded', 'Tab1 Frame1 Page1 navigatedTo'], []]; TKUnit.assertDeepEqual(actualEventsRaised, expectedEventsRaisedAfterTabCreated); resetActualEventsRaised(); waitUntilNavigatedToMaxTimeout([items[2].page], () => (tabView.selectedIndex = 2)); - const expectedEventsRaisedAfterSelectThirdTab = [['Tab0 Frame0 Page0 unloaded', 'Tab0 Frame0 unloaded'], [], ['Tab2 Frame2 Page2 navigatingTo', 'Tab2 Frame2 loaded', 'Tab2 Frame2 Page2 loaded', 'Tab2 Frame2 Page2 navigatedTo']]; + const expectedEventsRaisedAfterSelectThirdTab = [['Tab0 Frame0 Page0 unloaded', 'Tab0 Frame0 unloaded'], [], ['Tab2 Frame2 loaded', 'Tab2 Frame2 Page2 navigatingTo', 'Tab2 Frame2 Page2 loaded', 'Tab2 Frame2 Page2 navigatedTo']]; TKUnit.assertDeepEqual(actualEventsRaised, expectedEventsRaisedAfterSelectThirdTab); From 8052bd860d86c8031c55dfaa6a537ac1d4629fc7 Mon Sep 17 00:00:00 2001 From: Dimitris - Rafail Katsampas Date: Mon, 3 Mar 2025 14:20:52 +0200 Subject: [PATCH 05/23] ref: Use event listeners to process nested frame entries in the correct order --- packages/core/ui/core/view-base/index.ts | 23 +++++++++++++--------- packages/core/ui/frame/frame-common.ts | 25 ++++++++++++++++++++++++ packages/core/ui/frame/index.android.ts | 9 +++------ packages/core/ui/frame/index.ios.ts | 10 +--------- 4 files changed, 43 insertions(+), 24 deletions(-) diff --git a/packages/core/ui/core/view-base/index.ts b/packages/core/ui/core/view-base/index.ts index 46215bbacb..58a53d41ef 100644 --- a/packages/core/ui/core/view-base/index.ts +++ b/packages/core/ui/core/view-base/index.ts @@ -345,6 +345,7 @@ export abstract class ViewBase extends Observable implements ViewBaseDefinition private _androidView: Object; private _style: Style; private _isLoaded: boolean; + private _isLoadingSubviews: boolean; /** * @deprecated @@ -637,6 +638,10 @@ export abstract class ViewBase extends Observable implements ViewBaseDefinition return this._isLoaded; } + get isLoadingSubviews(): boolean { + return this._isLoadingSubviews; + } + get ['class'](): string { return this.className; } @@ -686,14 +691,6 @@ export abstract class ViewBase extends Observable implements ViewBaseDefinition this[name] = WrappedValue.unwrap(value); } - public loadSubviews(): void { - this.eachChild((child) => { - this.loadView(child); - - return true; - }); - } - @profile public onLoaded() { this.setFlag(Flags.superOnLoadedCalled, true); @@ -705,7 +702,15 @@ export abstract class ViewBase extends Observable implements ViewBaseDefinition this._cssState.onLoaded(); this._resumeNativeUpdates(SuspendType.Loaded); - this.loadSubviews(); + this._isLoadingSubviews = true; + + this.eachChild((child) => { + this.loadView(child); + + return true; + }); + + this._isLoadingSubviews = false; this._emit('loaded'); } diff --git a/packages/core/ui/frame/frame-common.ts b/packages/core/ui/frame/frame-common.ts index 78f92fcbe4..f298c59dbe 100644 --- a/packages/core/ui/frame/frame-common.ts +++ b/packages/core/ui/frame/frame-common.ts @@ -119,6 +119,31 @@ export class FrameBase extends CustomLayoutView { throw new Error(`Frame should not have a view. Use 'defaultPage' property instead.`); } + @profile + public onLoaded() { + const parentFrame = this.page?.frame; + let pendingFrame: FrameBase; + + // This frame is a nested frame as it resides inside the Page view of another frame + if (parentFrame && parentFrame.isLoadingSubviews) { + pendingFrame = parentFrame; + } else { + pendingFrame = this; + } + + // Process the entry of a nested frame once its parent has been loaded + // or wait for it to be loaded in case it's not nested inside another frame + pendingFrame.once(FrameBase.loadedEvent, () => { + this.onFrameLoaded(); + }); + + super.onLoaded(); + } + + public onFrameLoaded(): void { + this._processNextNavigationEntry(); + } + public canGoBack(): boolean { let backstack = this._backStack.length; let previousForwardNotInBackstack = false; diff --git a/packages/core/ui/frame/index.android.ts b/packages/core/ui/frame/index.android.ts index cd55537736..f620e3c89d 100644 --- a/packages/core/ui/frame/index.android.ts +++ b/packages/core/ui/frame/index.android.ts @@ -245,10 +245,8 @@ export class Frame extends FrameBase { super.onLoaded(); } - public override loadSubviews(): void { - // Process navigation entry after loading view but before loading children views, otherwise the navigation entries of nested frames will be processed first - // and needed fragments won't have been attached yet - this._processNextNavigationEntry(); + public onFrameLoaded(): void { + super.onFrameLoaded(); // TODO: Check if this is still needed since there have been new improvements regarding fragment restoration // there's a bug with nested frames where sometimes the nested fragment is not recreated at all @@ -266,12 +264,11 @@ export class Frame extends FrameBase { this._frameCreateTimeout = null; }, 0); - - super.loadSubviews(); } onUnloaded() { super.onUnloaded(); + if (typeof this._frameCreateTimeout === 'number') { clearTimeout(this._frameCreateTimeout); this._frameCreateTimeout = null; diff --git a/packages/core/ui/frame/index.ios.ts b/packages/core/ui/frame/index.ios.ts index 623da3b092..9591700648 100644 --- a/packages/core/ui/frame/index.ios.ts +++ b/packages/core/ui/frame/index.ios.ts @@ -75,14 +75,6 @@ export class Frame extends FrameBase { return this._ios; } - public override loadSubviews(): void { - // Process navigation entry after loading view but before loading children views, otherwise the navigation entries of nested frames will be processed first - // and needed fragments won't have been attached yet - this._processNextNavigationEntry(); - - super.loadSubviews(); - } - public setCurrent(entry: BackstackEntry, navigationType: NavigationType): void { const current = this._currentEntry; const currentEntryChanged = current !== entry; @@ -525,7 +517,7 @@ class UINavigationControllerImpl extends UINavigationController { } } - private animateWithDuration(navigationTransition: NavigationTransition, nativeTransition: UIViewAnimationTransition, transitionType: string, baseCallback: Function): void { + private animateWithDuration(navigationTransition: NavigationTransition, nativeTransition: UIViewAnimationTransition, transitionType: string, baseCallback: () => void): void { const duration = navigationTransition.duration ? navigationTransition.duration / 1000 : CORE_ANIMATION_DEFAULTS.duration; const curve = _getNativeCurve(navigationTransition); From f557233cc2d16ee2fea0795ddb68da8ac1db1bff Mon Sep 17 00:00:00 2001 From: Dimitris - Rafail Katsampas Date: Mon, 3 Mar 2025 16:03:46 +0200 Subject: [PATCH 06/23] chore: Making onLoaded less busy --- packages/core/ui/frame/frame-common.ts | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/packages/core/ui/frame/frame-common.ts b/packages/core/ui/frame/frame-common.ts index f298c59dbe..a8b9f18347 100644 --- a/packages/core/ui/frame/frame-common.ts +++ b/packages/core/ui/frame/frame-common.ts @@ -122,17 +122,9 @@ export class FrameBase extends CustomLayoutView { @profile public onLoaded() { const parentFrame = this.page?.frame; - let pendingFrame: FrameBase; + // Pending frame can be the first frame in the view tree or a nested frame + const pendingFrame = parentFrame && parentFrame.isLoadingSubviews ? parentFrame : this; - // This frame is a nested frame as it resides inside the Page view of another frame - if (parentFrame && parentFrame.isLoadingSubviews) { - pendingFrame = parentFrame; - } else { - pendingFrame = this; - } - - // Process the entry of a nested frame once its parent has been loaded - // or wait for it to be loaded in case it's not nested inside another frame pendingFrame.once(FrameBase.loadedEvent, () => { this.onFrameLoaded(); }); From 0d1b8d9e13752e893251a07e16fc93806492f2ec Mon Sep 17 00:00:00 2001 From: Dimitris - Rafail Katsampas Date: Sun, 16 Mar 2025 20:30:57 +0200 Subject: [PATCH 07/23] ref: Use frameEntryLoaded event to properly process entry --- packages/core/ui/frame/frame-common.ts | 18 ++++++++++++------ packages/core/ui/frame/index.android.ts | 7 +++++-- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/packages/core/ui/frame/frame-common.ts b/packages/core/ui/frame/frame-common.ts index a8b9f18347..d3ed4bd5bc 100644 --- a/packages/core/ui/frame/frame-common.ts +++ b/packages/core/ui/frame/frame-common.ts @@ -122,18 +122,24 @@ export class FrameBase extends CustomLayoutView { @profile public onLoaded() { const parentFrame = this.page?.frame; - // Pending frame can be the first frame in the view tree or a nested frame - const pendingFrame = parentFrame && parentFrame.isLoadingSubviews ? parentFrame : this; - - pendingFrame.once(FrameBase.loadedEvent, () => { - this.onFrameLoaded(); - }); super.onLoaded(); + + if (parentFrame && parentFrame.isLoadingSubviews) { + parentFrame.once('frameEntryLoaded', () => { + this.onFrameLoaded(); + }); + } else { + this.onFrameLoaded(); + } } public onFrameLoaded(): void { this._processNextNavigationEntry(); + this.notify({ + eventName: 'frameEntryLoaded', + object: this, + }); } public canGoBack(): boolean { diff --git a/packages/core/ui/frame/index.android.ts b/packages/core/ui/frame/index.android.ts index f620e3c89d..9b3c85e6ed 100644 --- a/packages/core/ui/frame/index.android.ts +++ b/packages/core/ui/frame/index.android.ts @@ -628,9 +628,12 @@ function cloneExpandedTransitionListener(expandedTransitionListener: any) { function getTransitionState(entry: BackstackEntry): TransitionState { const expandedEntry = entry; - const transitionState = {}; + + let transitionState: TransitionState; if (expandedEntry.enterTransitionListener && expandedEntry.exitTransitionListener) { + transitionState = {}; + transitionState.enterTransitionListener = cloneExpandedTransitionListener(expandedEntry.enterTransitionListener); transitionState.exitTransitionListener = cloneExpandedTransitionListener(expandedEntry.exitTransitionListener); transitionState.reenterTransitionListener = cloneExpandedTransitionListener(expandedEntry.reenterTransitionListener); @@ -638,7 +641,7 @@ function getTransitionState(entry: BackstackEntry): TransitionState { transitionState.transitionName = expandedEntry.transitionName; transitionState.entry = entry; } else { - return null; + transitionState = null; } return transitionState; From 67eb657f8ec0b8caa70f4beb22d90868ffb55f02 Mon Sep 17 00:00:00 2001 From: Dimitris - Rafail Katsampas Date: Tue, 1 Apr 2025 12:32:30 +0300 Subject: [PATCH 08/23] test: Added automated test --- .../src/ui/frame/frame-tests-common.ts | 49 +++++++++++++++++++ packages/core/ui/frame/index.android.ts | 2 +- 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/apps/automated/src/ui/frame/frame-tests-common.ts b/apps/automated/src/ui/frame/frame-tests-common.ts index d078dcb5a2..4f6292cc2f 100644 --- a/apps/automated/src/ui/frame/frame-tests-common.ts +++ b/apps/automated/src/ui/frame/frame-tests-common.ts @@ -5,6 +5,7 @@ import { Frame, NavigationEntry } from '@nativescript/core/ui/frame'; import { Label } from '@nativescript/core/ui/label'; import { Page } from '@nativescript/core/ui/page'; import * as TKUnit from '../../tk-unit'; +import { StackLayout } from '@nativescript/core'; const NAV_WAIT = 15; function emptyNavigationQueue(frame: Frame) { @@ -267,3 +268,51 @@ export function test_frame_retrieval_API_when_navigating() { initialFrame._removeFromFrameStack(); newFrame._removeFromFrameStack(); } + +export function test_frame_entry_loaded_order() { + const loadedFrames: Frame[] = []; + const rootFrame = Frame.topmost(); + + const rootPage = new Page(); + const rootPageContent = new StackLayout(); + + const nestedFrame = new Frame(); + nestedFrame.navigate(() => new Page()); + + rootPageContent.addChild(nestedFrame); + rootPage.content = rootPageContent; + + rootFrame.navigate(() => rootPage); + + emptyNavigationQueue(rootFrame); + + rootFrame.callUnloaded(); + + const onRootFrameLoaded = rootFrame.onFrameLoaded; + const onNestedFrameLoaded = nestedFrame.onFrameLoaded; + + rootFrame.onFrameLoaded = function () { + loadedFrames.push(this); + onRootFrameLoaded.call(this); + }; + + nestedFrame.onFrameLoaded = function () { + loadedFrames.push(this); + onNestedFrameLoaded.call(this); + }; + + rootFrame.callLoaded(); + + TKUnit.assertEqual(rootFrame, loadedFrames[0]); + TKUnit.assertEqual(nestedFrame, loadedFrames[1]); + + rootFrame.goBack(); + + emptyNavigationQueue(rootFrame); + + rootFrame.onFrameLoaded = onRootFrameLoaded; + nestedFrame.onFrameLoaded = onNestedFrameLoaded; + + // clean up the frame stack + nestedFrame._removeFromFrameStack(); +} diff --git a/packages/core/ui/frame/index.android.ts b/packages/core/ui/frame/index.android.ts index 9b3c85e6ed..6604e6e708 100644 --- a/packages/core/ui/frame/index.android.ts +++ b/packages/core/ui/frame/index.android.ts @@ -556,7 +556,7 @@ export class Frame extends FrameBase { this._disposeBackstackEntry(current); } - // There are cases transition state is still cached even during disposal as setCurrent may not necessarily be called to clean it up + // Dispose cached transition and store it again if view ever gets re-used this._cachedTransitionState = null; this._android.rootViewGroup = null; From af0bb62c2cd15b412dbdb492b0e1611b6a089e3d Mon Sep 17 00:00:00 2001 From: Dimitris - Rafail Katsampas Date: Sun, 11 May 2025 22:06:25 +0300 Subject: [PATCH 09/23] test: Corrected automated test for android --- apps/automated/src/ui/frame/frame-tests-common.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/apps/automated/src/ui/frame/frame-tests-common.ts b/apps/automated/src/ui/frame/frame-tests-common.ts index 4f6292cc2f..87688a6f01 100644 --- a/apps/automated/src/ui/frame/frame-tests-common.ts +++ b/apps/automated/src/ui/frame/frame-tests-common.ts @@ -306,6 +306,8 @@ export function test_frame_entry_loaded_order() { TKUnit.assertEqual(rootFrame, loadedFrames[0]); TKUnit.assertEqual(nestedFrame, loadedFrames[1]); + // Remove nested frame before going back as it triggers problems during onResume + rootPageContent.removeChild(nestedFrame); rootFrame.goBack(); emptyNavigationQueue(rootFrame); From 132527f5410aaa97e8379e26da9e8dfe13f61e67 Mon Sep 17 00:00:00 2001 From: Dimitris - Rafail Katsampas Date: Tue, 20 May 2025 22:21:01 +0300 Subject: [PATCH 10/23] fix: dispose native transitions during view disposal --- .../ui/frame/fragment.transitions.android.ts | 72 +++++++++++++++++-- .../core/ui/frame/fragment.transitions.d.ts | 17 ++++- packages/core/ui/frame/index.android.ts | 62 ++-------------- packages/core/ui/frame/index.ios.ts | 2 +- 4 files changed, 91 insertions(+), 62 deletions(-) diff --git a/packages/core/ui/frame/fragment.transitions.android.ts b/packages/core/ui/frame/fragment.transitions.android.ts index 70ce2aeb9f..b691ebf554 100644 --- a/packages/core/ui/frame/fragment.transitions.android.ts +++ b/packages/core/ui/frame/fragment.transitions.android.ts @@ -1,5 +1,5 @@ // Definitions. -import { NavigationType } from './frame-common'; +import { NavigationType, TransitionState } from './frame-common'; import { NavigationTransition, BackstackEntry } from '.'; // Types. @@ -428,6 +428,16 @@ function addToWaitingQueue(entry: ExpandedEntry): void { entries.add(entry); } +function cloneExpandedTransitionListener(expandedTransitionListener: ExpandedTransitionListener) { + if (!expandedTransitionListener) { + return null; + } + + const cloneTransition = expandedTransitionListener.transition.clone(); + + return addNativeTransitionListener(expandedTransitionListener.entry, cloneTransition); +} + function clearExitAndReenterTransitions(entry: ExpandedEntry, removeListener: boolean): void { const fragment: androidx.fragment.app.Fragment = entry.fragment; const exitListener = entry.exitTransitionListener; @@ -469,15 +479,69 @@ function clearExitAndReenterTransitions(entry: ExpandedEntry, removeListener: bo } } +export function _getTransitionState(entry: ExpandedEntry): TransitionState { + let transitionState: TransitionState; + + if (entry.enterTransitionListener && entry.exitTransitionListener) { + transitionState = { + enterTransitionListener: cloneExpandedTransitionListener(entry.enterTransitionListener), + exitTransitionListener: cloneExpandedTransitionListener(entry.exitTransitionListener), + reenterTransitionListener: cloneExpandedTransitionListener(entry.reenterTransitionListener), + returnTransitionListener: cloneExpandedTransitionListener(entry.returnTransitionListener), + transitionName: entry.transitionName, + entry, + }; + } else { + transitionState = null; + } + + return transitionState; +} + +export function _restoreTransitionState(entry: ExpandedEntry, snapshot: TransitionState): void { + if (snapshot.enterTransitionListener) { + entry.enterTransitionListener = snapshot.enterTransitionListener; + } + + if (snapshot.exitTransitionListener) { + entry.exitTransitionListener = snapshot.exitTransitionListener; + } + + if (snapshot.reenterTransitionListener) { + entry.reenterTransitionListener = snapshot.reenterTransitionListener; + } + + if (snapshot.returnTransitionListener) { + entry.returnTransitionListener = snapshot.returnTransitionListener; + } + + entry.transitionName = snapshot.transitionName; +} + +export function _unsetTransitionProperties(entry: ExpandedEntry): void { + entry.enterTransitionListener = null; + entry.exitTransitionListener = null; + entry.reenterTransitionListener = null; + entry.returnTransitionListener = null; + entry.enterAnimator = null; + entry.exitAnimator = null; + entry.popEnterAnimator = null; + entry.popExitAnimator = null; + entry.transition = null; + entry.transitionName = null; + entry.isNestedDefaultTransition = false; + entry.isAnimationRunning = false; +} + export function _clearFragment(entry: ExpandedEntry): void { - clearEntry(entry, false); + clearTransitions(entry, false); } export function _clearEntry(entry: ExpandedEntry): void { - clearEntry(entry, true); + clearTransitions(entry, true); } -function clearEntry(entry: ExpandedEntry, removeListener: boolean): void { +function clearTransitions(entry: ExpandedEntry, removeListener: boolean): void { clearExitAndReenterTransitions(entry, removeListener); const fragment: androidx.fragment.app.Fragment = entry.fragment; diff --git a/packages/core/ui/frame/fragment.transitions.d.ts b/packages/core/ui/frame/fragment.transitions.d.ts index b9f3494947..ef2d086f29 100644 --- a/packages/core/ui/frame/fragment.transitions.d.ts +++ b/packages/core/ui/frame/fragment.transitions.d.ts @@ -1,4 +1,4 @@ -import { NavigationTransition, BackstackEntry } from '.'; +import { NavigationTransition, BackstackEntry, TransitionState } from '.'; /** * @private @@ -20,6 +20,21 @@ export function _updateTransitions(entry: BackstackEntry): void; * Reverse transitions from entry to fragment if any. */ export function _reverseTransitions(previousEntry: BackstackEntry, currentEntry: BackstackEntry): boolean; +/** + * @private + */ +export function _getTransitionState(entry: BackstackEntry): TransitionState; +/** + * @private + */ +export function _restoreTransitionState(entry: BackstackEntry, snapshot: TransitionState): void; +/** + * @private + * Called when entry is removed from backstack (either back navigation or + * navigate with clear history). Removes all animations and transitions from entry + * and clears all listeners in order to prevent memory leaks. + */ +export function _unsetTransitionProperties(entry: BackstackEntry): void; /** * @private * Called when entry is removed from backstack (either back navigation or diff --git a/packages/core/ui/frame/index.android.ts b/packages/core/ui/frame/index.android.ts index 6604e6e708..180300682a 100644 --- a/packages/core/ui/frame/index.android.ts +++ b/packages/core/ui/frame/index.android.ts @@ -11,7 +11,7 @@ import { Trace } from '../../trace'; import { View } from '../core/view'; import { _stack, FrameBase, NavigationType } from './frame-common'; -import { _clearEntry, _clearFragment, _getAnimatedEntries, _reverseTransitions, _setAndroidFragmentTransitions, _updateTransitions, addNativeTransitionListener } from './fragment.transitions'; +import { _clearEntry, _clearFragment, _getAnimatedEntries, _getTransitionState, _restoreTransitionState, _reverseTransitions, _setAndroidFragmentTransitions, _unsetTransitionProperties, _updateTransitions, addNativeTransitionListener } from './fragment.transitions'; import { profile } from '../../profiling'; import { android as androidUtils } from '../../utils/native-helper'; @@ -194,7 +194,7 @@ export class Frame extends FrameBase { // simulated navigation (NoTransition, zero duration animator) and thus the fragment immediately disappears; // the user only sees the animation of the entering fragment as per its specific enter animation settings. // NOTE: we are restoring the animation settings in Frame.setCurrent(...) as navigation completes asynchronously - const cachedTransitionState = getTransitionState(this._currentEntry); + const cachedTransitionState = _getTransitionState(this._currentEntry); if (cachedTransitionState) { this._cachedTransitionState = cachedTransitionState; @@ -359,7 +359,7 @@ export class Frame extends FrameBase { // restore cached animation settings if we just completed simulated first navigation (no animation) if (this._cachedTransitionState) { - restoreTransitionState(this._currentEntry, this._cachedTransitionState); + _restoreTransitionState(this._currentEntry, this._cachedTransitionState); this._cachedTransitionState = null; } @@ -398,7 +398,7 @@ export class Frame extends FrameBase { // HACK: This @profile decorator creates a circular dependency // HACK: because the function parameter type is evaluated with 'typeof' @profile - public _navigateCore(newEntry: any) { + public _navigateCore(newEntry: BackstackEntry) { // should be (newEntry: BackstackEntry) super._navigateCore(newEntry); @@ -495,6 +495,7 @@ export class Frame extends FrameBase { if (removed.fragment) { _clearEntry(removed); } + _unsetTransitionProperties(removed); removed.fragment = null; removed.viewSavedState = null; @@ -504,6 +505,7 @@ export class Frame extends FrameBase { if (entry.fragment) { _clearFragment(entry); } + _unsetTransitionProperties(entry); entry.recreated = false; entry.fragment = null; @@ -616,58 +618,6 @@ export function reloadPage(context?: ModuleContext): void { // attach on global, so it can be overwritten in NativeScript Angular global.__onLiveSyncCore = Frame.reloadPage; -function cloneExpandedTransitionListener(expandedTransitionListener: any) { - if (!expandedTransitionListener) { - return null; - } - - const cloneTransition = expandedTransitionListener.transition.clone(); - - return addNativeTransitionListener(expandedTransitionListener.entry, cloneTransition); -} - -function getTransitionState(entry: BackstackEntry): TransitionState { - const expandedEntry = entry; - - let transitionState: TransitionState; - - if (expandedEntry.enterTransitionListener && expandedEntry.exitTransitionListener) { - transitionState = {}; - - transitionState.enterTransitionListener = cloneExpandedTransitionListener(expandedEntry.enterTransitionListener); - transitionState.exitTransitionListener = cloneExpandedTransitionListener(expandedEntry.exitTransitionListener); - transitionState.reenterTransitionListener = cloneExpandedTransitionListener(expandedEntry.reenterTransitionListener); - transitionState.returnTransitionListener = cloneExpandedTransitionListener(expandedEntry.returnTransitionListener); - transitionState.transitionName = expandedEntry.transitionName; - transitionState.entry = entry; - } else { - transitionState = null; - } - - return transitionState; -} - -function restoreTransitionState(entry: BackstackEntry, snapshot: TransitionState): void { - const expandedEntry = entry; - if (snapshot.enterTransitionListener) { - expandedEntry.enterTransitionListener = snapshot.enterTransitionListener; - } - - if (snapshot.exitTransitionListener) { - expandedEntry.exitTransitionListener = snapshot.exitTransitionListener; - } - - if (snapshot.reenterTransitionListener) { - expandedEntry.reenterTransitionListener = snapshot.reenterTransitionListener; - } - - if (snapshot.returnTransitionListener) { - expandedEntry.returnTransitionListener = snapshot.returnTransitionListener; - } - - expandedEntry.transitionName = snapshot.transitionName; -} - let framesCounter = 0; const framesCache = new Array>(); diff --git a/packages/core/ui/frame/index.ios.ts b/packages/core/ui/frame/index.ios.ts index 9591700648..9eca248309 100644 --- a/packages/core/ui/frame/index.ios.ts +++ b/packages/core/ui/frame/index.ios.ts @@ -88,7 +88,7 @@ export class Frame extends FrameBase { // !!! THIS PROFILE DECORATOR CREATES A CIRCULAR DEPENDENCY // !!! BECAUSE THE PARAMETER TYPE IS EVALUATED WITH TYPEOF @profile - public _navigateCore(backstackEntry: any) { + public _navigateCore(backstackEntry: BackstackEntry) { super._navigateCore(backstackEntry); const viewController: UIViewController = backstackEntry.resolvedPage.ios; From 720d4deb1e8ad6dc580e017e890df7216800d5f8 Mon Sep 17 00:00:00 2001 From: Dimitris - Rafail Katsampas Date: Sun, 25 May 2025 23:10:45 +0300 Subject: [PATCH 11/23] chore: minor refactoring and deprecation warnings --- .../ui/frame/callbacks/activity-callbacks.ts | 2 -- .../ui/frame/fragment.transitions.android.ts | 23 +++++++++++-------- .../core/ui/frame/fragment.transitions.d.ts | 2 +- packages/core/ui/frame/index.android.ts | 14 +++++++---- 4 files changed, 25 insertions(+), 16 deletions(-) diff --git a/packages/core/ui/frame/callbacks/activity-callbacks.ts b/packages/core/ui/frame/callbacks/activity-callbacks.ts index bb2924290b..23bc6f41f6 100644 --- a/packages/core/ui/frame/callbacks/activity-callbacks.ts +++ b/packages/core/ui/frame/callbacks/activity-callbacks.ts @@ -5,8 +5,6 @@ import { AndroidActivityBackPressedEventData, AndroidActivityNewIntentEventData, import { Trace } from '../../../trace'; import { View } from '../../core/view'; -import { _clearEntry, _clearFragment, _getAnimatedEntries, _reverseTransitions, _setAndroidFragmentTransitions, _updateTransitions } from '../fragment.transitions'; - import { profile } from '../../../profiling'; import { isEmbedded, setEmbeddedView } from '../../embedding'; diff --git a/packages/core/ui/frame/fragment.transitions.android.ts b/packages/core/ui/frame/fragment.transitions.android.ts index b691ebf554..15a1d0c5b3 100644 --- a/packages/core/ui/frame/fragment.transitions.android.ts +++ b/packages/core/ui/frame/fragment.transitions.android.ts @@ -152,7 +152,7 @@ export function _setAndroidFragmentTransitions(animated: boolean, navigationTran setupCurrentFragmentExplodeTransition(navigationTransition, currentEntry); } } else if (name.indexOf('flip') === 0) { - const direction = name.substr('flip'.length) || 'right'; //Extract the direction from the string + const direction = name.substring('flip'.length) || 'right'; //Extract the direction from the string const flipTransition = new FlipTransition(direction, navigationTransition.duration, navigationTransition.curve); setupNewFragmentCustomTransition(navigationTransition, newEntry, flipTransition); @@ -282,23 +282,28 @@ export function _getAnimatedEntries(frameId: number): Set { export function _updateTransitions(entry: ExpandedEntry): void { const fragment = entry.fragment; + + if (!fragment) { + return; + } + const enterTransitionListener = entry.enterTransitionListener; - if (enterTransitionListener && fragment) { + if (enterTransitionListener) { fragment.setEnterTransition(enterTransitionListener.transition); } const exitTransitionListener = entry.exitTransitionListener; - if (exitTransitionListener && fragment) { + if (exitTransitionListener) { fragment.setExitTransition(exitTransitionListener.transition); } const reenterTransitionListener = entry.reenterTransitionListener; - if (reenterTransitionListener && fragment) { + if (reenterTransitionListener) { fragment.setReenterTransition(reenterTransitionListener.transition); } const returnTransitionListener = entry.returnTransitionListener; - if (returnTransitionListener && fragment) { + if (returnTransitionListener) { fragment.setReturnTransition(returnTransitionListener.transition); } } @@ -518,7 +523,7 @@ export function _restoreTransitionState(entry: ExpandedEntry, snapshot: Transiti entry.transitionName = snapshot.transitionName; } -export function _unsetTransitionProperties(entry: ExpandedEntry): void { +export function _disposeTransitionReferences(entry: ExpandedEntry): void { entry.enterTransitionListener = null; entry.exitTransitionListener = null; entry.reenterTransitionListener = null; @@ -528,7 +533,7 @@ export function _unsetTransitionProperties(entry: ExpandedEntry): void { entry.popEnterAnimator = null; entry.popExitAnimator = null; entry.transition = null; - entry.transitionName = null; + entry.transitionName = ''; entry.isNestedDefaultTransition = false; entry.isAnimationRunning = false; } @@ -633,7 +638,7 @@ function setReturnTransition(navigationTransition: NavigationTransition, entry: function setupNewFragmentSlideTransition(navTransition: NavigationTransition, entry: ExpandedEntry, name: string): void { setupCurrentFragmentSlideTransition(navTransition, entry, name); - const direction = name.substr('slide'.length) || 'left'; //Extract the direction from the string + const direction = name.substring('slide'.length) || 'left'; //Extract the direction from the string switch (direction) { case 'left': setEnterTransition(navTransition, entry, new androidx.transition.Slide(android.view.Gravity.RIGHT)); @@ -658,7 +663,7 @@ function setupNewFragmentSlideTransition(navTransition: NavigationTransition, en } function setupCurrentFragmentSlideTransition(navTransition: NavigationTransition, entry: ExpandedEntry, name: string): void { - const direction = name.substr('slide'.length) || 'left'; //Extract the direction from the string + const direction = name.substring('slide'.length) || 'left'; //Extract the direction from the string switch (direction) { case 'left': setExitTransition(navTransition, entry, new androidx.transition.Slide(android.view.Gravity.LEFT)); diff --git a/packages/core/ui/frame/fragment.transitions.d.ts b/packages/core/ui/frame/fragment.transitions.d.ts index ef2d086f29..69c4b833e1 100644 --- a/packages/core/ui/frame/fragment.transitions.d.ts +++ b/packages/core/ui/frame/fragment.transitions.d.ts @@ -34,7 +34,7 @@ export function _restoreTransitionState(entry: BackstackEntry, snapshot: Transit * navigate with clear history). Removes all animations and transitions from entry * and clears all listeners in order to prevent memory leaks. */ -export function _unsetTransitionProperties(entry: BackstackEntry): void; +export function _disposeTransitionReferences(entry: BackstackEntry): void; /** * @private * Called when entry is removed from backstack (either back navigation or diff --git a/packages/core/ui/frame/index.android.ts b/packages/core/ui/frame/index.android.ts index 180300682a..726cfd7b5c 100644 --- a/packages/core/ui/frame/index.android.ts +++ b/packages/core/ui/frame/index.android.ts @@ -11,7 +11,7 @@ import { Trace } from '../../trace'; import { View } from '../core/view'; import { _stack, FrameBase, NavigationType } from './frame-common'; -import { _clearEntry, _clearFragment, _getAnimatedEntries, _getTransitionState, _restoreTransitionState, _reverseTransitions, _setAndroidFragmentTransitions, _unsetTransitionProperties, _updateTransitions, addNativeTransitionListener } from './fragment.transitions'; +import { _clearEntry, _getAnimatedEntries, _getTransitionState, _restoreTransitionState, _reverseTransitions, _setAndroidFragmentTransitions, _disposeTransitionReferences, _updateTransitions, addNativeTransitionListener } from './fragment.transitions'; import { profile } from '../../profiling'; import { android as androidUtils } from '../../utils/native-helper'; @@ -492,20 +492,26 @@ export class Frame extends FrameBase { public _removeEntry(removed: BackstackEntry): void { super._removeEntry(removed); + // There is the case of this condition being false due to fragment callbacks onDestroy() call which unsets entry fragment. + // This results in entry keeping unwanted references so _unsetTransitionProperties comes into play and cleans everything up if (removed.fragment) { _clearEntry(removed); } - _unsetTransitionProperties(removed); + + _disposeTransitionReferences(removed); removed.fragment = null; removed.viewSavedState = null; } protected _disposeBackstackEntry(entry: BackstackEntry): void { + // There is the case of this condition being false due to fragment callbacks onDestroy() call which unsets entry fragment. + // This results in entry keeping unwanted references so _unsetTransitionProperties comes into play and cleans everything up if (entry.fragment) { - _clearFragment(entry); + _clearEntry(entry); } - _unsetTransitionProperties(entry); + + _disposeTransitionReferences(entry); entry.recreated = false; entry.fragment = null; From 1fcf2fba5a250f3a36b8109264f39586c39e925c Mon Sep 17 00:00:00 2001 From: Dimitris - Rafail Katsampas Date: Mon, 26 May 2025 02:11:06 +0300 Subject: [PATCH 12/23] fix: restore transitions for the correct entry --- packages/core/ui/frame/fragment.transitions.android.ts | 4 +++- packages/core/ui/frame/fragment.transitions.d.ts | 2 +- packages/core/ui/frame/index.android.ts | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/core/ui/frame/fragment.transitions.android.ts b/packages/core/ui/frame/fragment.transitions.android.ts index 15a1d0c5b3..8b73acd31e 100644 --- a/packages/core/ui/frame/fragment.transitions.android.ts +++ b/packages/core/ui/frame/fragment.transitions.android.ts @@ -503,7 +503,9 @@ export function _getTransitionState(entry: ExpandedEntry): TransitionState { return transitionState; } -export function _restoreTransitionState(entry: ExpandedEntry, snapshot: TransitionState): void { +export function _restoreTransitionState(snapshot: TransitionState): void { + const entry = snapshot.entry as ExpandedEntry; + if (snapshot.enterTransitionListener) { entry.enterTransitionListener = snapshot.enterTransitionListener; } diff --git a/packages/core/ui/frame/fragment.transitions.d.ts b/packages/core/ui/frame/fragment.transitions.d.ts index 69c4b833e1..19bfba451a 100644 --- a/packages/core/ui/frame/fragment.transitions.d.ts +++ b/packages/core/ui/frame/fragment.transitions.d.ts @@ -27,7 +27,7 @@ export function _getTransitionState(entry: BackstackEntry): TransitionState; /** * @private */ -export function _restoreTransitionState(entry: BackstackEntry, snapshot: TransitionState): void; +export function _restoreTransitionState(snapshot: TransitionState): void; /** * @private * Called when entry is removed from backstack (either back navigation or diff --git a/packages/core/ui/frame/index.android.ts b/packages/core/ui/frame/index.android.ts index 726cfd7b5c..03d852658c 100644 --- a/packages/core/ui/frame/index.android.ts +++ b/packages/core/ui/frame/index.android.ts @@ -359,7 +359,7 @@ export class Frame extends FrameBase { // restore cached animation settings if we just completed simulated first navigation (no animation) if (this._cachedTransitionState) { - _restoreTransitionState(this._currentEntry, this._cachedTransitionState); + _restoreTransitionState(this._cachedTransitionState); this._cachedTransitionState = null; } From dcee835bc0ea7852d68ce5e558888d16386ac9ce Mon Sep 17 00:00:00 2001 From: Dimitris - Rafail Katsampas Date: Mon, 26 May 2025 03:07:40 +0300 Subject: [PATCH 13/23] fix: added nested frames old fix to the right place --- .../ui/frame/fragment.transitions.android.ts | 15 -------- .../core/ui/frame/fragment.transitions.d.ts | 7 ---- packages/core/ui/frame/frame-common.ts | 12 ++++--- packages/core/ui/frame/index.android.ts | 34 ++++++++----------- 4 files changed, 22 insertions(+), 46 deletions(-) diff --git a/packages/core/ui/frame/fragment.transitions.android.ts b/packages/core/ui/frame/fragment.transitions.android.ts index 8b73acd31e..c8c897b164 100644 --- a/packages/core/ui/frame/fragment.transitions.android.ts +++ b/packages/core/ui/frame/fragment.transitions.android.ts @@ -525,21 +525,6 @@ export function _restoreTransitionState(snapshot: TransitionState): void { entry.transitionName = snapshot.transitionName; } -export function _disposeTransitionReferences(entry: ExpandedEntry): void { - entry.enterTransitionListener = null; - entry.exitTransitionListener = null; - entry.reenterTransitionListener = null; - entry.returnTransitionListener = null; - entry.enterAnimator = null; - entry.exitAnimator = null; - entry.popEnterAnimator = null; - entry.popExitAnimator = null; - entry.transition = null; - entry.transitionName = ''; - entry.isNestedDefaultTransition = false; - entry.isAnimationRunning = false; -} - export function _clearFragment(entry: ExpandedEntry): void { clearTransitions(entry, false); } diff --git a/packages/core/ui/frame/fragment.transitions.d.ts b/packages/core/ui/frame/fragment.transitions.d.ts index 19bfba451a..8f145f9a99 100644 --- a/packages/core/ui/frame/fragment.transitions.d.ts +++ b/packages/core/ui/frame/fragment.transitions.d.ts @@ -28,13 +28,6 @@ export function _getTransitionState(entry: BackstackEntry): TransitionState; * @private */ export function _restoreTransitionState(snapshot: TransitionState): void; -/** - * @private - * Called when entry is removed from backstack (either back navigation or - * navigate with clear history). Removes all animations and transitions from entry - * and clears all listeners in order to prevent memory leaks. - */ -export function _disposeTransitionReferences(entry: BackstackEntry): void; /** * @private * Called when entry is removed from backstack (either back navigation or diff --git a/packages/core/ui/frame/frame-common.ts b/packages/core/ui/frame/frame-common.ts index d3ed4bd5bc..482a767c22 100644 --- a/packages/core/ui/frame/frame-common.ts +++ b/packages/core/ui/frame/frame-common.ts @@ -136,10 +136,7 @@ export class FrameBase extends CustomLayoutView { public onFrameLoaded(): void { this._processNextNavigationEntry(); - this.notify({ - eventName: 'frameEntryLoaded', - object: this, - }); + this._notifyFrameEntryLoaded(); } public canGoBack(): boolean { @@ -338,6 +335,13 @@ export class FrameBase extends CustomLayoutView { } } + protected _notifyFrameEntryLoaded(): void { + this.notify({ + eventName: 'frameEntryLoaded', + object: this, + }); + } + private isNestedWithin(parentFrameCandidate: FrameBase): boolean { let frameAncestor: FrameBase = this; while (frameAncestor) { diff --git a/packages/core/ui/frame/index.android.ts b/packages/core/ui/frame/index.android.ts index 03d852658c..b559205c86 100644 --- a/packages/core/ui/frame/index.android.ts +++ b/packages/core/ui/frame/index.android.ts @@ -11,7 +11,7 @@ import { Trace } from '../../trace'; import { View } from '../core/view'; import { _stack, FrameBase, NavigationType } from './frame-common'; -import { _clearEntry, _getAnimatedEntries, _getTransitionState, _restoreTransitionState, _reverseTransitions, _setAndroidFragmentTransitions, _disposeTransitionReferences, _updateTransitions, addNativeTransitionListener } from './fragment.transitions'; +import { _clearEntry, _clearFragment, _getAnimatedEntries, _getTransitionState, _restoreTransitionState, _reverseTransitions, _setAndroidFragmentTransitions, _updateTransitions, addNativeTransitionListener } from './fragment.transitions'; import { profile } from '../../profiling'; import { android as androidUtils } from '../../utils/native-helper'; @@ -245,25 +245,27 @@ export class Frame extends FrameBase { super.onLoaded(); } - public onFrameLoaded(): void { - super.onFrameLoaded(); + protected override _notifyFrameEntryLoaded(): void { + const currentEntry = this._currentEntry || this._executingContext?.entry; - // TODO: Check if this is still needed since there have been new improvements regarding fragment restoration - // there's a bug with nested frames where sometimes the nested fragment is not recreated at all + // Note: This is kept as a precaution and must execute before emitting the frame entry event. + // There's a bug with nested frames where sometimes the nested fragment is not recreated at all // so we manually check on loaded event if the fragment is not recreated and recreate it - this._frameCreateTimeout = setTimeout(() => { - const currentEntry = this._currentEntry || this._executingContext?.entry; - if (currentEntry && !currentEntry.fragment) { + if (currentEntry && !currentEntry.fragment) { + this._frameCreateTimeout = setTimeout(() => { const manager = this._getFragmentManager(); const transaction = manager.beginTransaction(); + currentEntry.fragment = this.createFragment(currentEntry, currentEntry.fragmentTag); _updateTransitions(currentEntry); transaction.replace(this.containerViewId, currentEntry.fragment, currentEntry.fragmentTag); transaction.commitAllowingStateLoss(); - } - this._frameCreateTimeout = null; - }, 0); + this._frameCreateTimeout = null; + }, 0); + } + + super._notifyFrameEntryLoaded(); } onUnloaded() { @@ -492,27 +494,19 @@ export class Frame extends FrameBase { public _removeEntry(removed: BackstackEntry): void { super._removeEntry(removed); - // There is the case of this condition being false due to fragment callbacks onDestroy() call which unsets entry fragment. - // This results in entry keeping unwanted references so _unsetTransitionProperties comes into play and cleans everything up if (removed.fragment) { _clearEntry(removed); } - _disposeTransitionReferences(removed); - removed.fragment = null; removed.viewSavedState = null; } protected _disposeBackstackEntry(entry: BackstackEntry): void { - // There is the case of this condition being false due to fragment callbacks onDestroy() call which unsets entry fragment. - // This results in entry keeping unwanted references so _unsetTransitionProperties comes into play and cleans everything up if (entry.fragment) { - _clearEntry(entry); + _clearFragment(entry); } - _disposeTransitionReferences(entry); - entry.recreated = false; entry.fragment = null; From e5cf840b88fc4e1c2fb52ac3d2c9554bb30cbb53 Mon Sep 17 00:00:00 2001 From: Dimitris - Rafail Katsampas Date: Sat, 31 May 2025 14:10:07 +0300 Subject: [PATCH 14/23] chore: added optional chaining --- packages/core/ui/frame/frame-common.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/ui/frame/frame-common.ts b/packages/core/ui/frame/frame-common.ts index 482a767c22..2e649ad8c1 100644 --- a/packages/core/ui/frame/frame-common.ts +++ b/packages/core/ui/frame/frame-common.ts @@ -125,7 +125,7 @@ export class FrameBase extends CustomLayoutView { super.onLoaded(); - if (parentFrame && parentFrame.isLoadingSubviews) { + if (parentFrame?.isLoadingSubviews) { parentFrame.once('frameEntryLoaded', () => { this.onFrameLoaded(); }); From a25fc5b1ef014bcfd61fd0e8ce06b5015852a6a1 Mon Sep 17 00:00:00 2001 From: Dimitris - Rafail Katsampas Date: Sat, 31 May 2025 15:09:45 +0300 Subject: [PATCH 15/23] chore: added comment for clarification --- packages/core/ui/frame/frame-common.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/core/ui/frame/frame-common.ts b/packages/core/ui/frame/frame-common.ts index 2e649ad8c1..fa55868bb4 100644 --- a/packages/core/ui/frame/frame-common.ts +++ b/packages/core/ui/frame/frame-common.ts @@ -121,6 +121,7 @@ export class FrameBase extends CustomLayoutView { @profile public onLoaded() { + // Property page refers to the page this frame is nested into const parentFrame = this.page?.frame; super.onLoaded(); From d43d852d8e1415c88149add16b40538a99cc86f9 Mon Sep 17 00:00:00 2001 From: Dimitris - Rafail Katsampas Date: Sat, 31 May 2025 17:41:50 +0300 Subject: [PATCH 16/23] chore: added weakref precaution --- packages/core/ui/core/view-base/index.ts | 9 ++++--- packages/core/ui/frame/frame-common.ts | 32 ++++++++++++++++++------ 2 files changed, 30 insertions(+), 11 deletions(-) diff --git a/packages/core/ui/core/view-base/index.ts b/packages/core/ui/core/view-base/index.ts index 58a53d41ef..0a74d77b59 100644 --- a/packages/core/ui/core/view-base/index.ts +++ b/packages/core/ui/core/view-base/index.ts @@ -117,12 +117,13 @@ export interface ShowModalOptions { * @param criterion - The type of ancestor view we are looking for. Could be a string containing a class name or an actual type. * Returns an instance of a view (if found), otherwise undefined. */ -export function getAncestor(view: ViewBaseDefinition, criterion: string | { new () }): ViewBaseDefinition { - let matcher: (view: ViewBaseDefinition) => boolean = null; +export function getAncestor(view: T, criterion: string | { new () }): T { + let matcher: (view: ViewBaseDefinition) => view is T; + if (typeof criterion === 'string') { - matcher = (view: ViewBaseDefinition) => view.typeName === criterion; + matcher = (view: ViewBaseDefinition): view is T => view.typeName === criterion; } else { - matcher = (view: ViewBaseDefinition) => view instanceof criterion; + matcher = (view: ViewBaseDefinition): view is T => view instanceof criterion; } for (let parent = view.parent; parent != null; parent = parent.parent) { diff --git a/packages/core/ui/frame/frame-common.ts b/packages/core/ui/frame/frame-common.ts index fa55868bb4..c2e8125dca 100644 --- a/packages/core/ui/frame/frame-common.ts +++ b/packages/core/ui/frame/frame-common.ts @@ -17,6 +17,8 @@ import { NavigationData } from '.'; export { NavigationType } from './frame-interfaces'; export type { AndroidActivityCallbacks, AndroidFragmentCallbacks, AndroidFrame, BackstackEntry, NavigationContext, NavigationEntry, NavigationTransition, TransitionState, ViewEntry, iOSFrame } from './frame-interfaces'; +const FRAME_ENTRY_LOADED_EVENT = '_frameEntryLoaded'; + function buildEntryFromArgs(arg: any): NavigationEntry { let entry: NavigationEntry; if (typeof arg === 'string') { @@ -76,13 +78,13 @@ export class FrameBase extends CustomLayoutView { return true; } else if (top) { let parentFrameCanGoBack = false; - let parentFrame = getAncestor(top, 'Frame'); + let parentFrame = getAncestor(top, 'Frame'); while (parentFrame && !parentFrameCanGoBack) { if (parentFrame && parentFrame.canGoBack()) { parentFrameCanGoBack = true; } else { - parentFrame = getAncestor(parentFrame, 'Frame'); + parentFrame = getAncestor(parentFrame, 'Frame'); } } @@ -127,14 +129,29 @@ export class FrameBase extends CustomLayoutView { super.onLoaded(); if (parentFrame?.isLoadingSubviews) { - parentFrame.once('frameEntryLoaded', () => { - this.onFrameLoaded(); + const frameRef = new WeakRef(this); + + parentFrame.once(FRAME_ENTRY_LOADED_EVENT, () => { + const frame = frameRef.deref(); + if (frame) { + frame.onFrameLoaded(); + } }); } else { this.onFrameLoaded(); } } + @profile + public onUnloaded() { + super.onUnloaded(); + + // This is a precaution in case the method is called asynchronously during the loading procedure + if (this.hasListeners(FRAME_ENTRY_LOADED_EVENT)) { + this.off(FRAME_ENTRY_LOADED_EVENT); + } + } + public onFrameLoaded(): void { this._processNextNavigationEntry(); this._notifyFrameEntryLoaded(); @@ -338,15 +355,16 @@ export class FrameBase extends CustomLayoutView { protected _notifyFrameEntryLoaded(): void { this.notify({ - eventName: 'frameEntryLoaded', + eventName: FRAME_ENTRY_LOADED_EVENT, object: this, }); } private isNestedWithin(parentFrameCandidate: FrameBase): boolean { - let frameAncestor: FrameBase = this; + let frameAncestor = this as FrameBase; + while (frameAncestor) { - frameAncestor = getAncestor(frameAncestor, FrameBase); + frameAncestor = getAncestor(frameAncestor, FrameBase); if (frameAncestor === parentFrameCandidate) { return true; } From a0a74b333c6222ad3df81f5fb291bbf85f40b384 Mon Sep 17 00:00:00 2001 From: Dimitris - Rafail Katsampas Date: Sat, 31 May 2025 19:47:30 +0300 Subject: [PATCH 17/23] fix: corrected frame new unloaded handling --- packages/core/ui/frame/frame-common.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/core/ui/frame/frame-common.ts b/packages/core/ui/frame/frame-common.ts index c2e8125dca..dbd2406628 100644 --- a/packages/core/ui/frame/frame-common.ts +++ b/packages/core/ui/frame/frame-common.ts @@ -144,11 +144,14 @@ export class FrameBase extends CustomLayoutView { @profile public onUnloaded() { + // Property page refers to the page this frame is nested into + const parentFrame = this.page?.frame; + super.onUnloaded(); // This is a precaution in case the method is called asynchronously during the loading procedure - if (this.hasListeners(FRAME_ENTRY_LOADED_EVENT)) { - this.off(FRAME_ENTRY_LOADED_EVENT); + if (parentFrame && parentFrame.hasListeners(FRAME_ENTRY_LOADED_EVENT)) { + parentFrame.off(FRAME_ENTRY_LOADED_EVENT); } } From 8f406274113fb62ba3d46dc332bc66c39309fc74 Mon Sep 17 00:00:00 2001 From: Dimitris - Rafail Katsampas Date: Mon, 2 Jun 2025 11:38:12 +0300 Subject: [PATCH 18/23] fix: avoid removing needed frame entry listeners --- packages/core/ui/frame/frame-common.ts | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/packages/core/ui/frame/frame-common.ts b/packages/core/ui/frame/frame-common.ts index dbd2406628..23117b44ef 100644 --- a/packages/core/ui/frame/frame-common.ts +++ b/packages/core/ui/frame/frame-common.ts @@ -45,6 +45,7 @@ export class FrameBase extends CustomLayoutView { private _transition: NavigationTransition; private _backStack = new Array(); private _navigationQueue = new Array(); + private _frameEntryLoadedCallback: () => void; public actionBarVisibility: 'auto' | 'never' | 'always'; public _currentEntry: BackstackEntry; @@ -130,13 +131,17 @@ export class FrameBase extends CustomLayoutView { if (parentFrame?.isLoadingSubviews) { const frameRef = new WeakRef(this); - - parentFrame.once(FRAME_ENTRY_LOADED_EVENT, () => { + const callback = () => { const frame = frameRef.deref(); if (frame) { frame.onFrameLoaded(); } - }); + + this._frameEntryLoadedCallback = null; + }; + + parentFrame.once(FRAME_ENTRY_LOADED_EVENT, callback); + this._frameEntryLoadedCallback = callback; } else { this.onFrameLoaded(); } @@ -146,12 +151,14 @@ export class FrameBase extends CustomLayoutView { public onUnloaded() { // Property page refers to the page this frame is nested into const parentFrame = this.page?.frame; + const frameEntryLoadedCallback = this._frameEntryLoadedCallback; super.onUnloaded(); + this._frameEntryLoadedCallback = null; // This is a precaution in case the method is called asynchronously during the loading procedure - if (parentFrame && parentFrame.hasListeners(FRAME_ENTRY_LOADED_EVENT)) { - parentFrame.off(FRAME_ENTRY_LOADED_EVENT); + if (parentFrame && frameEntryLoadedCallback) { + parentFrame.off(FRAME_ENTRY_LOADED_EVENT, frameEntryLoadedCallback); } } From 360ba2d6644b35d4fcf1e93a9a2a591290e7bab9 Mon Sep 17 00:00:00 2001 From: Dimitris - Rafail Katsampas Date: Mon, 2 Jun 2025 11:40:45 +0300 Subject: [PATCH 19/23] chore: small refactor --- packages/core/ui/frame/frame-common.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/core/ui/frame/frame-common.ts b/packages/core/ui/frame/frame-common.ts index 23117b44ef..ceb5321af8 100644 --- a/packages/core/ui/frame/frame-common.ts +++ b/packages/core/ui/frame/frame-common.ts @@ -151,14 +151,18 @@ export class FrameBase extends CustomLayoutView { public onUnloaded() { // Property page refers to the page this frame is nested into const parentFrame = this.page?.frame; - const frameEntryLoadedCallback = this._frameEntryLoadedCallback; super.onUnloaded(); - this._frameEntryLoadedCallback = null; // This is a precaution in case the method is called asynchronously during the loading procedure - if (parentFrame && frameEntryLoadedCallback) { - parentFrame.off(FRAME_ENTRY_LOADED_EVENT, frameEntryLoadedCallback); + if (this._frameEntryLoadedCallback) { + const cb = this._frameEntryLoadedCallback; + + this._frameEntryLoadedCallback = null; + + if (parentFrame) { + parentFrame.off(FRAME_ENTRY_LOADED_EVENT, cb); + } } } From 417c6d694e47647558b139245c04c1bb45c2448b Mon Sep 17 00:00:00 2001 From: Dimitris - Rafail Katsampas Date: Mon, 2 Jun 2025 11:52:18 +0300 Subject: [PATCH 20/23] chore: set callback earlier --- packages/core/ui/frame/frame-common.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/ui/frame/frame-common.ts b/packages/core/ui/frame/frame-common.ts index ceb5321af8..c7c5c34394 100644 --- a/packages/core/ui/frame/frame-common.ts +++ b/packages/core/ui/frame/frame-common.ts @@ -140,8 +140,8 @@ export class FrameBase extends CustomLayoutView { this._frameEntryLoadedCallback = null; }; - parentFrame.once(FRAME_ENTRY_LOADED_EVENT, callback); this._frameEntryLoadedCallback = callback; + parentFrame.once(FRAME_ENTRY_LOADED_EVENT, callback); } else { this.onFrameLoaded(); } From 2c6403773a96f39fd36c9586392415dcc396bbb5 Mon Sep 17 00:00:00 2001 From: Dimitris - Rafail Katsampas Date: Sun, 29 Jun 2025 16:33:26 +0300 Subject: [PATCH 21/23] ref: Removed new frame entry init implementation --- .../src/ui/frame/frame-tests-common.ts | 51 ------------------ packages/core/ui/core/view-base/index.ts | 9 ---- packages/core/ui/frame/frame-common.ts | 53 ------------------- packages/core/ui/frame/index.android.ts | 39 ++++++-------- 4 files changed, 17 insertions(+), 135 deletions(-) diff --git a/apps/automated/src/ui/frame/frame-tests-common.ts b/apps/automated/src/ui/frame/frame-tests-common.ts index 87688a6f01..d078dcb5a2 100644 --- a/apps/automated/src/ui/frame/frame-tests-common.ts +++ b/apps/automated/src/ui/frame/frame-tests-common.ts @@ -5,7 +5,6 @@ import { Frame, NavigationEntry } from '@nativescript/core/ui/frame'; import { Label } from '@nativescript/core/ui/label'; import { Page } from '@nativescript/core/ui/page'; import * as TKUnit from '../../tk-unit'; -import { StackLayout } from '@nativescript/core'; const NAV_WAIT = 15; function emptyNavigationQueue(frame: Frame) { @@ -268,53 +267,3 @@ export function test_frame_retrieval_API_when_navigating() { initialFrame._removeFromFrameStack(); newFrame._removeFromFrameStack(); } - -export function test_frame_entry_loaded_order() { - const loadedFrames: Frame[] = []; - const rootFrame = Frame.topmost(); - - const rootPage = new Page(); - const rootPageContent = new StackLayout(); - - const nestedFrame = new Frame(); - nestedFrame.navigate(() => new Page()); - - rootPageContent.addChild(nestedFrame); - rootPage.content = rootPageContent; - - rootFrame.navigate(() => rootPage); - - emptyNavigationQueue(rootFrame); - - rootFrame.callUnloaded(); - - const onRootFrameLoaded = rootFrame.onFrameLoaded; - const onNestedFrameLoaded = nestedFrame.onFrameLoaded; - - rootFrame.onFrameLoaded = function () { - loadedFrames.push(this); - onRootFrameLoaded.call(this); - }; - - nestedFrame.onFrameLoaded = function () { - loadedFrames.push(this); - onNestedFrameLoaded.call(this); - }; - - rootFrame.callLoaded(); - - TKUnit.assertEqual(rootFrame, loadedFrames[0]); - TKUnit.assertEqual(nestedFrame, loadedFrames[1]); - - // Remove nested frame before going back as it triggers problems during onResume - rootPageContent.removeChild(nestedFrame); - rootFrame.goBack(); - - emptyNavigationQueue(rootFrame); - - rootFrame.onFrameLoaded = onRootFrameLoaded; - nestedFrame.onFrameLoaded = onNestedFrameLoaded; - - // clean up the frame stack - nestedFrame._removeFromFrameStack(); -} diff --git a/packages/core/ui/core/view-base/index.ts b/packages/core/ui/core/view-base/index.ts index 0a74d77b59..44c345b4b4 100644 --- a/packages/core/ui/core/view-base/index.ts +++ b/packages/core/ui/core/view-base/index.ts @@ -346,7 +346,6 @@ export abstract class ViewBase extends Observable implements ViewBaseDefinition private _androidView: Object; private _style: Style; private _isLoaded: boolean; - private _isLoadingSubviews: boolean; /** * @deprecated @@ -639,10 +638,6 @@ export abstract class ViewBase extends Observable implements ViewBaseDefinition return this._isLoaded; } - get isLoadingSubviews(): boolean { - return this._isLoadingSubviews; - } - get ['class'](): string { return this.className; } @@ -703,16 +698,12 @@ export abstract class ViewBase extends Observable implements ViewBaseDefinition this._cssState.onLoaded(); this._resumeNativeUpdates(SuspendType.Loaded); - this._isLoadingSubviews = true; - this.eachChild((child) => { this.loadView(child); return true; }); - this._isLoadingSubviews = false; - this._emit('loaded'); } diff --git a/packages/core/ui/frame/frame-common.ts b/packages/core/ui/frame/frame-common.ts index c7c5c34394..e67b61c452 100644 --- a/packages/core/ui/frame/frame-common.ts +++ b/packages/core/ui/frame/frame-common.ts @@ -17,8 +17,6 @@ import { NavigationData } from '.'; export { NavigationType } from './frame-interfaces'; export type { AndroidActivityCallbacks, AndroidFragmentCallbacks, AndroidFrame, BackstackEntry, NavigationContext, NavigationEntry, NavigationTransition, TransitionState, ViewEntry, iOSFrame } from './frame-interfaces'; -const FRAME_ENTRY_LOADED_EVENT = '_frameEntryLoaded'; - function buildEntryFromArgs(arg: any): NavigationEntry { let entry: NavigationEntry; if (typeof arg === 'string') { @@ -45,7 +43,6 @@ export class FrameBase extends CustomLayoutView { private _transition: NavigationTransition; private _backStack = new Array(); private _navigationQueue = new Array(); - private _frameEntryLoadedCallback: () => void; public actionBarVisibility: 'auto' | 'never' | 'always'; public _currentEntry: BackstackEntry; @@ -124,51 +121,8 @@ export class FrameBase extends CustomLayoutView { @profile public onLoaded() { - // Property page refers to the page this frame is nested into - const parentFrame = this.page?.frame; - super.onLoaded(); - - if (parentFrame?.isLoadingSubviews) { - const frameRef = new WeakRef(this); - const callback = () => { - const frame = frameRef.deref(); - if (frame) { - frame.onFrameLoaded(); - } - - this._frameEntryLoadedCallback = null; - }; - - this._frameEntryLoadedCallback = callback; - parentFrame.once(FRAME_ENTRY_LOADED_EVENT, callback); - } else { - this.onFrameLoaded(); - } - } - - @profile - public onUnloaded() { - // Property page refers to the page this frame is nested into - const parentFrame = this.page?.frame; - - super.onUnloaded(); - - // This is a precaution in case the method is called asynchronously during the loading procedure - if (this._frameEntryLoadedCallback) { - const cb = this._frameEntryLoadedCallback; - - this._frameEntryLoadedCallback = null; - - if (parentFrame) { - parentFrame.off(FRAME_ENTRY_LOADED_EVENT, cb); - } - } - } - - public onFrameLoaded(): void { this._processNextNavigationEntry(); - this._notifyFrameEntryLoaded(); } public canGoBack(): boolean { @@ -367,13 +321,6 @@ export class FrameBase extends CustomLayoutView { } } - protected _notifyFrameEntryLoaded(): void { - this.notify({ - eventName: FRAME_ENTRY_LOADED_EVENT, - object: this, - }); - } - private isNestedWithin(parentFrameCandidate: FrameBase): boolean { let frameAncestor = this as FrameBase; diff --git a/packages/core/ui/frame/index.android.ts b/packages/core/ui/frame/index.android.ts index b559205c86..2ba71aeb55 100644 --- a/packages/core/ui/frame/index.android.ts +++ b/packages/core/ui/frame/index.android.ts @@ -242,30 +242,25 @@ export class Frame extends FrameBase { this._originalBackground = null; } - super.onLoaded(); - } - - protected override _notifyFrameEntryLoaded(): void { - const currentEntry = this._currentEntry || this._executingContext?.entry; - - // Note: This is kept as a precaution and must execute before emitting the frame entry event. - // There's a bug with nested frames where sometimes the nested fragment is not recreated at all - // so we manually check on loaded event if the fragment is not recreated and recreate it - if (currentEntry && !currentEntry.fragment) { - this._frameCreateTimeout = setTimeout(() => { - const manager = this._getFragmentManager(); - const transaction = manager.beginTransaction(); - - currentEntry.fragment = this.createFragment(currentEntry, currentEntry.fragmentTag); - _updateTransitions(currentEntry); - transaction.replace(this.containerViewId, currentEntry.fragment, currentEntry.fragmentTag); - transaction.commitAllowingStateLoss(); + this._frameCreateTimeout = setTimeout(() => { + // there's a bug with nested frames where sometimes the nested fragment is not recreated at all + // so we manually check on loaded event if the fragment is not recreated and recreate it + const currentEntry = this._currentEntry || this._executingContext?.entry; + if (currentEntry) { + if (!currentEntry.fragment) { + const manager = this._getFragmentManager(); + const transaction = manager.beginTransaction(); + currentEntry.fragment = this.createFragment(currentEntry, currentEntry.fragmentTag); + _updateTransitions(currentEntry); + transaction.replace(this.containerViewId, currentEntry.fragment, currentEntry.fragmentTag); + transaction.commitAllowingStateLoss(); + } + } - this._frameCreateTimeout = null; - }, 0); - } + this._frameCreateTimeout = null; + }, 0); - super._notifyFrameEntryLoaded(); + super.onLoaded(); } onUnloaded() { From 8a54695d4530292da507bad709525813119ef00e Mon Sep 17 00:00:00 2001 From: Dimitris - Rafail Katsampas Date: Sun, 29 Jun 2025 18:47:24 +0300 Subject: [PATCH 22/23] fix: Allow attach listener to handle recreating entry fragment --- packages/core/ui/frame/index.android.ts | 82 +++++++++++++++++-------- 1 file changed, 55 insertions(+), 27 deletions(-) diff --git a/packages/core/ui/frame/index.android.ts b/packages/core/ui/frame/index.android.ts index 2ba71aeb55..cddf826da7 100644 --- a/packages/core/ui/frame/index.android.ts +++ b/packages/core/ui/frame/index.android.ts @@ -28,6 +28,7 @@ const FRAMEID = '_frameId'; const CALLBACKS = '_callbacks'; const ownerSymbol = Symbol('_owner'); +const isPendingDetachSymbol = Symbol('_isPendingDetach'); let navDepth = -1; let fragmentId = -1; @@ -58,6 +59,12 @@ function getAttachListener(): android.view.View.OnAttachStateChangeListener { if (owner) { owner._onDetachedFromWindow(); } + + if (view[isPendingDetachSymbol]) { + delete view[isPendingDetachSymbol]; + view.removeOnAttachStateChangeListener(this); + view[ownerSymbol] = null; + } }, }); @@ -73,7 +80,10 @@ export class Frame extends FrameBase { private _containerViewId = -1; private _tearDownPending = false; private _attachedToWindow = false; - private _wasReset = false; + /** + * This property indicates that the view is to be reused as a root view or has been previously disposed. + */ + private _isReset = false; private _cachedTransitionState: TransitionState; private _frameCreateTimeout: NodeJS.Timeout; @@ -143,7 +153,7 @@ export class Frame extends FrameBase { } this._attachedToWindow = true; - this._wasReset = false; + this._isReset = false; this._processNextNavigationEntry(); } @@ -162,12 +172,12 @@ export class Frame extends FrameBase { return; } - // in case the activity is "reset" using resetRootView we must wait for + // in case the activity is "reset" using resetRootView or disposed we must wait for // the attachedToWindow event to make the first navigation or it will crash // https://github.com/NativeScript/NativeScript/commit/9dd3e1a8076e5022e411f2f2eeba34aabc68d112 // though we should not do it on app "start" // or it will create a "flash" to activity background color - if (this._wasReset && !this._attachedToWindow) { + if (this._isReset && !this._attachedToWindow) { return; } @@ -228,7 +238,7 @@ export class Frame extends FrameBase { public _onRootViewReset(): void { super._onRootViewReset(); // used to handle the "first" navigate differently on first run and on reset - this._wasReset = true; + this._isReset = true; // call this AFTER the super call to ensure descendants apply their rootview-reset logic first // i.e. in a scenario with nested frames / frame with tabview let the descendandt cleanup the inner // fragments first, and then cleanup the parent fragments @@ -242,23 +252,27 @@ export class Frame extends FrameBase { this._originalBackground = null; } - this._frameCreateTimeout = setTimeout(() => { - // there's a bug with nested frames where sometimes the nested fragment is not recreated at all - // so we manually check on loaded event if the fragment is not recreated and recreate it - const currentEntry = this._currentEntry || this._executingContext?.entry; - if (currentEntry) { - if (!currentEntry.fragment) { - const manager = this._getFragmentManager(); - const transaction = manager.beginTransaction(); - currentEntry.fragment = this.createFragment(currentEntry, currentEntry.fragmentTag); - _updateTransitions(currentEntry); - transaction.replace(this.containerViewId, currentEntry.fragment, currentEntry.fragmentTag); - transaction.commitAllowingStateLoss(); + // Avoid recreating fragment here after view reset/disposal as it can cause errors + if (!this._isReset) { + // TODO: Check if this fragment precaution is still needed + this._frameCreateTimeout = setTimeout(() => { + // there's a bug with nested frames where sometimes the nested fragment is not recreated at all + // so we manually check on loaded event if the fragment is not recreated and recreate it + const currentEntry = this._currentEntry || this._executingContext?.entry; + if (currentEntry) { + if (!currentEntry.fragment) { + const manager = this._getFragmentManager(); + const transaction = manager.beginTransaction(); + currentEntry.fragment = this.createFragment(currentEntry, currentEntry.fragmentTag); + _updateTransitions(currentEntry); + transaction.replace(this.containerViewId, currentEntry.fragment, currentEntry.fragmentTag); + transaction.commitAllowingStateLoss(); + } } - } - this._frameCreateTimeout = null; - }, 0); + this._frameCreateTimeout = null; + }, 0); + } super.onLoaded(); } @@ -491,19 +505,19 @@ export class Frame extends FrameBase { if (removed.fragment) { _clearEntry(removed); + removed.fragment = null; } - removed.fragment = null; removed.viewSavedState = null; } protected _disposeBackstackEntry(entry: BackstackEntry): void { if (entry.fragment) { _clearFragment(entry); + entry.fragment = null; } entry.recreated = false; - entry.fragment = null; super._disposeBackstackEntry(entry); } @@ -522,9 +536,12 @@ export class Frame extends FrameBase { public initNativeView(): void { super.initNativeView(); const listener = getAttachListener(); - this.nativeViewProtected.addOnAttachStateChangeListener(listener); - this.nativeViewProtected[ownerSymbol] = this; - this._android.rootViewGroup = this.nativeViewProtected; + const nativeView = this.nativeViewProtected as android.view.ViewGroup; + + nativeView.addOnAttachStateChangeListener(listener); + nativeView[ownerSymbol] = this; + + this._android.rootViewGroup = nativeView; if (this._containerViewId < 0) { this._containerViewId = android.view.View.generateViewId(); } @@ -532,10 +549,18 @@ export class Frame extends FrameBase { } public disposeNativeView() { + const nativeView = this.nativeViewProtected as android.view.ViewGroup; const listener = getAttachListener(); - this.nativeViewProtected.removeOnAttachStateChangeListener(listener); - this.nativeViewProtected[ownerSymbol] = null; + // There are cases like root view when detach listener is not called upon removing view from view-tree + // so mark those views as pending and remove listener once the view is detached + if (nativeView.isAttachedToWindow()) { + nativeView[isPendingDetachSymbol] = true; + } else { + nativeView.removeOnAttachStateChangeListener(listener); + nativeView[ownerSymbol] = null; + } + this._tearDownPending = !!this._executingContext; const current = this._currentEntry; @@ -556,6 +581,9 @@ export class Frame extends FrameBase { // Dispose cached transition and store it again if view ever gets re-used this._cachedTransitionState = null; + // Mark as reset in order to properly re-initialize fragments if view ever gets re-used + this._isReset = true; + this._android.rootViewGroup = null; this._removeFromFrameStack(); super.disposeNativeView(); From 211f28ff644901b30872829822fe86ee15a27bac Mon Sep 17 00:00:00 2001 From: Dimitris - Rafail Katsampas Date: Sun, 29 Jun 2025 19:12:32 +0300 Subject: [PATCH 23/23] fix: Emit frame create timeout during attach state --- packages/core/ui/frame/index.android.ts | 56 +++++++++++++++---------- 1 file changed, 34 insertions(+), 22 deletions(-) diff --git a/packages/core/ui/frame/index.android.ts b/packages/core/ui/frame/index.android.ts index cddf826da7..c1eebfe0cb 100644 --- a/packages/core/ui/frame/index.android.ts +++ b/packages/core/ui/frame/index.android.ts @@ -155,6 +155,7 @@ export class Frame extends FrameBase { this._attachedToWindow = true; this._isReset = false; this._processNextNavigationEntry(); + this._ensureEntryFragment(); } _onDetachedFromWindow(): void { @@ -252,28 +253,7 @@ export class Frame extends FrameBase { this._originalBackground = null; } - // Avoid recreating fragment here after view reset/disposal as it can cause errors - if (!this._isReset) { - // TODO: Check if this fragment precaution is still needed - this._frameCreateTimeout = setTimeout(() => { - // there's a bug with nested frames where sometimes the nested fragment is not recreated at all - // so we manually check on loaded event if the fragment is not recreated and recreate it - const currentEntry = this._currentEntry || this._executingContext?.entry; - if (currentEntry) { - if (!currentEntry.fragment) { - const manager = this._getFragmentManager(); - const transaction = manager.beginTransaction(); - currentEntry.fragment = this.createFragment(currentEntry, currentEntry.fragmentTag); - _updateTransitions(currentEntry); - transaction.replace(this.containerViewId, currentEntry.fragment, currentEntry.fragmentTag); - transaction.commitAllowingStateLoss(); - } - } - - this._frameCreateTimeout = null; - }, 0); - } - + this._ensureEntryFragment(); super.onLoaded(); } @@ -286,6 +266,38 @@ export class Frame extends FrameBase { } } + /** + * TODO: Check if this fragment precaution is still needed + */ + private _ensureEntryFragment(): void { + // in case the activity is "reset" using resetRootView or disposed we must wait for + // the attachedToWindow event to make the first navigation or it will crash + // https://github.com/NativeScript/NativeScript/commit/9dd3e1a8076e5022e411f2f2eeba34aabc68d112 + // though we should not do it on app "start" + // or it will create a "flash" to activity background color + if (this._isReset && !this._attachedToWindow) { + return; + } + + this._frameCreateTimeout = setTimeout(() => { + // there's a bug with nested frames where sometimes the nested fragment is not recreated at all + // so we manually check on loaded event if the fragment is not recreated and recreate it + const currentEntry = this._currentEntry || this._executingContext?.entry; + if (currentEntry) { + if (!currentEntry.fragment) { + const manager = this._getFragmentManager(); + const transaction = manager.beginTransaction(); + currentEntry.fragment = this.createFragment(currentEntry, currentEntry.fragmentTag); + _updateTransitions(currentEntry); + transaction.replace(this.containerViewId, currentEntry.fragment, currentEntry.fragmentTag); + transaction.commitAllowingStateLoss(); + } + } + + this._frameCreateTimeout = null; + }, 0); + } + private disposeCurrentFragment(): void { if (!this._currentEntry || !this._currentEntry.fragment || !this._currentEntry.fragment.isAdded()) { return;