fix(angular_1_router): ng-link is generating wrong hrefs

At the moment ng-link is generating html5mode URLs for `href`s.
Instead it should check whether or not html5mode is enabled and create
the `href`s accordingly. The renaming in the `getLink` function is
aligning it to `RouterLink`'s `_updateLink`.

Closes #7423
This commit is contained in:
David Reher 2016-03-04 09:14:27 +01:00 committed by Pete Bacon Darwin
parent 980491b08f
commit 69c1405196
3 changed files with 103 additions and 102 deletions

View File

@ -316,3 +316,13 @@ Location.prototype.path = function () {
Location.prototype.go = function (path, query) { Location.prototype.go = function (path, query) {
return $location.url(path + query); return $location.url(path + query);
}; };
Location.prototype.prepareExternalUrl = function(url) {
if (url.length > 0 && !url.startsWith('/')) {
url = '/' + url;
}
if(!$location.$$html5) {
return '#' + url;
} else {
return '.' + url;
}
};

View File

@ -207,13 +207,13 @@ function ngLinkDirective($rootRouter, $parse) {
return; return;
} }
let instruction = null; let navigationInstruction = null;
let link = attrs.ngLink || ''; let link = attrs.ngLink || '';
function getLink(params) { function getLink(params) {
instruction = router.generate(params); navigationInstruction = router.generate(params);
scope.$watch(function() { return router.isRouteActive(instruction); }, scope.$watch(function() { return router.isRouteActive(navigationInstruction); },
function(active) { function(active) {
if (active) { if (active) {
element.addClass('ng-link-active'); element.addClass('ng-link-active');
@ -222,7 +222,8 @@ function ngLinkDirective($rootRouter, $parse) {
} }
}); });
return './' + angular.stringifyInstruction(instruction); const navigationHref = navigationInstruction.toLinkUrl();
return $rootRouter._location.prepareExternalUrl(navigationHref);
} }
let routeParamsGetter = $parse(link); let routeParamsGetter = $parse(link);
@ -236,11 +237,11 @@ function ngLinkDirective($rootRouter, $parse) {
} }
element.on('click', event => { element.on('click', event => {
if (event.which !== 1 || !instruction) { if (event.which !== 1 || !navigationInstruction) {
return; return;
} }
$rootRouter.navigateByInstruction(instruction); $rootRouter.navigateByInstruction(navigationInstruction);
event.preventDefault(); event.preventDefault();
}); });
} }

View File

@ -2,183 +2,173 @@
describe('ngLink', function () { describe('ngLink', function () {
var elt,
$compile,
$rootScope,
$rootRouter,
$compileProvider;
beforeEach(function () {
module('ng');
module('ngComponentRouter');
module(function (_$compileProvider_) {
$compileProvider = _$compileProvider_;
});
inject(function (_$compile_, _$rootScope_, _$rootRouter_) {
$compile = _$compile_;
$rootScope = _$rootScope_;
$rootRouter = _$rootRouter_;
});
registerComponent('userCmp', '<div>hello {{userCmp.$routeParams.name}}</div>', function () {});
registerComponent('oneCmp', '<div>{{oneCmp.number}}</div>', function () {this.number = 'one'});
registerComponent('twoCmp', '<div><a ng-link="[\'/Two\']">{{twoCmp.number}}</a></div>', function () {this.number = 'two'});
});
it('should allow linking from the parent to the child', function () { it('should allow linking from the parent to the child', function () {
$rootRouter.config([ setup();
configureRouter([
{ path: '/a', component: 'oneCmp' }, { path: '/a', component: 'oneCmp' },
{ path: '/b', component: 'twoCmp', name: 'Two' } { path: '/b', component: 'twoCmp', name: 'Two' }
]); ]);
compile('<a ng-link="[\'/Two\']">link</a> | outer { <div ng-outlet></div> }');
$rootRouter.navigateByUrl('/a');
$rootScope.$digest();
var elt = compile('<a ng-link="[\'/Two\']">link</a> | outer { <div ng-outlet></div> }');
navigateTo('/a');
expect(elt.find('a').attr('href')).toBe('./b'); expect(elt.find('a').attr('href')).toBe('./b');
}); });
it('should allow linking from the child and the parent', function () { it('should allow linking from the child and the parent', function () {
$rootRouter.config([ setup();
configureRouter([
{ path: '/a', component: 'oneCmp' }, { path: '/a', component: 'oneCmp' },
{ path: '/b', component: 'twoCmp', name: 'Two' } { path: '/b', component: 'twoCmp', name: 'Two' }
]); ]);
compile('outer { <div ng-outlet></div> }');
$rootRouter.navigateByUrl('/b');
$rootScope.$digest();
var elt = compile('outer { <div ng-outlet></div> }');
navigateTo('/b');
expect(elt.find('a').attr('href')).toBe('./b'); expect(elt.find('a').attr('href')).toBe('./b');
}); });
it('should allow params in routerLink directive', function () { it('should allow params in routerLink directive', function () {
setup();
registerComponent('twoLinkCmp', '<div><a ng-link="[\'/Two\', {param: \'lol\'}]">{{twoLinkCmp.number}}</a></div>', function () {this.number = 'two'}); registerComponent('twoLinkCmp', '<div><a ng-link="[\'/Two\', {param: \'lol\'}]">{{twoLinkCmp.number}}</a></div>', function () {this.number = 'two'});
configureRouter([
$rootRouter.config([
{ path: '/a', component: 'twoLinkCmp' }, { path: '/a', component: 'twoLinkCmp' },
{ path: '/b/:param', component: 'twoCmp', name: 'Two' } { path: '/b/:param', component: 'twoCmp', name: 'Two' }
]); ]);
compile('<div ng-outlet></div>');
$rootRouter.navigateByUrl('/a');
$rootScope.$digest();
var elt = compile('<div ng-outlet></div>');
navigateTo('/a');
expect(elt.find('a').attr('href')).toBe('./b/lol'); expect(elt.find('a').attr('href')).toBe('./b/lol');
}); });
it('should update the href of links with bound params', function () { it('should update the href of links with bound params', function () {
registerComponent('twoLinkCmp', '<div><a ng-link="[\'/Two\', {param: twoLinkCmp.number}]">{{twoLinkCmp.number}}</a></div>', function () {this.number = 'param'}); setup();
$rootRouter.config([ registerComponent('twoLinkCmp', '<div><a ng-link="[\'/Two\', {param: $ctrl.number}]">{{$ctrl.number}}</a></div>', function () {this.number = 43});
configureRouter([
{ path: '/a', component: 'twoLinkCmp' }, { path: '/a', component: 'twoLinkCmp' },
{ path: '/b/:param', component: 'twoCmp', name: 'Two' } { path: '/b/:param', component: 'twoCmp', name: 'Two' }
]); ]);
compile('<div ng-outlet></div>');
$rootRouter.navigateByUrl('/a'); var elt = compile('<div ng-outlet></div>');
$rootScope.$digest(); navigateTo('/a');
expect(elt.find('a').text()).toBe('43');
expect(elt.find('a').attr('href')).toBe('./b/param'); expect(elt.find('a').attr('href')).toBe('./b/43');
}); });
it('should navigate on left-mouse click when a link url matches a route', function () { it('should navigate on left-mouse click when a link url matches a route', function () {
$rootRouter.config([ setup();
configureRouter([
{ path: '/', component: 'oneCmp' }, { path: '/', component: 'oneCmp' },
{ path: '/two', component: 'twoCmp', name: 'Two'} { path: '/two', component: 'twoCmp', name: 'Two'}
]); ]);
compile('<a ng-link="[\'/Two\']">link</a> | <div ng-outlet></div>'); var elt = compile('<a ng-link="[\'/Two\']">link</a> | <div ng-outlet></div>');
$rootScope.$digest();
expect(elt.text()).toBe('link | one'); expect(elt.text()).toBe('link | one');
expect(elt.find('a').attr('href')).toBe('./two'); expect(elt.find('a').attr('href')).toBe('./two');
elt.find('a')[0].click(); elt.find('a')[0].click();
inject(function($rootScope) { $rootScope.$digest(); });
$rootScope.$digest();
expect(elt.text()).toBe('link | two'); expect(elt.text()).toBe('link | two');
}); });
it('should not navigate on non-left mouse click when a link url matches a route', inject(function ($rootRouter) { it('should not navigate on non-left mouse click when a link url matches a route', function() {
$rootRouter.config([ setup();
configureRouter([
{ path: '/', component: 'oneCmp' }, { path: '/', component: 'oneCmp' },
{ path: '/two', component: 'twoCmp', name: 'Two'} { path: '/two', component: 'twoCmp', name: 'Two'}
]); ]);
compile('<a ng-link="[\'/Two\']">link</a> | <div ng-outlet></div>'); var elt = compile('<a ng-link="[\'/Two\']">link</a> | <div ng-outlet></div>');
$rootScope.$digest();
expect(elt.text()).toBe('link | one'); expect(elt.text()).toBe('link | one');
elt.find('a').triggerHandler({ type: 'click', which: 3 }); elt.find('a').triggerHandler({ type: 'click', which: 3 });
inject(function($rootScope) { $rootScope.$digest(); });
$rootScope.$digest();
expect(elt.text()).toBe('link | one'); expect(elt.text()).toBe('link | one');
})); });
// See https://github.com/angular/router/issues/206 // See https://github.com/angular/router/issues/206
it('should not navigate a link without an href', function () { it('should not navigate a link without an href', function () {
$rootRouter.config([ setup();
configureRouter([
{ path: '/', component: 'oneCmp' }, { path: '/', component: 'oneCmp' },
{ path: '/two', component: 'twoCmp', name: 'Two'} { path: '/two', component: 'twoCmp', name: 'Two'}
]); ]);
expect(function () { expect(function () {
compile('<a>link</a>'); var elt = compile('<a>link</a>');
$rootScope.$digest();
expect(elt.text()).toBe('link'); expect(elt.text()).toBe('link');
elt.find('a')[0].click(); elt.find('a')[0].click();
$rootScope.$digest(); inject(function($rootScope) { $rootScope.$digest(); });
}).not.toThrow(); }).not.toThrow();
}); });
it('should add an ng-link-active class on the current link', inject(function ($rootRouter) { it('should add an ng-link-active class on the current link', function() {
$rootRouter.config([ setup();
configureRouter([
{ path: '/', component: 'oneCmp', name: 'One' } { path: '/', component: 'oneCmp', name: 'One' }
]); ]);
compile('<a ng-link="[\'/One\']">one</a> | <div ng-outlet></div>'); var elt = compile('<a ng-link="[\'/One\']">one</a> | <div ng-outlet></div>');
$rootScope.$digest(); navigateTo('/');
$rootRouter.navigateByUrl('/');
$rootScope.$digest();
expect(elt.find('a').attr('class')).toBe('ng-link-active'); expect(elt.find('a').attr('class')).toBe('ng-link-active');
})); });
function registerComponent(name, template, config) { describe('html5Mode disabled', function () {
var controller = function () {}; it('should prepend href with a hash', function () {
setup({ html5Mode: false });
module(function($locationProvider) {
$locationProvider.html5Mode(false);
});
configureRouter([
{ path: '/b', component: 'twoCmp', name: 'Two' }
]);
var elt = compile('<a ng-link="[\'/Two\']">link</a>');
expect(elt.find('a').attr('href')).toBe('#/b');
});
});
function factory() {
return { function registerComponent(name, template, controller) {
module(function($compileProvider) {
$compileProvider.component(name, {
template: template, template: template,
controllerAs: name,
controller: controller controller: controller
}; });
} });
}
if (!template) { function setup(config) {
template = ''; var html5Mode = !(config && config.html5Mode === false);
} module('ngComponentRouter')
if (angular.isArray(config)) { module(function($locationProvider) {
factory.annotations = [new angular.annotations.RouteConfig(config)]; $locationProvider.html5Mode(html5Mode);
} else if (typeof config === 'function') { });
controller = config; registerComponent('userCmp', '<div>hello {{$ctrl.$routeParams.name}}</div>', function () {});
} else if (typeof config === 'object') { registerComponent('oneCmp', '<div>{{$ctrl.number}}</div>', function () {this.number = 'one'});
if (config.canActivate) { registerComponent('twoCmp', '<div><a ng-link="[\'/Two\']">{{$ctrl.number}}</a></div>', function () {this.number = 'two'});
controller.$canActivate = config.canActivate; }
}
} function configureRouter(routeConfig) {
$compileProvider.directive(name, factory); inject(function($rootRouter) {
$rootRouter.config(routeConfig);
});
} }
function compile(template) { function compile(template) {
elt = $compile('<div>' + template + '</div>')($rootScope); var elt;
$rootScope.$digest(); inject(function($compile, $rootScope) {
elt = $compile('<div>' + template + '</div>')($rootScope);
$rootScope.$digest();
});
return elt; return elt;
} }
function navigateTo(url) {
inject(function($rootRouter, $rootScope) {
$rootRouter.navigateByUrl(url);
$rootScope.$digest();
});
}
}); });