From 77d1fc149a0eb34bc872b8444c763bd05466f53c Mon Sep 17 00:00:00 2001 From: Rado Kirov Date: Wed, 6 May 2015 18:30:37 -0700 Subject: [PATCH] fix(router): router-link works without params Router-link attaches a listener to prevent default behavior and navigate. Closes: 1689 --- modules/angular2/src/dom/parse5_adapter.cjs | 7 +++- .../angular2/src/router/path_recognizer.js | 8 +++- modules/angular2/src/router/router_link.js | 20 +++++++--- modules/angular2/src/router/router_outlet.js | 2 +- modules/angular2/test/router/outlet_spec.js | 38 +++++++++++++++++-- .../test/router/route_recognizer_spec.js | 6 +++ 6 files changed, 68 insertions(+), 13 deletions(-) diff --git a/modules/angular2/src/dom/parse5_adapter.cjs b/modules/angular2/src/dom/parse5_adapter.cjs index 28f9642237..371b144fe1 100644 --- a/modules/angular2/src/dom/parse5_adapter.cjs +++ b/modules/angular2/src/dom/parse5_adapter.cjs @@ -127,7 +127,12 @@ export class Parse5DomAdapter extends DomAdapter { return this.createEvent(eventType); } createEvent(eventType) { - return {type: eventType}; + var evt = { + type: eventType, + defaultPrevented: false, + preventDefault: () => {evt.defaultPrevented = true} + }; + return evt; } getInnerHTML(el) { return serializer.serialize(this.templateAwareRoot(el)); diff --git a/modules/angular2/src/router/path_recognizer.js b/modules/angular2/src/router/path_recognizer.js index dbd917a752..6e9ea101aa 100644 --- a/modules/angular2/src/router/path_recognizer.js +++ b/modules/angular2/src/router/path_recognizer.js @@ -1,4 +1,4 @@ -import {RegExp, RegExpWrapper, RegExpMatcherWrapper, StringWrapper, isPresent} from 'angular2/src/facade/lang'; +import {RegExp, RegExpWrapper, RegExpMatcherWrapper, StringWrapper, isPresent, isBlank, BaseException} from 'angular2/src/facade/lang'; import {Map, MapWrapper, StringMap, StringMapWrapper, List, ListWrapper} from 'angular2/src/facade/collection'; import {escapeRegex} from './url'; @@ -27,6 +27,9 @@ class DynamicSegment { } generate(params:StringMap) { + if (!StringMapWrapper.contains(params, this.name)) { + throw new BaseException(`Route generator for '${this.name}' was not included in parameters passed.`) + } return StringMapWrapper.get(params, this.name); } } @@ -118,6 +121,7 @@ export class PathRecognizer { } generate(params:StringMap):string { - return ListWrapper.join(ListWrapper.map(this.segments, (segment) => '/' + segment.generate(params)), ''); + return ListWrapper.join(ListWrapper.map(this.segments, (segment) => + '/' + segment.generate(params)), ''); } } diff --git a/modules/angular2/src/router/router_link.js b/modules/angular2/src/router/router_link.js index ca8ab4068a..b959133214 100644 --- a/modules/angular2/src/router/router_link.js +++ b/modules/angular2/src/router/router_link.js @@ -1,5 +1,6 @@ -import {Directive} from 'angular2/src/core/annotations_impl/annotations'; +import {Directive, onAllChangesDone} from 'angular2/src/core/annotations_impl/annotations'; import {ElementRef} from 'angular2/core'; +import {StringMap, StringMapWrapper} from 'angular2/src/facade/collection'; import {isPresent} from 'angular2/src/facade/lang'; import {DOM} from 'angular2/src/dom/dom_adapter'; @@ -32,33 +33,40 @@ import {Router} from './router'; properties: { 'route': 'routerLink', 'params': 'routerParams' - } + }, + lifecycle: [onAllChangesDone] }) export class RouterLink { _domEl; _route:string; _params:any; _router:Router; - //TODO: handle click events + _href:string; constructor(elementRef:ElementRef, router:Router) { this._domEl = elementRef.domElement; this._router = router; + this._params = StringMapWrapper.create(); + DOM.on(this._domEl, 'click', (evt) => { + evt.preventDefault(); + this._router.navigate(this._href); + }); } set route(changes) { this._route = changes; - this.updateHref(); } set params(changes) { this._params = changes; - this.updateHref(); } - updateHref() { + onAllChangesDone() { if (isPresent(this._route) && isPresent(this._params)) { var newHref = this._router.generate(this._route, this._params); + this._href = newHref; + // Keeping the link on the element to support contextual menu `copy link` + // and other in-browser affordances. DOM.setAttribute(this._domEl, 'href', newHref); } } diff --git a/modules/angular2/src/router/router_outlet.js b/modules/angular2/src/router/router_outlet.js index 56d93a7f1c..448aa6f24e 100644 --- a/modules/angular2/src/router/router_outlet.js +++ b/modules/angular2/src/router/router_outlet.js @@ -18,7 +18,7 @@ export class RouterOutlet { _router:routerMod.Router; _viewContainer:ViewContainerRef; - constructor(viewContainer:ViewContainerRef, compiler:Compiler, router:routerMod.Router, injector:Injector, @Attribute('name') nameAttr) { + constructor(viewContainer:ViewContainerRef, compiler:Compiler, router:routerMod.Router, injector:Injector, @Attribute('name') nameAttr:String) { if (isBlank(nameAttr)) { nameAttr = 'default'; } diff --git a/modules/angular2/test/router/outlet_spec.js b/modules/angular2/test/router/outlet_spec.js index a7689f4745..6e95f18bbb 100644 --- a/modules/angular2/test/router/outlet_spec.js +++ b/modules/angular2/test/router/outlet_spec.js @@ -34,7 +34,7 @@ import {DirectiveMetadataReader} from 'angular2/src/core/compiler/directive_meta export function main() { describe('Outlet Directive', () => { - var ctx, tb, view, rtr; + var ctx, tb, view, rtr, location; beforeEachBindings(() => [ Pipeline, @@ -46,10 +46,11 @@ export function main() { }, [RouteRegistry, Pipeline, Location]) ]); - beforeEach(inject([TestBed, Router], (testBed, router) => { + beforeEach(inject([TestBed, Router, Location], (testBed, router, loc) => { tb = testBed; ctx = new MyComp(); rtr = router; + location = loc; })); function compile(template:string = "") { @@ -131,8 +132,18 @@ export function main() { }); })); + it('should generate link hrefs without params', inject([AsyncTestCompleter], (async) => { + compile('') + .then((_) => rtr.config({'path': '/user', 'component': UserCmp, 'as': 'user'})) + .then((_) => rtr.navigate('/a/b')) + .then((_) => { + view.detectChanges(); + expect(DOM.getAttribute(view.rootNodes[0].childNodes[0], 'href')).toEqual('/user'); + async.done(); + }); + })); - it('should generate link hrefs', inject([AsyncTestCompleter], (async) => { + it('should generate link hrefs with params', inject([AsyncTestCompleter], (async) => { ctx.name = 'brian'; compile('{{name}}') .then((_) => rtr.config({'path': '/user/:name', 'component': UserCmp, 'as': 'user'})) @@ -145,6 +156,27 @@ export function main() { }); })); + + it('should generate link hrefs without params', inject([AsyncTestCompleter], (async) => { + compile('') + .then((_) => rtr.config({'path': '/user', 'component': UserCmp, 'as': 'user'})) + .then((_) => rtr.navigate('/a/b')) + .then((_) => { + view.detectChanges(); + var anchorEl = view.rootNodes[0].childNodes[0]; + expect(DOM.getAttribute(anchorEl, 'href')).toEqual('/user'); + + var dispatchedEvent = DOM.createMouseEvent('click'); + DOM.dispatchEvent(anchorEl, dispatchedEvent); + expect(dispatchedEvent.defaultPrevented).toBe(true); + + // router navigation is async. + rtr.subscribe((_) => { + expect(location.urlChanges).toEqual(['/user']); + async.done(); + }); + }); + })); }); } diff --git a/modules/angular2/test/router/route_recognizer_spec.js b/modules/angular2/test/router/route_recognizer_spec.js index 6cb84bc605..a35f54050a 100644 --- a/modules/angular2/test/router/route_recognizer_spec.js +++ b/modules/angular2/test/router/route_recognizer_spec.js @@ -68,5 +68,11 @@ export function main() { recognizer.addConfig('/app/user/:name', handler, 'user'); expect(recognizer.generate('user', {'name' : 'misko'})).toEqual('/app/user/misko'); }); + + it('should throw in the absence of required params URLs', () => { + recognizer.addConfig('/app/user/:name', handler, 'user'); + expect(() => recognizer.generate('user', {})).toThrowError( + 'Route generator for \'name\' was not included in parameters passed.'); + }); }); }