fix(RouterLink): do not prevent default behavior if target set on anchor element

If the anchor element on which the "router-link" directive is present has a target
attribute other than "_self," the handler will not prevent default behavior of
the browser.

Closes #4233
Closes #5082
This commit is contained in:
Jeff Cross 2015-11-02 15:11:57 -08:00
parent a9b1270a5a
commit a69e7fe297
4 changed files with 71 additions and 7 deletions

View File

@ -1,5 +1,5 @@
import {Directive} from '../core/metadata'; import {Directive} from '../core/metadata';
import {StringMapWrapper} from 'angular2/src/core/facade/collection'; import {isString} from 'angular2/src/core/facade/lang';
import {Router} from './router'; import {Router} from './router';
import {Location} from './location'; import {Location} from './location';
@ -36,7 +36,7 @@ import {Instruction, stringifyInstruction} from './instruction';
*/ */
@Directive({ @Directive({
selector: '[router-link]', selector: '[router-link]',
inputs: ['routeParams: routerLink'], inputs: ['routeParams: routerLink', 'target: target'],
host: { host: {
'(click)': 'onClick()', '(click)': 'onClick()',
'[attr.href]': 'visibleHref', '[attr.href]': 'visibleHref',
@ -48,6 +48,7 @@ export class RouterLink {
// the url displayed on the anchor element. // the url displayed on the anchor element.
visibleHref: string; visibleHref: string;
target: string;
// the instruction passed to the router to navigate // the instruction passed to the router to navigate
private _navigationInstruction: Instruction; private _navigationInstruction: Instruction;
@ -65,7 +66,11 @@ export class RouterLink {
} }
onClick(): boolean { onClick(): boolean {
this._router.navigateByInstruction(this._navigationInstruction); // If no target, or if target is _self, prevent default browser behavior
return false; if (!isString(this.target) || this.target == '_self') {
this._router.navigateByInstruction(this._navigationInstruction);
return false;
}
return true;
} }
} }

View File

@ -56,7 +56,8 @@ export function main() {
tcb.createAsync(TestComponent) tcb.createAsync(TestComponent)
.then((testComponent) => { .then((testComponent) => {
testComponent.detectChanges(); testComponent.detectChanges();
let anchorElement = testComponent.debugElement.query(By.css('a')).nativeElement; let anchorElement =
testComponent.debugElement.query(By.css('a.detail-view')).nativeElement;
expect(DOM.getAttribute(anchorElement, 'href')).toEqual('detail'); expect(DOM.getAttribute(anchorElement, 'href')).toEqual('detail');
async.done(); async.done();
}); });
@ -70,11 +71,38 @@ export function main() {
.then((testComponent) => { .then((testComponent) => {
testComponent.detectChanges(); testComponent.detectChanges();
// TODO: shouldn't this be just 'click' rather than '^click'? // TODO: shouldn't this be just 'click' rather than '^click'?
testComponent.debugElement.query(By.css('a')).triggerEventHandler('click', null); testComponent.debugElement.query(By.css('a.detail-view'))
.triggerEventHandler('click', null);
expect(router.spy('navigateByInstruction')).toHaveBeenCalledWith(dummyInstruction); expect(router.spy('navigateByInstruction')).toHaveBeenCalledWith(dummyInstruction);
async.done(); async.done();
}); });
})); }));
it('should call router.navigate when a link is clicked if target is _self',
inject([AsyncTestCompleter, Router], (async, router) => {
tcb.createAsync(TestComponent)
.then((testComponent) => {
testComponent.detectChanges();
testComponent.debugElement.query(By.css('a.detail-view-self'))
.triggerEventHandler('click', null);
expect(router.spy('navigateByInstruction')).toHaveBeenCalledWith(dummyInstruction);
async.done();
});
}));
it('should NOT call router.navigate when a link is clicked if target is set to other than _self',
inject([AsyncTestCompleter, Router], (async, router) => {
tcb.createAsync(TestComponent)
.then((testComponent) => {
testComponent.detectChanges();
testComponent.debugElement.query(By.css('a.detail-view-blank'))
.triggerEventHandler('click', null);
expect(router.spy('navigateByInstruction')).not.toHaveBeenCalled();
async.done();
});
}));
}); });
} }
@ -94,7 +122,20 @@ class UserCmp {
@View({ @View({
template: ` template: `
<div> <div>
<a [router-link]="['/Detail']">detail view</a> <a [router-link]="['/Detail']"
class="detail-view">
detail view
</a>
<a [router-link]="['/Detail']"
class="detail-view-self"
target="_self">
detail view with _self target
</a>
<a [router-link]="['/Detail']"
class="detail-view-blank"
target="_blank">
detail view with _blank target
</a>
</div>`, </div>`,
directives: [RouterLink] directives: [RouterLink]
}) })

View File

@ -26,4 +26,19 @@ describe('hash routing example app', function() {
expect(element(by.css('goodbye-cmp')).getText()).toContain('goodbye'); expect(element(by.css('goodbye-cmp')).getText()).toContain('goodbye');
}); });
it('should open in new window if target is _blank', () => {
var URL = 'playground/src/hash_routing/index.html';
browser.get(URL + '#/');
waitForElement('hello-cmp');
element(by.css('#goodbye-link-blank')).click();
expect(browser.driver.getCurrentUrl()).not.toContain('#/bye');
browser.getAllWindowHandles().then(function(windows) {
browser.switchTo()
.window(windows[1])
.then(function() { expect(browser.driver.getCurrentUrl()).toContain("#/bye"); });
});
});
}); });

View File

@ -29,6 +29,9 @@ class GoodByeCmp {
<nav> <nav>
<a href="#/" id="hello-link">Navigate via href</a> | <a href="#/" id="hello-link">Navigate via href</a> |
<a [router-link]="['/GoodbyeCmp']" id="goodbye-link">Navigate with Link DSL</a> <a [router-link]="['/GoodbyeCmp']" id="goodbye-link">Navigate with Link DSL</a>
<a [router-link]="['/GoodbyeCmp']" id="goodbye-link-blank" target="_blank">
Navigate with Link DSL _blank target
</a>
</nav> </nav>
<router-outlet></router-outlet> <router-outlet></router-outlet>
`, `,