github-mirrors/kibana
1
0
Fork 0
mirror of https://github.com/elastic/kibana.git synced 2025-04-23 09:19:04 -04:00
Code Issues Projects Releases Packages Wiki Activity

[kbnUrl] reload the route when going from "" to "/" (#8998)

Browse source 
Backports PR #8815

**Commit 1:**
[kbnUrl] reload the route when going from "" to "/"

In timelion the initial route is set to '' which might not be perfectly correct, but works fine. When clicking the "new" button for the first time this causes the route to update from '' to '/', which the kbnUrl service assumes will cause a route change and does not try to force the route to reload. Instead, the router sees this as a noop and the change to the route has no effect unless you click the "new" button a second time.

* Original sha: a7304ec425
* Authored by spalger <email@spalger.com> on 2016-10-24T20:43:40Z

**Commit 2:**
[kbnUrl] clarify the purpose _shouldAutoReload

* Original sha: fbfbae3926
* Authored by spalger <email@spalger.com> on 2016-10-24T23:45:52Z

**Commit 3:**
[kbnUrl] fix the tests

* Original sha: cc9c2f66d2
* Authored by spalger <email@spalger.com> on 2016-10-25T00:31:22Z
This commit is contained in:
jasper 2016-11-08 10:19:51 -05:00 • committed by Spencer
parent 5b2bb402a0
commit fc8cef75a6
2 changed files with 20 additions and 16 deletions
Download patch file Download diff file Expand all files Collapse all files

24
src/ui/public/url/__tests__/url.js
View file

@ -369,7 +369,7 @@ describe('kbnUrl', function () {
});
it('should call replace on $location', function () {
sinon.stub(kbnUrl, '_shouldAutoReload').returns(false);
sinon.stub(kbnUrl, '_shouldForceReload').returns(false);
sinon.stub($location, 'replace');
expect($location.replace.callCount).to.be(0);
@ -404,7 +404,7 @@ describe('kbnUrl', function () {
});
it('should call replace on $location', function () {
sinon.stub(kbnUrl, '_shouldAutoReload').returns(false);
sinon.stub(kbnUrl, '_shouldForceReload').returns(false);
sinon.stub($location, 'replace');
expect($location.replace.callCount).to.be(0);
@ -413,7 +413,7 @@ describe('kbnUrl', function () {
});
});
describe('_shouldAutoReload', function () {
describe('_shouldForceReload', function () {
let next;
let prev;
@ -431,7 +431,7 @@ describe('kbnUrl', function () {
it('returns false if the passed url doesn\'t match the current route', function () {
next.path = '/not current';
expect(kbnUrl._shouldAutoReload(next, prev, $route)).to.be(false);
expect(kbnUrl._shouldForceReload(next, prev, $route)).to.be(false);
});
describe('if the passed url does match the route', function () {
@ -439,14 +439,14 @@ describe('kbnUrl', function () {
describe('and the path is the same', function () {
describe('and the search params are the same', function () {
it('returns true', function () {
expect(kbnUrl._shouldAutoReload(next, prev, $route)).to.be(true);
expect(kbnUrl._shouldForceReload(next, prev, $route)).to.be(true);
});
});
describe('but the search params are different', function () {
it('returns false', function () {
next.search = {};
prev.search = { q: 'search term' };
expect(kbnUrl._shouldAutoReload(next, prev, $route)).to.be(false);
expect(kbnUrl._shouldForceReload(next, prev, $route)).to.be(false);
});
});
});
@ -458,14 +458,14 @@ describe('kbnUrl', function () {
describe('and the search params are the same', function () {
it('returns false', function () {
expect(kbnUrl._shouldAutoReload(next, prev, $route)).to.be(false);
expect(kbnUrl._shouldForceReload(next, prev, $route)).to.be(false);
});
});
describe('but the search params are different', function () {
it('returns false', function () {
next.search = {};
prev.search = { q: 'search term' };
expect(kbnUrl._shouldAutoReload(next, prev, $route)).to.be(false);
expect(kbnUrl._shouldForceReload(next, prev, $route)).to.be(false);
});
});
});
@ -479,14 +479,14 @@ describe('kbnUrl', function () {
describe('and the path is the same', function () {
describe('and the search params are the same', function () {
it('returns true', function () {
expect(kbnUrl._shouldAutoReload(next, prev, $route)).to.be(true);
expect(kbnUrl._shouldForceReload(next, prev, $route)).to.be(true);
});
});
describe('but the search params are different', function () {
it('returns true', function () {
next.search = {};
prev.search = { q: 'search term' };
expect(kbnUrl._shouldAutoReload(next, prev, $route)).to.be(true);
expect(kbnUrl._shouldForceReload(next, prev, $route)).to.be(true);
});
});
});
@ -498,14 +498,14 @@ describe('kbnUrl', function () {
describe('and the search params are the same', function () {
it('returns false', function () {
expect(kbnUrl._shouldAutoReload(next, prev, $route)).to.be(false);
expect(kbnUrl._shouldForceReload(next, prev, $route)).to.be(false);
});
});
describe('but the search params are different', function () {
it('returns false', function () {
next.search = {};
prev.search = { q: 'search term' };
expect(kbnUrl._shouldAutoReload(next, prev, $route)).to.be(false);
expect(kbnUrl._shouldForceReload(next, prev, $route)).to.be(false);
});
});
});

12
src/ui/public/url/url.js
View file

@ -165,7 +165,7 @@ function KbnUrlProvider($injector, $location, $rootScope, $parse, Private) {
if ($injector.has('$route')) {
const $route = $injector.get('$route');
if (self._shouldAutoReload(next, prev, $route)) {
if (self._shouldForceReload(next, prev, $route)) {
const appState = Private(AppStateProvider).getAppState();
if (appState) appState.destroy();
@ -180,17 +180,21 @@ function KbnUrlProvider($injector, $location, $rootScope, $parse, Private) {
}
};
self._shouldAutoReload = function (next, prev, $route) {
// determine if the router will automatically reload the route
self._shouldForceReload = function (next, prev, $route) {
if (reloading) return false;
let route = $route.current && $route.current.$$route;
if (!route) return false;
if (next.path !== prev.path) return false;
// for the purposes of determining whether the router will
// automatically be reloading, '' and '/' are equal
const nextPath = next.path || '/';
const prevPath = prev.path || '/';
if (nextPath !== prevPath) return false;
let reloadOnSearch = route.reloadOnSearch;
let searchSame = _.isEqual(next.search, prev.search);
return (reloadOnSearch && searchSame) || !reloadOnSearch;
};
}