Skip to content

Commit

Permalink
fix(router): routerLinkActive should not throw when not initialized (a…
Browse files Browse the repository at this point in the history
  • Loading branch information
Dzmitry Shylovich authored and juleskremer committed Aug 24, 2017
1 parent a09cbbf commit 75b1c2d
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 21 deletions.
39 changes: 23 additions & 16 deletions modules/@angular/router/src/directives/router_link_active.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,15 @@
* found in the LICENSE file at https://angular.io/license
*/

import {AfterContentInit, ChangeDetectorRef, ContentChildren, Directive, ElementRef, Input, OnChanges, OnDestroy, QueryList, Renderer} from '@angular/core';
import {AfterContentInit, ChangeDetectorRef, ContentChildren, Directive, ElementRef, Input, OnChanges, OnDestroy, QueryList, Renderer, SimpleChanges} from '@angular/core';
import {Subscription} from 'rxjs/Subscription';

import {NavigationEnd, Router} from '../router';

import {RouterLink, RouterLinkWithHref} from './router_link';



/**
* @whatItDoes Lets you add a CSS class to an element when the link's route becomes active.
*
Expand Down Expand Up @@ -82,25 +83,28 @@ import {RouterLink, RouterLinkWithHref} from './router_link';
exportAs: 'routerLinkActive',
})
export class RouterLinkActive implements OnChanges,
OnDestroy, AfterContentInit {
OnDestroy, AfterContentInit {
@ContentChildren(RouterLink, {descendants: true}) links: QueryList<RouterLink>;
@ContentChildren(RouterLinkWithHref, {descendants: true})
linksWithHrefs: QueryList<RouterLinkWithHref>;

private classes: string[] = [];
private subscription: Subscription;
private active: boolean = false;

@Input() routerLinkActiveOptions: {exact: boolean} = {exact: false};

constructor(private router: Router, private element: ElementRef, private renderer: Renderer) {
constructor(
private router: Router, private element: ElementRef, private renderer: Renderer,
private cdr: ChangeDetectorRef) {
this.subscription = router.events.subscribe(s => {
if (s instanceof NavigationEnd) {
this.update();
}
});
}

get isActive(): boolean { return this.hasActiveLink(); }
get isActive(): boolean { return this.active; }

ngAfterContentInit(): void {
this.links.changes.subscribe(_ => this.update());
Expand All @@ -110,30 +114,33 @@ export class RouterLinkActive implements OnChanges,

@Input()
set routerLinkActive(data: string[]|string) {
this.classes = (Array.isArray(data) ? data : data.split(' ')).filter(c => !!c);
const classes = Array.isArray(data) ? data : data.split(' ');
this.classes = classes.filter(c => !!c);
}

ngOnChanges(changes: {}): void { this.update(); }
ngOnChanges(changes: SimpleChanges): void { this.update(); }
ngOnDestroy(): void { this.subscription.unsubscribe(); }

private update(): void {
if (!this.links || !this.linksWithHrefs || !this.router.navigated) return;

const isActive = this.hasActiveLink();
this.classes.forEach(c => {
if (c) {
this.renderer.setElementClass(this.element.nativeElement, c, isActive);
}
});
const hasActiveLinks = this.hasActiveLinks();

// react only when status has changed to prevent unnecessary dom updates
if (this.active !== hasActiveLinks) {
this.active = hasActiveLinks;
this.classes.forEach(
c => this.renderer.setElementClass(this.element.nativeElement, c, hasActiveLinks));
this.cdr.detectChanges();
}
}

private isLinkActive(router: Router): (link: (RouterLink|RouterLinkWithHref)) => boolean {
return (link: RouterLink | RouterLinkWithHref) =>
router.isActive(link.urlTree, this.routerLinkActiveOptions.exact);
router.isActive(link.urlTree, this.routerLinkActiveOptions.exact);
}

private hasActiveLink(): boolean {
private hasActiveLinks(): boolean {
return this.links.some(this.isLinkActive(this.router)) ||
this.linksWithHrefs.some(this.isLinkActive(this.router));
this.linksWithHrefs.some(this.isLinkActive(this.router));
}
}
10 changes: 6 additions & 4 deletions modules/@angular/router/test/integration.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2096,6 +2096,8 @@ describe('Integration', () => {
@Component({
template: `<a routerLink="/team" routerLinkActive #rla="routerLinkActive"></a>
<p>{{rla.isActive}}</p>
<span *ngIf="rla.isActive"></span>
<span [ngClass]="{'highlight': rla.isActive}"></span>
<router-outlet></router-outlet>`
})
class ComponentWithRouterLink {
Expand All @@ -2115,15 +2117,15 @@ describe('Integration', () => {
}
]);

const f = TestBed.createComponent(ComponentWithRouterLink);
const fixture = TestBed.createComponent(ComponentWithRouterLink);
router.navigateByUrl('/team');
advance(f);
expect(() => advance(fixture)).not.toThrow();

const paragraph = f.nativeElement.querySelector('p');
const paragraph = fixture.nativeElement.querySelector('p');
expect(paragraph.textContent).toEqual('true');

router.navigateByUrl('/otherteam');
advance(f);
advance(fixture);

expect(paragraph.textContent).toEqual('false');
}));
Expand Down
2 changes: 1 addition & 1 deletion tools/public_api_guard/router/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ export declare class RouterLinkActive implements OnChanges, OnDestroy, AfterCont
};
constructor(router: Router, element: ElementRef, renderer: Renderer, cdr: ChangeDetectorRef);
ngAfterContentInit(): void;
ngOnChanges(changes: {}): void;
ngOnChanges(changes: SimpleChanges): void;
ngOnDestroy(): void;
}

Expand Down

0 comments on commit 75b1c2d

Please sign in to comment.