From db54a84d1418950a9c1ad07f2553179cbf231135 Mon Sep 17 00:00:00 2001 From: vsavkin Date: Wed, 20 Jul 2016 17:51:21 -0700 Subject: [PATCH] fix(router): routerLinkActive should only set classes after the router has successfully navigated --- modules/@angular/common/testing.ts | 1 + .../common/testing/mock_location_strategy.ts | 2 ++ .../src/directives/router_link_active.ts | 2 +- modules/@angular/router/src/router.ts | 8 +++++++ modules/@angular/router/test/router.spec.ts | 23 +++++++++++++++++++ .../router/testing/router_testing_module.ts | 4 ++-- modules/@angular/router/tsconfig-es2015.json | 1 + modules/@angular/router/tsconfig-es5.json | 1 + tools/public_api_guard/common/testing.d.ts | 18 +++++++++++++++ tools/public_api_guard/router/index.d.ts | 13 ++++++++++- 10 files changed, 69 insertions(+), 4 deletions(-) diff --git a/modules/@angular/common/testing.ts b/modules/@angular/common/testing.ts index 48f16c0111..1e1ae6ca46 100644 --- a/modules/@angular/common/testing.ts +++ b/modules/@angular/common/testing.ts @@ -7,3 +7,4 @@ */ export {SpyLocation} from './testing/location_mock'; +export {MockLocationStrategy} from './testing/mock_location_strategy'; \ No newline at end of file diff --git a/modules/@angular/common/testing/mock_location_strategy.ts b/modules/@angular/common/testing/mock_location_strategy.ts index 88960b79ee..b2dc6d4dcf 100644 --- a/modules/@angular/common/testing/mock_location_strategy.ts +++ b/modules/@angular/common/testing/mock_location_strategy.ts @@ -16,6 +16,8 @@ import {EventEmitter, ObservableWrapper} from '../src/facade/async'; /** * A mock implementation of {@link LocationStrategy} that allows tests to fire simulated * location events. + * + * @stable */ @Injectable() export class MockLocationStrategy extends LocationStrategy { diff --git a/modules/@angular/router/src/directives/router_link_active.ts b/modules/@angular/router/src/directives/router_link_active.ts index 51afdd1b1b..b08cf23af3 100644 --- a/modules/@angular/router/src/directives/router_link_active.ts +++ b/modules/@angular/router/src/directives/router_link_active.ts @@ -94,7 +94,7 @@ export class RouterLinkActive implements OnChanges, OnDestroy, AfterContentInit ngOnDestroy(): any { this.subscription.unsubscribe(); } private update(): void { - if (!this.links || !this.linksWithHrefs) return; + if (!this.links || !this.linksWithHrefs || !this.router.navigated) return; const currentUrlTree = this.router.parseUrl(this.router.url); const isActiveLinks = this.reduceList(currentUrlTree, this.links); diff --git a/modules/@angular/router/src/router.ts b/modules/@angular/router/src/router.ts index d00f0ddf40..f3ba893ae9 100644 --- a/modules/@angular/router/src/router.ts +++ b/modules/@angular/router/src/router.ts @@ -133,6 +133,13 @@ export class Router { private config: Routes; private configLoader: RouterConfigLoader; + /** + * Indicates if at least one navigation happened. + * + * @experimental + */ + navigated: boolean = false; + /** * Creates the router service. */ @@ -385,6 +392,7 @@ export class Router { }) .then( () => { + this.navigated = true; this.routerEvents.next( new NavigationEnd(id, this.serializeUrl(url), this.serializeUrl(appliedUrl))); resolvePromise(navigationIsSuccessful); diff --git a/modules/@angular/router/test/router.spec.ts b/modules/@angular/router/test/router.spec.ts index 9c667d41a7..50c75c2498 100644 --- a/modules/@angular/router/test/router.spec.ts +++ b/modules/@angular/router/test/router.spec.ts @@ -1194,6 +1194,29 @@ describe('Integration', () => { expect(nativeButton.className).toEqual(''); }))); + it('should not set the class until the first navigation succeeds', + fakeAsync(inject( + [Router, TestComponentBuilder, Location], + (router: Router, tcb: TestComponentBuilder, location: Location) => { + @Component({ + template: + '' + }) + class RootCmpWithLink { + } + + const f = tcb.createFakeAsync(RootCmpWithLink); + advance(f); + + const link = f.debugElement.nativeElement.querySelector('a'); + expect(link.className).toEqual(''); + + router.initialNavigation(); + advance(f); + + expect(link.className).toEqual('active'); + }))); + it('should set the class on a parent element when the link is active', fakeAsync(inject( diff --git a/modules/@angular/router/testing/router_testing_module.ts b/modules/@angular/router/testing/router_testing_module.ts index 7581064a42..1019961f6f 100644 --- a/modules/@angular/router/testing/router_testing_module.ts +++ b/modules/@angular/router/testing/router_testing_module.ts @@ -7,8 +7,7 @@ */ import {Location, LocationStrategy} from '@angular/common'; -import {SpyLocation} from '@angular/common/testing'; -import {MockLocationStrategy} from '@angular/common/testing/mock_location_strategy'; +import {MockLocationStrategy, SpyLocation} from '@angular/common/testing'; import {AppModule, AppModuleFactory, AppModuleFactoryLoader, Compiler, ComponentResolver, Injectable, Injector} from '@angular/core'; import {Router, RouterOutletMap, Routes, UrlSerializer} from '../index'; @@ -16,6 +15,7 @@ import {ROUTES} from '../src/router_config_loader'; import {ROUTER_DIRECTIVES, ROUTER_PROVIDERS} from '../src/router_module'; + /** * A spy for {@link AppModuleFactoryLoader} that allows tests to simulate the loading of app module * factories. diff --git a/modules/@angular/router/tsconfig-es2015.json b/modules/@angular/router/tsconfig-es2015.json index 2b3869ec1a..3bd4b4097e 100644 --- a/modules/@angular/router/tsconfig-es2015.json +++ b/modules/@angular/router/tsconfig-es2015.json @@ -10,6 +10,7 @@ "paths": { "@angular/core": ["../../../dist/packages-dist/core"], "@angular/common": ["../../../dist/packages-dist/common"], + "@angular/common/testing": ["../../../dist/packages-dist/common/testing"], "@angular/compiler": ["../../../dist/packages-dist/compiler"], "@angular/platform-browser": ["../../../dist/packages-dist/platform-browser"], "@angular/platform-browser-dynamic": ["../../../dist/packages-dist/platform-browser-dynamic"] diff --git a/modules/@angular/router/tsconfig-es5.json b/modules/@angular/router/tsconfig-es5.json index 260dd8e5cf..4d8c4878e9 100644 --- a/modules/@angular/router/tsconfig-es5.json +++ b/modules/@angular/router/tsconfig-es5.json @@ -10,6 +10,7 @@ "paths": { "@angular/core": ["../../../dist/packages-dist/core"], "@angular/common": ["../../../dist/packages-dist/common"], + "@angular/common/testing": ["../../../dist/packages-dist/common/testing"], "@angular/compiler": ["../../../dist/packages-dist/compiler"], "@angular/platform-browser": ["../../../dist/packages-dist/platform-browser"], "@angular/platform-browser-dynamic": ["../../../dist/packages-dist/platform-browser-dynamic"] diff --git a/tools/public_api_guard/common/testing.d.ts b/tools/public_api_guard/common/testing.d.ts index e9d5546826..ee77b5853e 100644 --- a/tools/public_api_guard/common/testing.d.ts +++ b/tools/public_api_guard/common/testing.d.ts @@ -1,3 +1,21 @@ +/** @stable */ +export declare class MockLocationStrategy extends LocationStrategy { + internalBaseHref: string; + internalPath: string; + internalTitle: string; + urlChanges: string[]; + constructor(); + back(): void; + forward(): void; + getBaseHref(): string; + onPopState(fn: (value: any) => void): void; + path(includeHash?: boolean): string; + prepareExternalUrl(internal: string): string; + pushState(ctx: any, title: string, path: string, query: string): void; + replaceState(ctx: any, title: string, path: string, query: string): void; + simulatePopState(url: string): void; +} + /** @experimental */ export declare class SpyLocation implements Location { urlChanges: string[]; diff --git a/tools/public_api_guard/router/index.d.ts b/tools/public_api_guard/router/index.d.ts index 40cec17341..67e09d6215 100644 --- a/tools/public_api_guard/router/index.d.ts +++ b/tools/public_api_guard/router/index.d.ts @@ -82,6 +82,8 @@ export declare class NavigationError { /** @experimental */ export interface NavigationExtras { fragment?: string; + preserveFragment?: boolean; + preserveQueryParams?: boolean; queryParams?: Params; relativeTo?: ActivatedRoute; } @@ -130,10 +132,11 @@ export interface Route { /** @stable */ export declare class Router { events: Observable; + /** @experimental */ navigated: boolean; routerState: RouterState; url: string; constructor(rootComponentType: Type, resolver: ComponentResolver, urlSerializer: UrlSerializer, outletMap: RouterOutletMap, location: Location, injector: Injector, loader: AppModuleFactoryLoader, config: Routes); - createUrlTree(commands: any[], {relativeTo, queryParams, fragment}?: NavigationExtras): UrlTree; + createUrlTree(commands: any[], {relativeTo, queryParams, fragment, preserveQueryParams, preserveFragment}?: NavigationExtras): UrlTree; dispose(): void; initialNavigation(): void; navigate(commands: any[], extras?: NavigationExtras): Promise; @@ -152,6 +155,8 @@ export declare type RouterConfig = Route[]; /** @stable */ export declare class RouterLink { fragment: string; + preserveFragment: boolean; + preserveQueryParams: boolean; queryParams: { [k: string]: any; }; @@ -176,10 +181,16 @@ export declare class RouterLinkActive implements OnChanges, OnDestroy, AfterCont export declare class RouterLinkWithHref implements OnChanges, OnDestroy { fragment: string; href: string; + preserveFragment: boolean; + preserveQueryParams: boolean; queryParams: { [k: string]: any; }; routerLink: any[] | string; + routerLinkOptions: { + preserveQueryParams: boolean; + preserveFragment: boolean; + }; target: string; urlTree: UrlTree; ngOnChanges(changes: {}): any;