fix(tabs): delay loadView when animation runs (NativeScript#8353)

vtrifonov · web-flow · commit e649a6cfd618 · 2020-02-21T16:47:33.000+02:00
* fix(tabs): delay loadView when animation runs

* chore: update api.md

* chore: remove unnecessary casting

* test: Added disabled test for changing tabs
diff --git a/api-reports/NativeScript.api.md b/api-reports/NativeScript.api.md
@@ -772,6 +772,9 @@ export class Frame extends View {
 
     animated: boolean;
 
+    // (undocumented)
+    _animationInProgress: boolean;
+
     backStack: Array<BackstackEntry>;
 
     canGoBack(): boolean;
@@ -2931,6 +2934,7 @@ export abstract class ViewBase extends Observable {
     // (undocumented)
     _setupAsRootView(context: any): void;
     _setupUI(context: any /* android.content.Context */, atIndex?: number): void;
+    _shouldDelayLoad(): boolean;
     showModal(moduleName: string, modalOptions: ShowModalOptions): ViewBase;
     showModal(view: ViewBase, modalOptions: ShowModalOptions): ViewBase;
     public readonly style: Style;
diff --git a/e2e/ui-tests-app/app/tabs/frame-in-tabs-inner-page-1.xml b/e2e/ui-tests-app/app/tabs/frame-in-tabs-inner-page-1.xml
@@ -0,0 +1,3 @@
+<Page class="page" >
+    <Label text="Inner label 1" backgroundColor="blue"/>
+</Page>
diff --git a/e2e/ui-tests-app/app/tabs/frame-in-tabs-inner-page-2.xml b/e2e/ui-tests-app/app/tabs/frame-in-tabs-inner-page-2.xml
@@ -0,0 +1,3 @@
+<Page class="page">
+    <Label text="Inner label 2"  backgroundColor="red"/>
+</Page>
diff --git a/e2e/ui-tests-app/app/tabs/frame-in-tabs-inner-page-3.xml b/e2e/ui-tests-app/app/tabs/frame-in-tabs-inner-page-3.xml
@@ -0,0 +1,3 @@
+<Page class="page">
+    <Label text="Inner label 3"  backgroundColor="orange"/>
+</Page>
diff --git a/e2e/ui-tests-app/app/tabs/frame-in-tabs-inner-page-4.xml b/e2e/ui-tests-app/app/tabs/frame-in-tabs-inner-page-4.xml
@@ -0,0 +1,3 @@
+<Page class="page">
+    <Label text="Inner label 4"  backgroundColor="green"/>
+</Page>
diff --git a/e2e/ui-tests-app/app/tabs/frame-in-tabs.ts b/e2e/ui-tests-app/app/tabs/frame-in-tabs.ts
@@ -0,0 +1,3 @@
+export function onItemTap(args) {
+    console.log(`Item with index: ${args.index} tapped`);
+}
diff --git a/e2e/ui-tests-app/app/tabs/frame-in-tabs.xml b/e2e/ui-tests-app/app/tabs/frame-in-tabs.xml
@@ -0,0 +1,24 @@
+<Page class="page">
+    <StackLayout>
+        <Tabs highlightColor="red" offscreenTabLimit="1">
+            <TabStrip highlightColor="green" itemTap="onItemTap">
+                <TabStripItem title="1"></TabStripItem>
+                <TabStripItem title="2"></TabStripItem>
+                <TabStripItem title="3"></TabStripItem>
+                <TabStripItem title="4"></TabStripItem>
+            </TabStrip>
+            <TabContentItem>
+                <Frame defaultPage="tabs/frame-in-tabs-inner-page-1"></Frame>
+            </TabContentItem>
+            <TabContentItem>
+                <Frame defaultPage="tabs/frame-in-tabs-inner-page-2"></Frame>
+            </TabContentItem>
+            <TabContentItem>
+                <Frame defaultPage="tabs/frame-in-tabs-inner-page-3"></Frame>
+            </TabContentItem>
+            <TabContentItem>
+                <Frame defaultPage="tabs/frame-in-tabs-inner-page-4"></Frame>
+            </TabContentItem>
+        </Tabs>
+    </StackLayout>
+</Page>
diff --git a/e2e/ui-tests-app/e2e/suites/tab-navigation/tabs/tabs-tests.e2e-spec.ts b/e2e/ui-tests-app/e2e/suites/tab-navigation/tabs/tabs-tests.e2e-spec.ts
@@ -283,4 +283,22 @@ describe(`${imagePrefix}-suite`, async function () {
         assert.isTrue(driver.imageHelper.hasImageComparisonPassed());
         await tabsViewBasePage.navigateBackToSuitMainPage();
     });
+
+    // it(`${imagePrefix}-frame-in-tabs`, async function () {
+    //     await tabsViewBasePage.navigateToSample("frame-in-tabs");
+    //     await driver.imageHelper.compareScreen();
+
+    //     // go through the tabs and check that they are loaded
+    //     await tabsViewBasePage.tabOnItem(1);
+    //     await driver.imageHelper.compareScreen();
+
+    //     await tabsViewBasePage.tabOnItem(2);
+    //     await driver.imageHelper.compareScreen();
+
+    //     await tabsViewBasePage.tabOnItem(3);
+    //     await driver.imageHelper.compareScreen();
+
+    //     assert.isTrue(driver.imageHelper.hasImageComparisonPassed());
+    //     await tabsViewBasePage.navigateBackToSuitMainPage();
+    // });
 });
diff --git a/nativescript-core/ui/core/view-base/view-base.d.ts b/nativescript-core/ui/core/view-base/view-base.d.ts
@@ -470,6 +470,12 @@ export abstract class ViewBase extends Observable {
      */
     _setupAsRootView(context: any): void;
 
+    /**
+     * When returning true the callLoaded method will be run in setTimeout
+     * Method is intended to be overridden by inheritors and used as "protected"
+     */
+    _shouldDelayLoad(): boolean;
+
     /**
      * @private
      */
diff --git a/nativescript-core/ui/core/view-base/view-base.ts b/nativescript-core/ui/core/view-base/view-base.ts
@@ -615,10 +615,18 @@ export abstract class ViewBase extends Observable implements ViewBaseDefinition
 
     public loadView(view: ViewBase): void {
         if (view && !view.isLoaded) {
-            view.callLoaded();
+            if (this._shouldDelayLoad()) {
+                setTimeout(() => view.callLoaded());
+            } else {
+                view.callLoaded();
+            }
         }
     }
 
+    public _shouldDelayLoad(): boolean {
+        return false;
+    }
+
     public unloadView(view: ViewBase): void {
         if (view && view.isLoaded) {
             view.callUnloaded();
@@ -1053,7 +1061,7 @@ export const classNameProperty = new Property<ViewBase, string>({
         cssClasses.clear();
 
         if (shouldAddModalRootViewCssClasses) {
-            cssClasses.add(MODAL_ROOT_VIEW_CSS_CLASS);    
+            cssClasses.add(MODAL_ROOT_VIEW_CSS_CLASS);
         } else if (shouldAddRootViewCssClasses) {
             cssClasses.add(ROOT_VIEW_CSS_CLASS);
         }
diff --git a/nativescript-core/ui/frame/frame-common.ts b/nativescript-core/ui/frame/frame-common.ts
@@ -42,6 +42,7 @@ export class FrameBase extends CustomLayoutView implements FrameDefinition {
 
     public actionBarVisibility: "auto" | "never" | "always";
     public _currentEntry: BackstackEntry;
+    public _animationInProgress = false;
     public _executingContext: NavigationContext;
     public _isInFrameStack = false;
     public static defaultAnimatedNavigation = true;
diff --git a/nativescript-core/ui/frame/frame.d.ts b/nativescript-core/ui/frame/frame.d.ts
@@ -147,6 +147,10 @@ export class Frame extends View {
      * @private
      */
     navigationBarHeight: number;
+    /**
+     * @private
+     */
+    _animationInProgress: boolean;
     /**
      * @private
      */
@@ -218,21 +222,21 @@ export function setFragmentClass(clazz: any): void;
 
 /**
  * @deprecated Use Frame.getFrameById() instead.
- * 
+ *
  * Gets a frame by id.
  */
 export function getFrameById(id: string): Frame;
 
 /**
  * @deprecated Use Frame.topmost() instead.
- * 
+ *
  * Gets the topmost frame in the frames stack. An application will typically has one frame instance. Multiple frames handle nested (hierarchical) navigation scenarios.
  */
 export function topmost(): Frame;
 
 /**
  * @deprecated Use Frame.goBack() instead.
- * 
+ *
  * Navigates back using the navigation hierarchy (if any). Updates the Frame stack as needed.
  * This method will start from the topmost Frame and will recursively search for an instance that has the canGoBack operation available.
  */
@@ -241,7 +245,7 @@ export function goBack();
 //@private
 /**
  * @deprecated Use Frame._stack() instead.
- * 
+ *
  * @private
  */
 export function _stack(): Array<Frame>;
@@ -487,7 +491,7 @@ export function setActivityCallbacks(activity: any /*androidx.appcompat.app.AppC
 //@private
 /**
  * @deprecated Use Frame.reloadPage() instead.
- * 
+ *
  * @private
  */
 export function reloadPage(context?: ModuleContext): void;
diff --git a/nativescript-core/ui/page/page.ios.ts b/nativescript-core/ui/page/page.ios.ts
@@ -93,7 +93,7 @@ class UIViewControllerImpl extends UIViewController {
             return;
         }
 
-        const frame = this.navigationController ? (<any>this.navigationController).owner : null;
+        const frame = (this.navigationController ? (<any>this.navigationController).owner : null);
         const newEntry = this[ENTRY];
 
         // Don't raise event if currentPage was showing modal page.
@@ -199,7 +199,7 @@ class UIViewControllerImpl extends UIViewController {
         if (!owner) {
             return;
         }
-        
+
         // Cache presentedViewController if any. We don't want to raise
         // navigation events in case of presenting view controller.
         if (!owner._presentedViewController) {
@@ -230,7 +230,6 @@ class UIViewControllerImpl extends UIViewController {
         if (!page || page.modal || page._presentedViewController) {
             return;
         }
-        
         // Forward navigation does not remove page from frame so we raise unloaded manually.
         if (page.isLoaded) {
             page.callUnloaded();
@@ -349,6 +348,10 @@ export class Page extends PageBase {
         //
     }
 
+    public _shouldDelayLoad(): boolean {
+        return this._frame && this._frame._animationInProgress;
+    }
+
     public onLoaded(): void {
         super.onLoaded();
         if (this.hasActionBar) {
diff --git a/nativescript-core/ui/tabs/tabs.ios.ts b/nativescript-core/ui/tabs/tabs.ios.ts
@@ -1091,7 +1091,18 @@ export class Tabs extends TabsBase {
             }
 
             this._currentNativeSelectedIndex = value;
+
+            let itemControllerOwner = null;
+            if (itemController._owner) {
+                let itemControllerOwner = <Frame>itemController._owner.get();
+                // do not load new views while the animation is in progress https://stackoverflow.com/a/47031524/613113
+                itemControllerOwner._animationInProgress = true;
+            }
+
             this.viewController.setViewControllersDirectionAnimatedCompletion(controllers, navigationDirection, true, (finished: boolean) => {
+                if (itemControllerOwner) {
+                    itemControllerOwner._animationInProgress = false;
+                }
                 if (finished) {
                     // HACK: UIPageViewController fix; see https://stackoverflow.com/a/17330606
                     invokeOnRunLoop(() => this.viewController.setViewControllersDirectionAnimatedCompletion(controllers, navigationDirection, false, null));

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+<Page class="page" >`
	`2`	`+ <Label text="Inner label 1" backgroundColor="blue"/>`
	`3`	`+</Page>`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+<Page class="page">`
	`2`	`+ <Label text="Inner label 2" backgroundColor="red"/>`
	`3`	`+</Page>`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+<Page class="page">`
	`2`	`+ <Label text="Inner label 3" backgroundColor="orange"/>`
	`3`	`+</Page>`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+<Page class="page">`
	`2`	`+ <Label text="Inner label 4" backgroundColor="green"/>`
	`3`	`+</Page>`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+export function onItemTap(args) {`
	`2`	+ console.log(`Item with index: ${args.index} tapped`);
	`3`	`+}`