Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/app switcher #4994

Merged
merged 16 commits into from
Sep 23, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/ui/public/chrome/api/angular.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
var $ = require('jquery');
var _ = require('lodash');

require('../appSwitcher/appSwitcher.less');
require('../appSwitcher');
var modules = require('ui/modules');
var ConfigTemplate = require('ui/ConfigTemplate');
require('ui/directives/config');
Expand Down Expand Up @@ -63,7 +63,7 @@ module.exports = function (chrome, internals) {
$scope.httpActive = $http.pendingRequests;
$scope.notifList = require('ui/notify')._notifs;
$scope.appSwitcherTemplate = new ConfigTemplate({
switcher: require('../appSwitcher/appSwitcher.html')
switcher: '<app-switcher></app-switcher>'
});

return chrome;
Expand Down
4 changes: 4 additions & 0 deletions src/ui/public/chrome/api/nav.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ module.exports = function (chrome, internals) {
a.setAttribute('href', link.url);
link.url = a.href;
link.lastSubUrl = chrome.getLastSubUrlFor(link.url);

if (link.url === chrome.getAppUrl()) {
link.active = true;
}
});

};
237 changes: 237 additions & 0 deletions src/ui/public/chrome/appSwitcher/__tests__/appSwitcher.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,237 @@
var sinon = require('auto-release-sinon');
var ngMock = require('ngMock');
var $ = require('jquery');
var expect = require('expect.js');
var constant = require('lodash').constant;
var set = require('lodash').set;
var cloneDeep = require('lodash').cloneDeep;
var indexBy = require('lodash').indexBy;

require('ui/chrome');
require('ui/chrome/appSwitcher');
var DomLocationProvider = require('ui/domLocation');

function findTestSubject() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to be duplicated with findTestSubject.js?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, didn't mean to include findTestSubject.js. Planned to replace with #5001

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed findTestSubject.js

var subjectSelectors = [].slice.apply(arguments);
var $context = subjectSelectors.shift();
var $els = $();

subjectSelectors.forEach(function (subjects) {
var selector = subjects.split(/\s+/).map(function (subject) {
return '[data-test-subj~="' + subject + '"]';
}).join(' ');

$els = $els.add($context.find(selector));
});

return $els;
}

describe('appSwitcher directive', function () {
var env;

beforeEach(ngMock.module('kibana'));

function setup(href, links) {
return ngMock.inject(function ($window, $rootScope, $compile, Private) {
var domLocation = Private(DomLocationProvider);

$rootScope.chrome = {
getNavLinks: constant(cloneDeep(links)),
};

env = {
$scope: $rootScope,
$el: $compile($('<app-switcher>'))($rootScope),
currentHref: href,
location: domLocation
};

Object.defineProperties(domLocation, {
href: {
get: function () { return env.currentHref; },
set: function (val) { return env.currentHref = val; },
},
reload: {
value: sinon.stub()
}
});

env.$scope.$digest();
});
}

context('when one link is for the active app', function () {
var myLink = {
active: true,
title: 'myLink',
url: 'http://localhost:555/app/myApp',
lastSubUrl: 'http://localhost:555/app/myApp#/lastSubUrl'
};

var notMyLink = {
active: false,
title: 'notMyLink',
url: 'http://localhost:555/app/notMyApp',
lastSubUrl: 'http://localhost:555/app/notMyApp#/lastSubUrl'
};

beforeEach(setup('http://localhost:5555/app/myApp/', [myLink, notMyLink]));

it('links to the inactive apps base url', function () {
var $myLink = findTestSubject(env.$el, 'appLink').eq(0);
expect($myLink.prop('href')).to.be(myLink.url);
expect($myLink.prop('href')).to.not.be(myLink.lastSubUrl);
});

it('links to the inactive apps last sub url', function () {
var $notMyLink = findTestSubject(env.$el, 'appLink').eq(1);
expect($notMyLink.prop('href')).to.be(notMyLink.lastSubUrl);
expect($notMyLink.prop('href')).to.not.be(notMyLink.url);
});
});

context('when none of the links are for the active app', function () {
var myLink = {
active: false,
title: 'myLink',
url: 'http://localhost:555/app/myApp',
lastSubUrl: 'http://localhost:555/app/myApp#/lastSubUrl'
};

var notMyLink = {
active: false,
title: 'notMyLink',
url: 'http://localhost:555/app/notMyApp',
lastSubUrl: 'http://localhost:555/app/notMyApp#/lastSubUrl'
};

beforeEach(setup('http://localhost:5555/app/myApp/', [myLink, notMyLink]));

it('links to the lastSubUrl for each', function () {
var $links = findTestSubject(env.$el, 'appLink');
var $myLink = $links.eq(0);
var $notMyLink = $links.eq(1);

expect($myLink.prop('href')).to.be(myLink.lastSubUrl);
expect($myLink.prop('href')).to.not.be(myLink.url);

expect($notMyLink.prop('href')).to.be(notMyLink.lastSubUrl);
expect($notMyLink.prop('href')).to.not.be(notMyLink.url);
});
});

context('clicking a link with matching href but missing hash', function () {
var url = 'http://localhost:555/app/myApp?query=1';
beforeEach(setup(url + '#/lastSubUrl', [
{ url: url }
]));

it('just prevents propogation (no reload)', function () {
var event = new $.Event('click');

expect(env.location.reload.callCount).to.be(0);
expect(event.isDefaultPrevented()).to.be(false);
expect(event.isPropagationStopped()).to.be(false);

var $link = findTestSubject(env.$el, 'appLink');
expect($link.prop('href')).to.be(url);
$link.trigger(event);

expect(env.location.reload.callCount).to.be(0);
expect(event.isDefaultPrevented()).to.be(false);
expect(event.isPropagationStopped()).to.be(true);
});
});

context('clicking a link that matches entire url', function () {
var url = 'http://localhost:555/app/myApp#/lastSubUrl';
beforeEach(setup(url, [
{ url: url }
]));

it('calls window.location.reload and prevents propogation', function () {
var event = new $.Event('click');

expect(env.location.reload.callCount).to.be(0);
expect(event.isDefaultPrevented()).to.be(false);
expect(event.isPropagationStopped()).to.be(false);

var $link = findTestSubject(env.$el, 'appLink');
expect($link.prop('href')).to.be(env.currentHref);
$link.trigger(event);

expect(env.location.reload.callCount).to.be(1);
expect(event.isDefaultPrevented()).to.be(false);
expect(event.isPropagationStopped()).to.be(true);
});
});

context('clicking a link with matching href but changed hash', function () {
var rootUrl = 'http://localhost:555/app/myApp?query=1';
var url = rootUrl + '#/lastSubUrl2';

beforeEach(setup(url + '#/lastSubUrl', [
{ url: url }
]));

it('calls window.location.reload and prevents propogation', function () {
var event = new $.Event('click');

expect(env.location.reload.callCount).to.be(0);
expect(event.isDefaultPrevented()).to.be(false);
expect(event.isPropagationStopped()).to.be(false);

var $link = findTestSubject(env.$el, 'appLink');
expect($link.prop('href')).to.be(url);
$link.trigger(event);

expect(env.location.reload.callCount).to.be(1);
expect(event.isDefaultPrevented()).to.be(false);
expect(event.isPropagationStopped()).to.be(true);
});
});

context('clicking a link with matching host', function () {
beforeEach(setup('http://localhost:555/someOtherPath', [
{
active: true,
url: 'http://localhost:555/app/myApp'
}
]));

it('allows click through', function () {
var event = new $.Event('click');

expect(env.location.reload.callCount).to.be(0);
expect(event.isPropagationStopped()).to.be(false);

findTestSubject(env.$el, 'appLink').trigger(event);

expect(env.location.reload.callCount).to.be(0);
expect(event.isPropagationStopped()).to.be(false);
});
});

context('clicking a link with matching host and path', function () {
beforeEach(setup('http://localhost:555/app/myApp?someQuery=true', [
{
active: true,
url: 'http://localhost:555/app/myApp?differentQuery=true'
}
]));

it('allows click through', function () {
var event = new $.Event('click');

expect(env.location.reload.callCount).to.be(0);
expect(event.isPropagationStopped()).to.be(false);

findTestSubject(env.$el, 'appLink').trigger(event);

expect(env.location.reload.callCount).to.be(0);
expect(event.isPropagationStopped()).to.be(false);
});
});

});
17 changes: 12 additions & 5 deletions src/ui/public/chrome/appSwitcher/appSwitcher.html
Original file line number Diff line number Diff line change
@@ -1,11 +1,18 @@
<div class="app-links">
<div class="app-link" ng-repeat="app in chrome.getNavLinks() | orderBy:'title'">
<a ng-href="{{ app.lastSubUrl || app.url }}">
<div
class="app-link"
ng-repeat="link in switcher.getNavLinks() | orderBy:'title'"
ng-class="{ active: link.active }">

<div ng-if="app.icon" ng-style="{ 'background-image': 'url(../' + app.icon + ')' }" class="app-icon"></div>
<div ng-if="!app.icon" class="app-icon app-icon-missing">{{app.title[0]}}</div>
<a
ng-click="switcher.ensureNavigation($event, link)"
ng-href="{{ link.active ? link.url : (link.lastSubUrl || link.url) }}"
data-test-subj="appLink">

<div class="app-title">{{ app.title }}</div>
<div ng-if="link.icon" ng-style="{ 'background-image': 'url(../' + link.icon + ')' }" class="app-icon"></div>
<div ng-if="!link.icon" class="app-icon app-icon-missing">{{ link.title[0] }}</div>

<div class="app-title">{{ link.title }}</div>
</a>
</div>
</div>
51 changes: 51 additions & 0 deletions src/ui/public/chrome/appSwitcher/appSwitcher.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
var parse = require('url').parse;
var bindKey = require('lodash').bindKey;

require('../appSwitcher/appSwitcher.less');
var DomLocationProvider = require('ui/domLocation');

require('ui/modules')
.get('kibana')
.directive('appSwitcher', function () {
return {
restrict: 'E',
template: require('./appSwitcher.html'),
controllerAs: 'switcher',
controller: function ($scope, Private) {
var domLocation = Private(DomLocationProvider);

// since we render this in an isolate scope we can't "require: ^chrome", but
// rather than remove all helpfull checks we can just check here.
if (!$scope.chrome || !$scope.chrome.getNavLinks) {
throw new TypeError('appSwitcher directive requires the "chrome" config-object');
}

this.getNavLinks = bindKey($scope.chrome, 'getNavLinks');

// links don't cause full-navigation events in certain scenarios
// so we force them when needed
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could use a test

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do

this.ensureNavigation = function (event, app) {
if (event.isDefaultPrevented() || event.altKey || event.metaKey || event.ctrlKey) {
return;
}

var toParsed = parse(event.delegateTarget.href);
var fromParsed = parse(domLocation.href);
var sameProto = toParsed.protocol === fromParsed.protocol;
var sameHost = toParsed.host === fromParsed.host;
var samePath = toParsed.path === fromParsed.path;

if (sameProto && sameHost && samePath) {
toParsed.hash && domLocation.reload();

// event.preventDefault() keeps the browser from seeing the new url as an update
// and even setting window.location does not mimic that behavior, so instead
// we use stopPropagation() to prevent angular from seeing the click and
// starting a digest cycle/attempting to handle it in the router.
event.stopPropagation();
}
};

}
};
});
14 changes: 12 additions & 2 deletions src/ui/public/chrome/appSwitcher/appSwitcher.less
Original file line number Diff line number Diff line change
@@ -1,16 +1,19 @@
@import (reference) "~ui/styles/variables";

@app-icon-size: 48px;
@app-icon-padding: 10px;

.app-links {
text-align: justify;

.app-link {
display: inline-block;
vertical-align: top;
width: @app-icon-size;
margin: 0px 10px;
text-align: left;
width: @app-icon-size + (@app-icon-padding * 2);
margin: 0px 10px;
padding: @app-icon-padding;
border-radius: @border-radius-base;

.app-icon {
display: block;
Expand Down Expand Up @@ -43,6 +46,13 @@
text-decoration: underline;
}

&.active {
background: @gray-lighter;
.app-title {
text-decoration: underline;
}
}

}

}
15 changes: 15 additions & 0 deletions src/ui/public/domLocation.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
module.exports = function DomLocationProvider($window) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious, why do you create this module instead of using something like $location?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$location doesn't provide the whole url, just the url as the angular router sees it (the part after the hash). Since we have different apps with potentially different base urls (like status page) I needed the whole url

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, $location.absUrl is only a getter, not a setter. Makes sense. Thanks

return {
reload: function (forceFetch) {
$window.location.reload(forceFetch);
},

get href() {
return $window.location.href;
},

set href(val) {
return ($window.location.href = val);
}
};
};