From cbdab120275e449102668f77462517541cb2fb84 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Mon, 11 Jul 2016 11:07:27 -0700 Subject: [PATCH 1/6] Refactor app-switcher to use app-switcher-link directive. Former-commit-id: 01f38688586f203d3883916844d98709636e9e6d --- .../directives/app_switcher/app_switcher.html | 35 ++++++------------- .../app_switcher_link/app_switcher_link.html | 28 +++++++++++++++ .../app_switcher_link/app_switcher_link.js | 21 +++++++++++ src/ui/public/chrome/directives/index.js | 1 + 4 files changed, 60 insertions(+), 25 deletions(-) create mode 100644 src/ui/public/chrome/directives/app_switcher_link/app_switcher_link.html create mode 100644 src/ui/public/chrome/directives/app_switcher_link/app_switcher_link.js diff --git a/src/ui/public/chrome/directives/app_switcher/app_switcher.html b/src/ui/public/chrome/directives/app_switcher/app_switcher.html index 3a70738dcb73b..7d8291bc26ae5 100644 --- a/src/ui/public/chrome/directives/app_switcher/app_switcher.html +++ b/src/ui/public/chrome/directives/app_switcher/app_switcher.html @@ -1,26 +1,11 @@ -
-
- - -
-
{{ link.title[0] }}
- -
{{ link.title }}
- -
-
+ diff --git a/src/ui/public/chrome/directives/app_switcher_link/app_switcher_link.html b/src/ui/public/chrome/directives/app_switcher_link/app_switcher_link.html new file mode 100644 index 0000000000000..8d91540bf2a36 --- /dev/null +++ b/src/ui/public/chrome/directives/app_switcher_link/app_switcher_link.html @@ -0,0 +1,28 @@ +
+ +
+ +
+ +
+ {{ title[0] }} +
+ +
+ {{ title }} +
+ +
diff --git a/src/ui/public/chrome/directives/app_switcher_link/app_switcher_link.js b/src/ui/public/chrome/directives/app_switcher_link/app_switcher_link.js new file mode 100644 index 0000000000000..d3b4586c17149 --- /dev/null +++ b/src/ui/public/chrome/directives/app_switcher_link/app_switcher_link.js @@ -0,0 +1,21 @@ + +import appSwitcherLinkTemplate from './app_switcher_link.html'; +import uiModules from 'ui/modules'; + +const module = uiModules.get('kibana'); + +module.directive('appSwitcherLink', function () { + return { + restrict: 'E', + scope: { + isActive: '=appSwitcherLinkIsActive', + isDisabled: '=appSwitcherLinkIsDisabled', + tooltip: '=appSwitcherLinkTooltip', + onClick: '&appSwitcherLinkOnClick', + href: '=appSwitcherLinkHref', + icon: '=appSwitcherLinkIcon', + title: '=appSwitcherLinkTitle' + }, + template: appSwitcherLinkTemplate + }; +}); diff --git a/src/ui/public/chrome/directives/index.js b/src/ui/public/chrome/directives/index.js index c39bb921d3a5b..7be137eb52539 100644 --- a/src/ui/public/chrome/directives/index.js +++ b/src/ui/public/chrome/directives/index.js @@ -1,4 +1,5 @@ import './app_switcher'; +import './app_switcher_link'; import kbnChromeProv from './kbn_chrome'; import kbnChromeNavControlsProv from './append_nav_controls'; import './kbn_loading_indicator'; From f3284ee19b807f98aa758cccbfe3df8f5e621070 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Mon, 11 Jul 2016 11:34:50 -0700 Subject: [PATCH 2/6] Refactor app-switcher and app-switcher-link LESS. Remove app-links class. Former-commit-id: 0d0399935b46d1ac2d7afccf52c92148b6eb7ae7 --- src/ui/public/chrome/chrome.html | 2 +- .../directives/app_switcher/app_switcher.js | 2 +- .../directives/app_switcher/app_switcher.less | 78 ------------------- .../app_switcher_link/app_switcher_link.html | 22 +++--- .../app_switcher_link/app_switcher_link.js | 1 + .../app_switcher_link/app_switcher_link.less | 70 +++++++++++++++++ src/ui/public/styles/variables/for-theme.less | 1 - 7 files changed, 86 insertions(+), 90 deletions(-) create mode 100644 src/ui/public/chrome/directives/app_switcher_link/app_switcher_link.less diff --git a/src/ui/public/chrome/chrome.html b/src/ui/public/chrome/chrome.html index 419615d665dda..42b78e7ee7f39 100644 --- a/src/ui/public/chrome/chrome.html +++ b/src/ui/public/chrome/chrome.html @@ -26,7 +26,7 @@
-
+
diff --git a/src/ui/public/chrome/directives/app_switcher/app_switcher.js b/src/ui/public/chrome/directives/app_switcher/app_switcher.js index 9119e0bf63828..3181f88d62cdc 100644 --- a/src/ui/public/chrome/directives/app_switcher/app_switcher.js +++ b/src/ui/public/chrome/directives/app_switcher/app_switcher.js @@ -1,7 +1,7 @@ import DomLocationProvider from 'ui/dom_location'; import { parse } from 'url'; import { bindKey } from 'lodash'; -import '../app_switcher/app_switcher.less'; +import './app_switcher.less'; import uiModules from 'ui/modules'; import appSwitcherTemplate from './app_switcher.html'; diff --git a/src/ui/public/chrome/directives/app_switcher/app_switcher.less b/src/ui/public/chrome/directives/app_switcher/app_switcher.less index e1cc894fd3a27..0d22b23c0aa0a 100644 --- a/src/ui/public/chrome/directives/app_switcher/app_switcher.less +++ b/src/ui/public/chrome/directives/app_switcher/app_switcher.less @@ -70,81 +70,3 @@ body { overflow-x: hidden; } .flex-parent(@shrink: 0); box-shadow: -4px 0px 3px rgba(0,0,0,0.2); } - -.app-links { - text-align: justify; - - .app-link { - width: @as-open-width; - height: @app-icon-height; - line-height: @app-line-height; - - > a { - display: block; - height: 100%; - color: #ebf7fa; - } - - &.is-app-switcher-app-link-disabled { - opacity: 0.5; - - .app-link__anchor { - cursor: default; - } - } - - .app-icon { - float: left; - font-weight: bold; - text-align: center; - font-size: 1.7em; - display: inline-block; - height: @app-icon-height; - width: @as-closed-width; - - > img { - height: 18px; - margin-top: 8px; - filter: invert(100%); - } - > i { - color: #fff; - line-height: @app-icon-height - } - } - - .app-icon-missing { - float: left; - text-align: center; - font-size: 1.7em; - display: inline-block; - height: @app-icon-height; - line-height: @app-icon-height; - width: @as-closed-width; - background-position: center; - background-size: contain; - background-repeat: no-repeat; - } - - .app-title { - width: calc(@as-open-width - @as-closed-width); - display: inline-block; - float: right; - font-size: 0.9em; - text-align: left; - padding-left: 3px; - line-height: @app-icon-height; - white-space: nowrap; - } - - &:hover, - &.active { - background-color: @app-links-active-background; - > a { - color: @white; - text-decoration: none; - } - } - } - -} diff --git a/src/ui/public/chrome/directives/app_switcher_link/app_switcher_link.html b/src/ui/public/chrome/directives/app_switcher_link/app_switcher_link.html index 8d91540bf2a36..14ef6455d2fdf 100644 --- a/src/ui/public/chrome/directives/app_switcher_link/app_switcher_link.html +++ b/src/ui/public/chrome/directives/app_switcher_link/app_switcher_link.html @@ -1,5 +1,5 @@
-
- -
+
+ -
- {{ title[0] }} + + {{ title[0] }} +
-
+
{{ title }}
diff --git a/src/ui/public/chrome/directives/app_switcher_link/app_switcher_link.js b/src/ui/public/chrome/directives/app_switcher_link/app_switcher_link.js index d3b4586c17149..8f510dd997052 100644 --- a/src/ui/public/chrome/directives/app_switcher_link/app_switcher_link.js +++ b/src/ui/public/chrome/directives/app_switcher_link/app_switcher_link.js @@ -1,5 +1,6 @@ import appSwitcherLinkTemplate from './app_switcher_link.html'; +import './app_switcher_link.less'; import uiModules from 'ui/modules'; const module = uiModules.get('kibana'); diff --git a/src/ui/public/chrome/directives/app_switcher_link/app_switcher_link.less b/src/ui/public/chrome/directives/app_switcher_link/app_switcher_link.less new file mode 100644 index 0000000000000..ccd7cae2161db --- /dev/null +++ b/src/ui/public/chrome/directives/app_switcher_link/app_switcher_link.less @@ -0,0 +1,70 @@ +@import (reference) "~ui/styles/variables"; + +.app-switcher-link { + width: @as-open-width; + height: @app-icon-height; + line-height: @app-line-height; + + &.is-app-switcher-link-disabled { + opacity: 0.5; + + .app-switcher-link__anchor { + cursor: default; + } + } + + &:hover, + &.active { + background-color: @app-links-active-background; + + .app-switcher-link__anchor { + color: @white; + text-decoration: none; + } + } +} + + .app-switcher-link__anchor { + display: block; + height: 100%; + color: #ebf7fa; + } + + .app-switcher-link__icon { + display: inline-block; + width: @as-closed-width; + height: @app-icon-height; + float: left; + text-align: center; + font-size: 1.7em; + } + + /** + * This imgae is used to display the icon. + */ + .app-switcher-link__icon-image { + height: 18px; + margin-top: 8px; + filter: invert(100%); + } + + /** + * This placeholder text gets shown if there is no specified icon. + */ + .app-switcher-link__icon-placeholder { + line-height: @app-icon-height; + background-position: center; + background-size: contain; + background-repeat: no-repeat; + } + + .app-switcher-link__title { + width: calc(@as-open-width - @as-closed-width); + display: inline-block; + float: right; + font-size: 0.9em; + text-align: left; + padding-left: 3px; + line-height: @app-icon-height; + white-space: nowrap; + } diff --git a/src/ui/public/styles/variables/for-theme.less b/src/ui/public/styles/variables/for-theme.less index 21511cc7dccc5..8f49e090bf361 100644 --- a/src/ui/public/styles/variables/for-theme.less +++ b/src/ui/public/styles/variables/for-theme.less @@ -106,7 +106,6 @@ // AppSwitcher ================================================================= @app-switcher-application-bg: @gray-lighter; -@app-switcher-app-link-color: @white; @app-switcher-app-title-color: @text-color; @app-switcher-app-description-color: @gray; From f4ecb39c1658e03ee69d2d91aab597a33f2b59d6 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Mon, 11 Jul 2016 14:23:10 -0700 Subject: [PATCH 3/6] Add tests for app-switcher-link directive. Former-commit-id: 0e2bbbc0698101a1af13f29f5e681721918f5fe2 --- .../__tests__/app-switcher-link.js | 169 ++++++++++++++++++ .../app_switcher_link/app_switcher_link.html | 6 +- .../app_switcher_link/app_switcher_link.js | 1 + 3 files changed, 175 insertions(+), 1 deletion(-) create mode 100644 src/ui/public/chrome/directives/app_switcher_link/__tests__/app-switcher-link.js diff --git a/src/ui/public/chrome/directives/app_switcher_link/__tests__/app-switcher-link.js b/src/ui/public/chrome/directives/app_switcher_link/__tests__/app-switcher-link.js new file mode 100644 index 0000000000000..86d925004dd38 --- /dev/null +++ b/src/ui/public/chrome/directives/app_switcher_link/__tests__/app-switcher-link.js @@ -0,0 +1,169 @@ +import sinon from 'auto-release-sinon'; +import ngMock from 'ng_mock'; +import expect from 'expect.js'; + +import '../app_switcher_link'; + +describe('appSwitcherLink directive', () => { + let scope; + let $compile; + + beforeEach(ngMock.module('kibana')); + + beforeEach(() => { + ngMock.inject(($rootScope, _$compile_) => { + scope = $rootScope.$new(); + $compile = _$compile_; + }); + }); + + function create(attrs) { + const template = ` + + `; + + const element = $compile(template)(scope); + + scope.$apply(() => { + Object.assign(scope, attrs); + }); + + return element; + } + + describe('interface', () => { + + describe('appSwitcherLinkIsActive attribute', () => { + it(`doesn't apply the active class when false`, () => { + const element = create({ + appSwitcherLinkIsActive: false, + }); + expect(element.hasClass('active')).to.be(false); + }); + + it('applies the active class when true', () => { + const element = create({ + appSwitcherLinkIsActive: true, + }); + expect(element.hasClass('active')).to.be(true); + }); + }); + + describe('appSwitcherLinkIsDisabled attribute', () => { + it(`doesn't apply the is-app-switcher-link-disabled class when false`, () => { + const element = create({ + appSwitcherLinkIsDisabled: false, + }); + expect(element.hasClass('is-app-switcher-link-disabled')).to.be(false); + }); + + it('applies the is-app-switcher-link-disabled class when true', () => { + const element = create({ + appSwitcherLinkIsDisabled: true, + }); + expect(element.hasClass('is-app-switcher-link-disabled')).to.be(true); + }); + }); + + describe('appSwitcherLinkTooltip attribute', () => { + it('is applied to the tooltip directive', () => { + const attrs = { + appSwitcherLinkTooltip: 'hello i am a tooltip', + }; + const element = create(attrs); + expect(element.attr('tooltip')).to.be(attrs.appSwitcherLinkTooltip); + }); + }); + + describe('appSwitcherLinkOnClick attribute', () => { + it('is called when the link is clicked', () => { + const attrs = { + appSwitcherLinkOnClick: sinon.spy(), + }; + const element = create(attrs); + element.find('[data-test-subj=appLink]').click(); + sinon.assert.called(attrs.appSwitcherLinkOnClick); + }); + }); + + describe('appSwitcherLinkHref attribute', () => { + it('is applied to the link', () => { + const attrs = { + appSwitcherLinkHref: 'link to a website', + }; + const element = create(attrs); + const link = element.find('[data-test-subj=appLink]'); + expect(link.attr('href')).to.be(attrs.appSwitcherLinkHref); + }); + }); + + describe('appSwitcherLinkIcon attribute', () => { + describe('when present', () => { + it('displays the img element', () => { + const attrs = { + appSwitcherLinkIcon: 'icon url', + }; + const element = create(attrs); + const img = element.find('img'); + expect(img.length).to.be(1); + }); + + it('hides the placeholder', () => { + const attrs = { + appSwitcherLinkIcon: 'icon url', + }; + const element = create(attrs); + const placeholder = element.find('[data-test-subj=appLinkIconPlaceholder]'); + expect(placeholder.length).to.be(0); + }); + + it(`is set as the img src`, () => { + const attrs = { + appSwitcherLinkIcon: 'icon url', + }; + const element = create(attrs); + const img = element.find('img'); + expect(img.attr('src')).to.contain(encodeURI(attrs.appSwitcherLinkIcon)); + }); + }); + + describe('when not present', () => { + it('hides the img element', () => { + const attrs = { + appSwitcherLinkIcon: undefined, + }; + const element = create(attrs); + const img = element.find('img'); + expect(img.length).to.be(0); + }); + + it('displays the placeholder', () => { + const attrs = { + appSwitcherLinkIcon: undefined, + }; + const element = create(attrs); + const placeholder = element.find('[data-test-subj=appLinkIconPlaceholder]'); + expect(placeholder.length).to.be(1); + }); + + it(`uses the title's first letter as the placeholder`, () => { + const attrs = { + appSwitcherLinkIcon: undefined, + appSwitcherLinkTitle: 'Xyz', + }; + const element = create(attrs); + const placeholder = element.find('[data-test-subj=appLinkIconPlaceholder]'); + expect(placeholder.text()).to.contain(attrs.appSwitcherLinkTitle[0]); + }); + }); + }); + }); +}); diff --git a/src/ui/public/chrome/directives/app_switcher_link/app_switcher_link.html b/src/ui/public/chrome/directives/app_switcher_link/app_switcher_link.html index 14ef6455d2fdf..4c36855a11ac3 100644 --- a/src/ui/public/chrome/directives/app_switcher_link/app_switcher_link.html +++ b/src/ui/public/chrome/directives/app_switcher_link/app_switcher_link.html @@ -20,7 +20,11 @@ kbn-src="{{ '/' + icon }}" > - + {{ title[0] }}
diff --git a/src/ui/public/chrome/directives/app_switcher_link/app_switcher_link.js b/src/ui/public/chrome/directives/app_switcher_link/app_switcher_link.js index 8f510dd997052..5da926e84f094 100644 --- a/src/ui/public/chrome/directives/app_switcher_link/app_switcher_link.js +++ b/src/ui/public/chrome/directives/app_switcher_link/app_switcher_link.js @@ -8,6 +8,7 @@ const module = uiModules.get('kibana'); module.directive('appSwitcherLink', function () { return { restrict: 'E', + replace: true, scope: { isActive: '=appSwitcherLinkIsActive', isDisabled: '=appSwitcherLinkIsDisabled', From 7be56757ab92a36d511bd6565d1ac80ef1078bcb Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Mon, 25 Jul 2016 10:54:48 -0700 Subject: [PATCH 4/6] Add tests for app-switcher-link directive's appSwitcherLinkTitle attribute. Fix broken appSwitcherLinkIsDisabled attribute test. Former-commit-id: 27a301b8b16c51bd920dae3313b5ed75274361b2 --- .../__tests__/app-switcher-link.js | 20 +++++++++++++++++++ .../app_switcher_link/app_switcher_link.html | 2 +- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/ui/public/chrome/directives/app_switcher_link/__tests__/app-switcher-link.js b/src/ui/public/chrome/directives/app_switcher_link/__tests__/app-switcher-link.js index 86d925004dd38..becda945bc075 100644 --- a/src/ui/public/chrome/directives/app_switcher_link/__tests__/app-switcher-link.js +++ b/src/ui/public/chrome/directives/app_switcher_link/__tests__/app-switcher-link.js @@ -165,5 +165,25 @@ describe('appSwitcherLink directive', () => { }); }); }); + + describe('appSwitcherLinkTitle attribute', () => { + it('is displayed', () => { + const attrs = { + appSwitcherLinkTitle: 'demo title', + }; + const element = create(attrs); + const title = element.find('.app-switcher-link__title'); + expect(title.text().trim()).to.be(attrs.appSwitcherLinkTitle); + }); + + it('is set as a title attribute on the anchor tag', () => { + const attrs = { + appSwitcherLinkTitle: 'demo title', + }; + const element = create(attrs); + const link = element.find('[data-test-subj=appLink]'); + expect(link.attr('title')).to.be(attrs.appSwitcherLinkTitle); + }); + }); }); }); diff --git a/src/ui/public/chrome/directives/app_switcher_link/app_switcher_link.html b/src/ui/public/chrome/directives/app_switcher_link/app_switcher_link.html index 4c36855a11ac3..78c90dcff4a4a 100644 --- a/src/ui/public/chrome/directives/app_switcher_link/app_switcher_link.html +++ b/src/ui/public/chrome/directives/app_switcher_link/app_switcher_link.html @@ -1,6 +1,6 @@
Date: Mon, 25 Jul 2016 12:28:45 -0700 Subject: [PATCH 5/6] Prevent anchor focus styles from affecting app-switcher-link. Former-commit-id: 56866fbf9b442f9a011840d3fa5e28581854aba0 --- .../directives/app_switcher_link/app_switcher_link.less | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/ui/public/chrome/directives/app_switcher_link/app_switcher_link.less b/src/ui/public/chrome/directives/app_switcher_link/app_switcher_link.less index ccd7cae2161db..c2b84d8376781 100644 --- a/src/ui/public/chrome/directives/app_switcher_link/app_switcher_link.less +++ b/src/ui/public/chrome/directives/app_switcher_link/app_switcher_link.less @@ -28,6 +28,13 @@ display: block; height: 100%; color: #ebf7fa; + + /** + * 1. TODO: Override anchor styles. Fix this by removing a tag styles. + */ + &:focus { + color: #ebf7fa; /* 1 */ + } } .app-switcher-link__icon { From e14de3b4ec9cdeb87b923cb1725bd0257c6655df Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Tue, 26 Jul 2016 14:35:59 -0700 Subject: [PATCH 6/6] Add app-switcher-link-kbn-route attribute to appSwitcherLink directive. This lets you link to Kibana routes. Former-commit-id: 568bdcffd327add011f1e6cdd0bab29c2d82fc98 --- .../__tests__/app-switcher-link.js | 22 +++++++++++++++++++ .../app_switcher_link/app_switcher_link.html | 2 +- .../app_switcher_link/app_switcher_link.js | 16 ++++++++++++-- 3 files changed, 37 insertions(+), 3 deletions(-) diff --git a/src/ui/public/chrome/directives/app_switcher_link/__tests__/app-switcher-link.js b/src/ui/public/chrome/directives/app_switcher_link/__tests__/app-switcher-link.js index becda945bc075..25768f2072893 100644 --- a/src/ui/public/chrome/directives/app_switcher_link/__tests__/app-switcher-link.js +++ b/src/ui/public/chrome/directives/app_switcher_link/__tests__/app-switcher-link.js @@ -25,6 +25,7 @@ describe('appSwitcherLink directive', () => { app-switcher-link-tooltip="appSwitcherLinkTooltip" app-switcher-link-on-click="appSwitcherLinkOnClick()" app-switcher-link-href="appSwitcherLinkHref" + app-switcher-link-kbn-route="appSwitcherLinkKbnRoute" app-switcher-link-icon="appSwitcherLinkIcon" app-switcher-link-title="appSwitcherLinkTitle" /> @@ -105,6 +106,27 @@ describe('appSwitcherLink directive', () => { }); }); + describe('appSwitcherLinkKbnRoute attribute', () => { + it(`is applied to the link when href isn't defined`, () => { + const attrs = { + appSwitcherLinkKbnRoute: '#test', + }; + const element = create(attrs); + const link = element.find('[data-test-subj=appLink]'); + expect(link.attr('href')).to.be(attrs.appSwitcherLinkKbnRoute); + }); + + it(`isn't applied to the link when href is defined`, () => { + const attrs = { + appSwitcherLinkHref: 'link to a website', + appSwitcherLinkKbnRoute: '#test', + }; + const element = create(attrs); + const link = element.find('[data-test-subj=appLink]'); + expect(link.attr('href')).not.to.be(attrs.appSwitcherLinkKbnRoute); + }); + }); + describe('appSwitcherLinkIcon attribute', () => { describe('when present', () => { it('displays the img element', () => { diff --git a/src/ui/public/chrome/directives/app_switcher_link/app_switcher_link.html b/src/ui/public/chrome/directives/app_switcher_link/app_switcher_link.html index 78c90dcff4a4a..6a7843d0430cf 100644 --- a/src/ui/public/chrome/directives/app_switcher_link/app_switcher_link.html +++ b/src/ui/public/chrome/directives/app_switcher_link/app_switcher_link.html @@ -8,7 +8,7 @@ > { return { restrict: 'E', replace: true, @@ -15,9 +15,21 @@ module.directive('appSwitcherLink', function () { tooltip: '=appSwitcherLinkTooltip', onClick: '&appSwitcherLinkOnClick', href: '=appSwitcherLinkHref', + kbnRoute: '=appSwitcherLinkKbnRoute', icon: '=appSwitcherLinkIcon', title: '=appSwitcherLinkTitle' }, - template: appSwitcherLinkTemplate + template: appSwitcherLinkTemplate, + link: scope => { + scope.getHref = () => { + if (scope.href) { + return scope.href; + } + + if (scope.kbnRoute) { + return chrome.addBasePath(scope.kbnRoute); + } + }; + } }; });