Skip to content

Commit

Permalink
fix(router): openning links in new tab
Browse files Browse the repository at this point in the history
Clicks on router-link should not prevent browser default action when
any mouse button other than left mouse button or ctrl/meta key is pressed.

router-link href should use location strategy external url.

Closes #5908
Closes #6806
Closes #7749
Closes #8806

Closes #8821
  • Loading branch information
dloukadakis authored and mhevery committed May 26, 2016
1 parent 172a566 commit fa2ce81
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 16 deletions.
24 changes: 15 additions & 9 deletions modules/@angular/router/src/directives/router_link.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {Router} from '../router';
import {RouteSegment} from '../segments';
import {isString, isArray, isPresent} from '../facade/lang';
import {ObservableWrapper} from '../facade/async';
import {LocationStrategy} from '@angular/common';

/**
* The RouterLink directive lets you link to specific parts of your app.
Expand Down Expand Up @@ -44,7 +45,8 @@ export class RouterLink implements OnDestroy {
@HostBinding() href: string;
@HostBinding('class.router-link-active') isActive: boolean = false;

constructor(private _routeSegment: RouteSegment, private _router: Router) {
constructor(private _routeSegment: RouteSegment, private _router: Router,
private _locationStrategy: LocationStrategy) {
// because auxiliary links take existing primary and auxiliary routes into account,
// we need to update the link whenever params or other routes change.
this._subscription =
Expand All @@ -64,20 +66,24 @@ export class RouterLink implements OnDestroy {
}


@HostListener("click")
onClick(): boolean {
// If no target, or if target is _self, prevent default browser behavior
if (!isString(this.target) || this.target == '_self') {
this._router.navigate(this._commands, this._routeSegment);
return false;
@HostListener("click", ["$event.button", "$event.ctrlKey", "$event.metaKey"])
onClick(button: number, ctrlKey: boolean, metaKey: boolean): boolean {
if (button != 0 || ctrlKey || metaKey) {
return true;
}
return true;

if (isString(this.target) && this.target != '_self') {
return true;
}

this._router.navigate(this._commands, this._routeSegment);
return false;
}

private _updateTargetUrlAndHref(): void {
let tree = this._router.createUrlTree(this._commands, this._routeSegment);
if (isPresent(tree)) {
this.href = this._router.serializeUrl(tree);
this.href = this._locationStrategy.prepareExternalUrl(this._router.serializeUrl(tree));
this.isActive = this._router.urlTree.contains(tree);
} else {
this.isActive = false;
Expand Down
15 changes: 8 additions & 7 deletions modules/@angular/router/test/router_link_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,16 @@ import {
OnActivate,
CanDeactivate
} from '@angular/router';
import {Location} from '@angular/common';
import {SpyLocation} from '@angular/common/testing';
import {Location, LocationStrategy} from '@angular/common';
import {SpyLocation, MockLocationStrategy} from '@angular/common/testing';

export function main() {
describe('RouterLink', () => {
beforeEachProviders(() => [
provide(RouterUrlSerializer, {useClass: DefaultRouterUrlSerializer}),
RouterOutletMap,
provide(Location, {useClass: SpyLocation}),
provide(LocationStrategy, {useClass: MockLocationStrategy}),
provide(Router,
{
useFactory: (resolver, urlParser, outletMap, location) => new Router(
Expand All @@ -44,14 +45,14 @@ export function main() {
]);

describe("routerLink=", () => {
it("should accept an array of commands", inject([Router], (router) => {
let link = new RouterLink(null, router);
it("should accept an array of commands", inject([Router, LocationStrategy], (router, locationStrategy) => {
let link = new RouterLink(null, router, locationStrategy);
link.routerLink = ['/one', 11];
expect(link.href).toEqual("/one/11");
}));

it("should accept a single command", inject([Router], (router) => {
let link = new RouterLink(null, router);
it("should accept a single command", inject([Router, LocationStrategy], (router, locationStrategy) => {
let link = new RouterLink(null, router, locationStrategy);
link.routerLink = '/one/11';
expect(link.href).toEqual("/one/11");
}));
Expand All @@ -61,4 +62,4 @@ export function main() {

@Component({template: ''})
class RootCmp {
}
}

0 comments on commit fa2ce81

Please sign in to comment.