fix(router): fixed the location wrapper for angular1#6698
fix(router): fixed the location wrapper for angular1#6698davidreher wants to merge 1 commit intoangular:masterfrom
Conversation
In angular2 `Location.path()` returns the complete path including query string. In angular1 the query parameters are missing. Similar to this `Location.go` does accept two parameters (path *and query*).
|
With this PR merged, query paramters will be reflected in the url as expected (if used within angular1.5). // Without PR
router.navigateUrl('/something?a=b&c=d'); // results in "/something" in adressbar
// With PR
router.navigateUrl('/something?a=b&c=d'); // results in "/something?a=b&c=d" in adressbar |
|
circle failed in npm install ... most likely not because of our change ^^ @btford can you take a look? |
|
LGTM - but we really ought to have tests for all this... |
petebacondarwin
added
pr_state: LGTM
area: build & ci
| Location.prototype.go = function (url) { | ||
| return $location.path(url); | ||
| Location.prototype.go = function (path, query) { | ||
| return $location.url(path + query); |
There was a problem hiding this comment.
I would say that this should be
return $location.url(path).search(query);since simply concatenating would end up with something like
go('a/b/c', 'x=y') -> 'a/b/cx=y'
There was a problem hiding this comment.
I take that back, it seems that the router passes the & as part of the query!
|
Closing in favour of #6943, which incorporates this fix and adds tests. |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
4 participants
In angular2
Location.path()returns the complete path including query string. In angular1 the query parameters are missing. Similar to thisLocation.godoes accept two parameters (path and query).Found and fixed by @Mischi