Skip to content

Commit

Permalink
fix(router): evaluate routerLinkActive state when routerLink changes (a…
Browse files Browse the repository at this point in the history 
…ngular#13865)

PR Close angular#13865
  • Loading branch information
@linnenschmidt
linnenschmidt committed Jun 14, 2019
1 parent 1a5c711 commit 09fd63c
Show file tree
Hide file tree
Showing 4 changed files with 169 additions and 9 deletions.
37 changes: 34 additions & 3 deletions packages/router/src/directives/router_link.ts
View file
Expand Up @@ -7,7 +7,7 @@
*/

import {LocationStrategy} from '@angular/common';
import {Attribute, Directive, ElementRef, HostBinding, HostListener, Input, OnChanges, OnDestroy, Renderer2, isDevMode} from '@angular/core';
import {Attribute, Directive, ElementRef, HostBinding, HostListener, Input, OnChanges, OnDestroy, Renderer2, SimpleChanges, isDevMode} from '@angular/core';
import {Subscription} from 'rxjs';

import {QueryParamsHandling} from '../config';
Expand All @@ -16,6 +16,7 @@ import {Router} from '../router';
import {ActivatedRoute} from '../router_state';
import {UrlTree} from '../url_tree';

import {RouterLinkActive} from './router_link_active';

/**
* @description
Expand Down Expand Up @@ -112,7 +113,7 @@ import {UrlTree} from '../url_tree';
* @publicApi
*/
@Directive({selector: ':not(a):not(area)[routerLink]'})
export class RouterLink {
export class RouterLink implements OnChanges {
// TODO(issue/24571): remove '!'.
@Input() queryParams !: {[k: string]: any};
// TODO(issue/24571): remove '!'.
Expand All @@ -129,6 +130,7 @@ export class RouterLink {
private commands: any[] = [];
// TODO(issue/24571): remove '!'.
private preserve !: boolean;
private routerLinkActives: RouterLinkActive[] = [];

constructor(
private router: Router, private route: ActivatedRoute,
Expand Down Expand Up @@ -178,6 +180,19 @@ export class RouterLink {
preserveFragment: attrBoolValue(this.preserveFragment),
});
}

ngOnChanges(changes: SimpleChanges): void { this.updateRouterLinkActives(); }

/** @internal */
addRouterLinkActive(routerLinkActive: RouterLinkActive) {
if (this.routerLinkActives.indexOf(routerLinkActive) === -1) {
this.routerLinkActives.push(routerLinkActive);
}
}

private updateRouterLinkActives() {
this.routerLinkActives.forEach(routerLinkActive => routerLinkActive.update());
}
}

/**
Expand Down Expand Up @@ -212,6 +227,7 @@ export class RouterLinkWithHref implements OnChanges, OnDestroy {
private subscription: Subscription;
// TODO(issue/24571): remove '!'.
private preserve !: boolean;
private routerLinkActives: RouterLinkActive[] = [];

// the url displayed on the anchor element.
// TODO(issue/24571): remove '!'.
Expand Down Expand Up @@ -244,7 +260,11 @@ export class RouterLinkWithHref implements OnChanges, OnDestroy {
this.preserve = value;
}

ngOnChanges(changes: {}): any { this.updateTargetUrlAndHref(); }
ngOnChanges(changes: SimpleChanges): any {
this.updateTargetUrlAndHref();
this.updateRouterLinkActives();
}

ngOnDestroy(): any { this.subscription.unsubscribe(); }

@HostListener('click', ['$event.button', '$event.ctrlKey', '$event.metaKey', '$event.shiftKey'])
Expand All @@ -266,6 +286,17 @@ export class RouterLinkWithHref implements OnChanges, OnDestroy {
return false;
}

/** @internal */
addRouterLinkActive(routerLinkActive: RouterLinkActive) {
if (this.routerLinkActives.indexOf(routerLinkActive) === -1) {
this.routerLinkActives.push(routerLinkActive);
}
}

private updateRouterLinkActives() {
this.routerLinkActives.forEach(routerLinkActive => routerLinkActive.update());
}

private updateTargetUrlAndHref(): void {
this.href = this.locationStrategy.prepareExternalUrl(this.router.serializeUrl(this.urlTree));
}
Expand Down
26 changes: 23 additions & 3 deletions packages/router/src/directives/router_link_active.ts
View file
Expand Up @@ -104,8 +104,17 @@ export class RouterLinkActive implements OnChanges,


ngAfterContentInit(): void {
this.links.changes.subscribe(_ => this.update());
this.linksWithHrefs.changes.subscribe(_ => this.update());
this.links.changes.subscribe(_ => {
this.connectWithContentChildrenLinks();
this.update();
});
this.linksWithHrefs.changes.subscribe(_ => {
this.connectWithContentChildrenLinks();
this.update();
});

this.connectWithInjectedLinks();
this.connectWithContentChildrenLinks();
this.update();
}

Expand All @@ -118,7 +127,8 @@ export class RouterLinkActive implements OnChanges,
ngOnChanges(changes: SimpleChanges): void { this.update(); }
ngOnDestroy(): void { this.subscription.unsubscribe(); }

private update(): void {
/** @internal */
update(): void {
if (!this.links || !this.linksWithHrefs || !this.router.navigated) return;
Promise.resolve().then(() => {
const hasActiveLinks = this.hasActiveLinks();
Expand All @@ -135,6 +145,16 @@ export class RouterLinkActive implements OnChanges,
});
}

private connectWithInjectedLinks() {
if (this.link) this.link.addRouterLinkActive(this);
if (this.linkWithHref) this.linkWithHref.addRouterLinkActive(this);
}

private connectWithContentChildrenLinks() {
this.links.forEach(link => link.addRouterLinkActive(this));
this.linksWithHrefs.forEach(linkWithHref => linkWithHref.addRouterLinkActive(this));
}

private isLinkActive(router: Router): (link: (RouterLink|RouterLinkWithHref)) => boolean {
return (link: RouterLink | RouterLinkWithHref) =>
router.isActive(link.urlTree, this.routerLinkActiveOptions.exact);
Expand Down
110 changes: 109 additions & 1 deletion packages/router/test/integration.spec.ts
View file
Expand Up @@ -13,7 +13,7 @@ import {ComponentFixture, TestBed, fakeAsync, inject, tick} from '@angular/core/
import {By} from '@angular/platform-browser/src/dom/debug/by';
import {expect} from '@angular/platform-browser/testing/src/matchers';
import {ActivatedRoute, ActivatedRouteSnapshot, ActivationEnd, ActivationStart, CanActivate, CanDeactivate, ChildActivationEnd, ChildActivationStart, DefaultUrlSerializer, DetachedRouteHandle, Event, GuardsCheckEnd, GuardsCheckStart, Navigation, NavigationCancel, NavigationEnd, NavigationError, NavigationStart, PRIMARY_OUTLET, ParamMap, Params, PreloadAllModules, PreloadingStrategy, Resolve, ResolveEnd, ResolveStart, RouteConfigLoadEnd, RouteConfigLoadStart, RouteReuseStrategy, Router, RouterEvent, RouterModule, RouterPreloader, RouterStateSnapshot, RoutesRecognized, RunGuardsAndResolvers, UrlHandlingStrategy, UrlSegmentGroup, UrlSerializer, UrlTree} from '@angular/router';
import {Observable, Observer, Subscription, of } from 'rxjs';
import {Observable, Observer, Subject, Subscription, of } from 'rxjs';
import {filter, first, map, tap} from 'rxjs/operators';

import {forEach} from '../src/utils/collection';
Expand Down Expand Up @@ -3700,6 +3700,78 @@ describe('Integration', () => {
expect(link.className).toEqual('active');
}));

it('should set the class when the routerLink changes asynchronously #13865',
fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
const fixture = createRoot(router, RootCmp);

router.resetConfig([{
path: 'team',
component: AsyncLinkCmp,
children: [{path: ':id', component: BlankCmp}]
}]);

router.navigateByUrl('/team/22');
advance(fixture);
advance(fixture);
expect(location.path()).toEqual('/team/22');

const asyncLinkCmp =
fixture.debugElement.query(By.directive(AsyncLinkCmp)).componentInstance;
asyncLinkCmp.subject.next(['/team/22']);
advance(fixture);
advance(fixture);

const link = fixture.nativeElement.querySelector('a');
const button = fixture.nativeElement.querySelector('button');
expect(link.className).toEqual('active');
expect(button.className).toEqual('active');

asyncLinkCmp.subject.next(['/team/21']);
advance(fixture);
advance(fixture);
expect(link.className).toEqual('');
expect(button.className).toEqual('');
})));


it('should set the class on a parent element when the routerLink changes asynchronously #13865',
fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
const fixture = createRoot(router, RootCmp);

router.resetConfig([{
path: 'team',
component: AsyncLinkWithParentCmp,
children: [{path: ':id', component: BlankCmp}]
}]);

router.navigateByUrl('/team/22');
advance(fixture);
advance(fixture);
expect(location.path()).toEqual('/team/22');

const asyncLinkCmp =
fixture.debugElement.query(By.directive(AsyncLinkWithParentCmp)).componentInstance;
asyncLinkCmp.subject.next(['/team/22']);
advance(fixture);
advance(fixture);

const linkOuter = fixture.nativeElement.querySelector('#link-outer-parent');
const link = fixture.nativeElement.querySelector('#link-parent');
const buttonOuter = fixture.nativeElement.querySelector('#button-outer-parent');
const button = fixture.nativeElement.querySelector('#button-parent');
expect(linkOuter.className).toEqual('active');
expect(link.className).toEqual('active');
expect(buttonOuter.className).toEqual('active');
expect(button.className).toEqual('active');

asyncLinkCmp.subject.next(['/team/21']);
advance(fixture);
advance(fixture);
expect(linkOuter.className).toEqual('');
expect(link.className).toEqual('');
expect(buttonOuter.className).toEqual('');
expect(button.className).toEqual('');
})));

it('should set the class on a parent element when the link is active',
fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
Expand Down Expand Up @@ -5035,6 +5107,36 @@ class OutletInNgIf {
alwaysTrue = true;
}

@Component({
selector: 'async-link-cmp',
template: `<router-outlet></router-outlet>
<a [routerLink]="observable$ | async" routerLinkActive="active">link</a>
<button [routerLink]="observable$ | async" routerLinkActive="active">button</button>`
})
class AsyncLinkCmp {
subject = new Subject<string>();
observable$ = this.subject.asObservable();
}

@Component({
selector: 'async-link-with-parent-cmp',
template: `<router-outlet></router-outlet>
<div id="link-outer-parent" routerLinkActive="active">
<div id="link-parent" routerLinkActive="active">
<a [routerLink]="observable$ | async">link</a>
</div>
</div>
<div id="button-outer-parent" routerLinkActive="active">
<div id="button-parent" routerLinkActive="active">
<button [routerLink]="observable$ | async">button</button>
</div>
</div>`
})
class AsyncLinkWithParentCmp {
subject = new Subject<string>();
observable$ = this.subject.asObservable();
}

@Component({
selector: 'link-cmp',
template: `<router-outlet></router-outlet>
Expand Down Expand Up @@ -5119,6 +5221,8 @@ class LazyComponent {
AbsoluteLinkCmp,
AbsoluteSimpleLinkCmp,
RelativeLinkCmp,
AsyncLinkCmp,
AsyncLinkWithParentCmp,
DummyLinkWithParentCmp,
LinkWithQueryParamsAndFragment,
LinkWithState,
Expand Down Expand Up @@ -5149,6 +5253,8 @@ class LazyComponent {
AbsoluteLinkCmp,
AbsoluteSimpleLinkCmp,
RelativeLinkCmp,
AsyncLinkCmp,
AsyncLinkWithParentCmp,
DummyLinkWithParentCmp,
LinkWithQueryParamsAndFragment,
LinkWithState,
Expand Down Expand Up @@ -5181,6 +5287,8 @@ class LazyComponent {
AbsoluteLinkCmp,
AbsoluteSimpleLinkCmp,
RelativeLinkCmp,
AsyncLinkCmp,
AsyncLinkWithParentCmp,
DummyLinkWithParentCmp,
LinkWithQueryParamsAndFragment,
LinkWithState,
Expand Down
5 changes: 3 additions & 2 deletions tools/public_api_guard/router/router.d.ts
View file
Expand Up @@ -366,7 +366,7 @@ export declare class RouterEvent {
url: string);
}

export declare class RouterLink {
export declare class RouterLink implements OnChanges {
fragment: string;
preserveFragment: boolean;
/** @deprecated */ preserveQueryParams: boolean;
Expand All @@ -382,6 +382,7 @@ export declare class RouterLink {
};
readonly urlTree: UrlTree;
constructor(router: Router, route: ActivatedRoute, tabIndex: string, renderer: Renderer2, el: ElementRef);
ngOnChanges(changes: SimpleChanges): void;
onClick(): boolean;
}

Expand Down Expand Up @@ -417,7 +418,7 @@ export declare class RouterLinkWithHref implements OnChanges, OnDestroy {
target: string;
readonly urlTree: UrlTree;
constructor(router: Router, route: ActivatedRoute, locationStrategy: LocationStrategy);
ngOnChanges(changes: {}): any;
ngOnChanges(changes: SimpleChanges): any;
ngOnDestroy(): any;
onClick(button: number, ctrlKey: boolean, metaKey: boolean, shiftKey: boolean): boolean;
}
Expand Down

0 comments on commit 09fd63c

Please sign in to comment.