chore: repsect PR comments

ADjenkov · ADjenkov · commit 452cb8fc0fbd · 2018-10-23T17:22:29.000+03:00
diff --git a/e2e/nested-router-tab-view/app/login/login.component.html b/e2e/nested-router-tab-view/app/login/login.component.html
@@ -4,6 +4,6 @@
 
 <StackLayout>
     <!-- <Button text="Go To Home Page" [nsRouterLink]="['/home', { outlets: { playerTab: ['players'], teamTab: ['teams'] } }]"></Button> -->
-    <Button text="Go To Home Page" [nsRouterLink]="['/home', { outlets: { playerTab: ['players'], teamTab: ['teams'] } }]"></Button>
-    <Button text="Go To Lazy Home Page" [nsRouterLink]="['/home-lazy/home', { outlets: { playerTab: ['players'], teamTab: ['teams'] } }]"></Button>
+    <Button text="Go To Home Page" [nsRouterLink]="['../home', { outlets: { playerTab: ['players'], teamTab: ['teams'] } }]"></Button>
+    <Button text="Go To Lazy Home Page" [nsRouterLink]="['../home-lazy/home', { outlets: { playerTab: ['players'], teamTab: ['teams'] } }]"></Button>
 </StackLayout> 
diff --git a/nativescript-angular/directives/dialogs.ts b/nativescript-angular/directives/dialogs.ts
@@ -132,7 +132,7 @@ export class ModalDialogService {
             doneCallback.apply(undefined, args);
             if (componentView) {
                 componentView.closeModal();
-                this.location._finishCloseModalNavigation();
+                this.location._closeModalNavigation();
                 detachedLoaderRef.instance.detectChanges();
                 detachedLoaderRef.destroy();
             }
diff --git a/nativescript-angular/router/ns-empty-outlet.component.ts b/nativescript-angular/router/ns-empty-outlet.component.ts
@@ -4,7 +4,7 @@ import { Page } from "tns-core-modules/ui/page";
     // tslint:disable-next-line:component-selector
     selector: "ns-empty-outlet",
     moduleId: module.id,
-    template: "<page-router-outlet isNSEmptyOutlet='true'></page-router-outlet>"
+    template: "<page-router-outlet isEmptyOutlet='true'></page-router-outlet>"
 })
 export class NSEmptyOutletComponent {
     constructor(private page: Page) {
diff --git a/nativescript-angular/router/ns-location-strategy.ts b/nativescript-angular/router/ns-location-strategy.ts
@@ -11,9 +11,12 @@ export class Outlet {
     modalNavigationDepth: number;
     parent: Outlet;
     isPageNavigationBack: boolean;
+
+    // More than one key available when using NSEmptyOutletComponent component
+    // in module that lazy loads children (loadChildren) and has outlet name.
     outletKeys: Array<string>;
     pathByOutlets: string;
-    statesByOutlet: Array<LocationState> = [];
+    states: Array<LocationState> = [];
     frame: Frame;
 
     // Used in reuse-strategy by its children to determine if they should be detached too.
@@ -27,13 +30,13 @@ export class Outlet {
     }
 
     peekState(): LocationState {
-        if (this.statesByOutlet.length > 0) {
-            return this.statesByOutlet[this.statesByOutlet.length - 1];
+        if (this.states.length > 0) {
+            return this.states[this.states.length - 1];
         }
         return null;
     }
 
-    containsLastState(stateUrl: string): boolean {
+    containsTopState(stateUrl: string): boolean {
         const lastState = this.peekState();
         return lastState && lastState.segmentGroup.toString() === stateUrl;
     }
@@ -92,7 +95,6 @@ export class NSLocationStrategy extends LocationStrategy {
         if (state.isRootSegmentGroup) {
             tree.root = state.segmentGroup;
         } else if (changedOutlet) {
-            // tslint:disable-next-line:max-line-length
             this.updateSegmentGroup(tree.root, changedOutlet, state.segmentGroup);
         }
 
@@ -129,49 +131,49 @@ export class NSLocationStrategy extends LocationStrategy {
             // The url serializer doesn't parse this url as having a primary outlet.
             const rootOutlet = this.createOutlet("primary", null, null);
             this.currentOutlet = rootOutlet;
-        } else {
-            const queue = [];
-            queue.push(urlTreeRoot);
-            let currentTree = queue.shift();
-
-            while (currentTree) {
-                Object.keys(currentTree.children).forEach(outletName => {
-                    const currentSegmentGroup = currentTree.children[outletName];
-                    currentSegmentGroup.outlet = outletName;
-                    currentSegmentGroup.root = urlTreeRoot;
-
-                    let outletKey = this.getSegmentGroupFullPath(currentTree) + outletName;
-                    let outlet = this.findOutletByKey(outletKey);
-                    const parentOutletName = currentTree.outlet || "";
-                    const parentOutletKey = this.getSegmentGroupFullPath(currentTree.parent) + parentOutletName;
-                    const parentOutlet = this.findOutletByKey(parentOutletKey);
-
-                    const containsLastState = outlet && outlet.containsLastState(currentSegmentGroup.toString());
-                    if (!outlet) {
-                        // tslint:disable-next-line:max-line-length
-                        outlet = this.createOutlet(outletKey, currentSegmentGroup, parentOutlet, this._modalNavigationDepth);
-                        this.currentOutlet = outlet;
-                    } else if (this._modalNavigationDepth > 0 && outlet.showingModal && !containsLastState) {
-                        // Navigation inside modal view.
-                        this.upsertModalOutlet(outlet, currentSegmentGroup);
-                    } else {
-                        outlet.parent = parentOutlet;
+            return;
+        }
+
+        const queue = [];
+        let currentTree = <any>urlTreeRoot;
+
+        while (currentTree) {
+            Object.keys(currentTree.children).forEach(outletName => {
+                const currentSegmentGroup = currentTree.children[outletName];
+                currentSegmentGroup.outlet = outletName;
+                currentSegmentGroup.root = urlTreeRoot;
+
+                let outletKey = this.getSegmentGroupFullPath(currentTree) + outletName;
+                let outlet = this.findOutletByKey(outletKey);
+                const parentOutletName = currentTree.outlet || "";
+                const parentOutletKey = this.getSegmentGroupFullPath(currentTree.parent) + parentOutletName;
+                const parentOutlet = this.findOutletByKey(parentOutletKey);
 
-                        if (this.updateStates(outlet, currentSegmentGroup)) {
-                            this.currentOutlet = outlet; // If states updated
-                        }
+                const containsLastState = outlet && outlet.containsTopState(currentSegmentGroup.toString());
+                if (!outlet) {
+                    // tslint:disable-next-line:max-line-length
+                    outlet = this.createOutlet(outletKey, currentSegmentGroup, parentOutlet, this._modalNavigationDepth);
+                    this.currentOutlet = outlet;
+                } else if (this._modalNavigationDepth > 0 && outlet.showingModal && !containsLastState) {
+                    // Navigation inside modal view.
+                    this.upsertModalOutlet(outlet, currentSegmentGroup);
+                } else {
+                    outlet.parent = parentOutlet;
+
+                    if (this.updateStates(outlet, currentSegmentGroup)) {
+                        this.currentOutlet = outlet; // If states updated
                     }
+                }
 
-                    queue.push(currentTree.children[outletName]);
-                });
+                queue.push(currentSegmentGroup);
+            });
 
-                currentTree = queue.shift();
-            }
+            currentTree = queue.shift();
         }
     }
 
     replaceState(state: any, title: string, url: string, queryParams: string): void {
-        const states = this.currentOutlet && this.currentOutlet.statesByOutlet;
+        const states = this.currentOutlet && this.currentOutlet.states;
 
         if (states && states.length > 0) {
             if (isLogEnabled()) {
@@ -195,7 +197,7 @@ export class NSLocationStrategy extends LocationStrategy {
         this.currentOutlet = outlet || this.currentOutlet;
 
         if (this.currentOutlet.isPageNavigationBack) {
-            const states = this.currentOutlet.statesByOutlet;
+            const states = this.currentOutlet.states;
             // We are navigating to the previous page
             // clear the stack until we get to a page navigation state
             let state = states.pop();
@@ -231,14 +233,14 @@ export class NSLocationStrategy extends LocationStrategy {
                         " state is not page - just pop");
                 }
 
-                this.callPopState(this.currentOutlet.statesByOutlet.pop(), true);
+                this.callPopState(this.currentOutlet.states.pop(), true);
             }
         }
     }
 
     canGoBack(outlet?: Outlet) {
         outlet = outlet || this.currentOutlet;
-        return outlet.statesByOutlet.length > 1;
+        return outlet.states.length > 1;
     }
 
     onPopState(fn: (_: any) => any): void {
@@ -279,7 +281,7 @@ export class NSLocationStrategy extends LocationStrategy {
         let result = [];
 
         this.outlets.forEach(outlet => {
-            const outletStates = outlet.statesByOutlet;
+            const outletStates = outlet.states;
             const outletLog = outletStates
                 // tslint:disable-next-line:max-line-length
                 .map((v, i) => `${outlet.outletKeys}.${i}.[${v.isPageNavigation ? "PAGE" : "INTERNAL"}].[${outlet.modalNavigationDepth ? "MODAL" : "BASE"}] "${v.segmentGroup.toString()}"`)
@@ -336,9 +338,9 @@ export class NSLocationStrategy extends LocationStrategy {
         this._modalNavigationDepth++;
     }
 
-    public _finishCloseModalNavigation() {
+    public _closeModalNavigation() {
         if (isLogEnabled()) {
-            routerLog("NSLocationStrategy.finishCloseModalNavigation()");
+            routerLog("NSLocationStrategy.closeModalNavigation()");
         }
         this._modalNavigationDepth--;
 
@@ -365,7 +367,7 @@ export class NSLocationStrategy extends LocationStrategy {
             if (isLogEnabled()) {
                 routerLog("NSLocationStrategy._beginPageNavigation clearing states history");
             }
-            this.currentOutlet.statesByOutlet = [lastState];
+            this.currentOutlet.states = [lastState];
         }
 
         this._currentNavigationOptions = undefined;
@@ -412,7 +414,13 @@ export class NSLocationStrategy extends LocationStrategy {
 
         while (segmentGroup) {
             const url = segmentGroup.toString();
-            fullPath = fullPath ? (url ? url + "/" : url) + fullPath : url;
+
+            if (fullPath) {
+                fullPath = (url ? url + "/" : "") + fullPath;
+            } else {
+                fullPath = url;
+            }
+
             segmentGroup = segmentGroup.parent;
         }
 
@@ -496,9 +504,9 @@ export class NSLocationStrategy extends LocationStrategy {
     }
 
     private updateStates(outlet: Outlet, currentSegmentGroup: UrlSegmentGroup): boolean {
-        const isNewPage = outlet.statesByOutlet.length === 0;
-        const lastState = outlet.statesByOutlet[outlet.statesByOutlet.length - 1];
-        const equalStateUrls = outlet.containsLastState(currentSegmentGroup.toString());
+        const isNewPage = outlet.states.length === 0;
+        const lastState = outlet.states[outlet.states.length - 1];
+        const equalStateUrls = outlet.containsTopState(currentSegmentGroup.toString());
 
         const locationState: LocationState = {
             segmentGroup: currentSegmentGroup,
@@ -507,7 +515,7 @@ export class NSLocationStrategy extends LocationStrategy {
         };
 
         if (!lastState || !equalStateUrls) {
-            outlet.statesByOutlet.push(locationState);
+            outlet.states.push(locationState);
 
             if (this._modalNavigationDepth === 0 && !outlet.showingModal) {
                 this.updateParentsStates(outlet, currentSegmentGroup.parent);
@@ -553,7 +561,7 @@ export class NSLocationStrategy extends LocationStrategy {
             isPageNavigation: true // It is a new OutletNode.
         };
 
-        newOutlet.statesByOutlet = [locationState];
+        newOutlet.states = [locationState];
         newOutlet.parent = parent;
         this.outlets.push(newOutlet);
 
@@ -571,6 +579,9 @@ export class NSLocationStrategy extends LocationStrategy {
             if (childrenCount && segmentGroup.children[currentPath]) {
                 segmentGroup = segmentGroup.children[currentPath];
             } else {
+                // If no child outlet found with the given name - forget about all previously found outlets.
+                // example: seaching for 'primary-second-primary' shouldn't return 'primary-second'
+                // if no 'primary' child available on 'second'.
                 segmentGroup = null;
                 break;
             }
@@ -580,10 +591,9 @@ export class NSLocationStrategy extends LocationStrategy {
     }
 
     // Traversal and replacement of segmentGroup.
-    private updateSegmentGroup(rootNode, oldSegmentGroup: UrlSegmentGroup, newSegmentGroup: UrlSegmentGroup) {
+    private updateSegmentGroup(rootNode: any, oldSegmentGroup: UrlSegmentGroup, newSegmentGroup: UrlSegmentGroup) {
         const queue = [];
-        queue.push(rootNode);
-        let currentTree = queue.shift();
+        let currentTree = rootNode;
 
         while (currentTree) {
             Object.keys(currentTree.children).forEach(outletName => {
diff --git a/nativescript-angular/router/page-router-outlet.ts b/nativescript-angular/router/page-router-outlet.ts
@@ -109,7 +109,7 @@ export class PageRouterOutlet implements OnDestroy { // tslint:disable-line:dire
 
     private outlet: Outlet;
     private name: string;
-    private isNSEmptyOutlet: boolean;
+    private isEmptyOutlet: boolean;
     private viewUtil: ViewUtil;
     private frame: Frame;
 
@@ -150,7 +150,7 @@ export class PageRouterOutlet implements OnDestroy { // tslint:disable-line:dire
         private parentContexts: ChildrenOutletContexts,
         private location: ViewContainerRef,
         @Attribute("name") name: string,
-        @Attribute("isNSEmptyOutlet") isNSEmptyOutlet: boolean,
+        @Attribute("isEmptyOutlet") isEmptyOutlet: boolean,
         private locationStrategy: NSLocationStrategy,
         private componentFactoryResolver: ComponentFactoryResolver,
         private resolver: ComponentFactoryResolver,
@@ -160,7 +160,7 @@ export class PageRouterOutlet implements OnDestroy { // tslint:disable-line:dire
         private routeReuseStrategy: NSRouteReuseStrategy,
         elRef: ElementRef
     ) {
-        this.isNSEmptyOutlet = isNSEmptyOutlet;
+        this.isEmptyOutlet = isEmptyOutlet;
         this.frame = elRef.nativeElement;
         if (isLogEnabled()) {
             log(`PageRouterOutlet.constructor frame: ${this.frame}`);
@@ -360,6 +360,9 @@ export class PageRouterOutlet implements OnDestroy { // tslint:disable-line:dire
         });
     }
 
+    // Find and mark the top activated route as an activated one.
+    // In ns-location-strategy we are reusing components only if their corresponing routes
+    // are marked as activated from this method.
     private markActivatedRoute(activatedRoute: ActivatedRoute) {
         const queue = [];
         queue.push(activatedRoute.snapshot);
@@ -409,7 +412,7 @@ export class PageRouterOutlet implements OnDestroy { // tslint:disable-line:dire
         }
 
         // Named lazy loaded outlet.
-        if (!outlet && this.isNSEmptyOutlet) {
+        if (!outlet && this.isEmptyOutlet) {
             const parentOutletKey = this.locationStrategy.getRouteFullPath(topActivatedRoute.parent);
             outlet = this.locationStrategy.findOutletByKey(parentOutletKey);
 

Original file line number	Diff line number	Diff line change
`@@ -132,7 +132,7 @@ export class ModalDialogService {`
`132`	`132`	`doneCallback.apply(undefined, args);`
`133`	`133`	`if (componentView) {`
`134`	`134`	`componentView.closeModal();`
`135`		`- this.location._finishCloseModalNavigation();`
	`135`	`+ this.location._closeModalNavigation();`
`136`	`136`	`detachedLoaderRef.instance.detectChanges();`
`137`	`137`	`detachedLoaderRef.destroy();`
`138`	`138`	`}`