From f46fa877b09aea774c15ecd2cb39fb18ea6e49ab Mon Sep 17 00:00:00 2001 From: Joe Fleming Date: Fri, 10 Jun 2016 14:15:26 -0700 Subject: [PATCH 01/14] move NavbarExtensions registry to kbn_top_nav remove navbar-extensions directive from top nav markup --- src/ui/public/kbn_top_nav/kbn_top_nav.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/ui/public/kbn_top_nav/kbn_top_nav.js b/src/ui/public/kbn_top_nav/kbn_top_nav.js index 9ceca14a8f524..d46f5f8162739 100644 --- a/src/ui/public/kbn_top_nav/kbn_top_nav.js +++ b/src/ui/public/kbn_top_nav/kbn_top_nav.js @@ -4,6 +4,7 @@ import angular from 'angular'; import 'ui/directives/input_focus'; import uiModules from 'ui/modules'; import KbnTopNavControllerProvider from './kbn_top_nav_controller'; +import RegistryNavbarExtensionsProvider from 'ui/registry/navbar_extensions'; const module = uiModules.get('kibana'); @@ -62,7 +63,6 @@ module.directive('kbnTopNav', function (Private) { ng-click="menuItem.run(menuItem)" ng-bind="menuItem.label"> - @@ -76,8 +76,16 @@ module.directive('kbnTopNav', function (Private) { }, controller($scope, $compile, $attrs, $element) { const KbnTopNavController = Private(KbnTopNavControllerProvider); + const navbarExtensions = Private(RegistryNavbarExtensionsProvider); + const getNavbarExtensions = _.memoize(function (name) { + if (!name) throw new Error('navbar directive requires a name attribute'); + return _.sortBy(navbarExtensions.byAppName[name], 'order'); + }); - $scope.kbnTopNav = new KbnTopNavController(_.get($scope, $attrs.config)); + const extensions = getNavbarExtensions($attrs.name); + const controls = _.get($scope, $attrs.config, []).concat(extensions); + + $scope.kbnTopNav = new KbnTopNavController(controls); $scope.kbnTopNav._link($scope, $element); return $scope.kbnTopNav; } From abda2f6934218da5424d9f9736e248a034e57033 Mon Sep 17 00:00:00 2001 From: Joe Fleming Date: Fri, 10 Jun 2016 16:38:06 -0700 Subject: [PATCH 02/14] make top-nav methods a little easier to understand --- src/ui/public/kbn_top_nav/kbn_top_nav.js | 6 +++--- src/ui/public/kbn_top_nav/kbn_top_nav_controller.js | 12 ++++++------ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/ui/public/kbn_top_nav/kbn_top_nav.js b/src/ui/public/kbn_top_nav/kbn_top_nav.js index d46f5f8162739..cb0ade8f3073f 100644 --- a/src/ui/public/kbn_top_nav/kbn_top_nav.js +++ b/src/ui/public/kbn_top_nav/kbn_top_nav.js @@ -58,9 +58,9 @@ module.directive('kbnTopNav', function (Private) { ng-repeat="menuItem in kbnTopNav.menuItems" aria-label="{{::menuItem.description}}" aria-haspopup="{{!menuItem.hasFunction}}" - aria-expanded="{{kbnTopNav.is(menuItem.key)}}" - ng-class="{active: kbnTopNav.is(menuItem.key)}" - ng-click="menuItem.run(menuItem)" + aria-expanded="{{kbnTopNav.isCurrent(menuItem.key)}}" + ng-class="{active: kbnTopNav.isCurrent(menuItem.key)}" + ng-click="menuItem.run(menuItem, kbnTopNav)" ng-bind="menuItem.label"> diff --git a/src/ui/public/kbn_top_nav/kbn_top_nav_controller.js b/src/ui/public/kbn_top_nav/kbn_top_nav_controller.js index b74917fb68e7e..01cefc32b1385 100644 --- a/src/ui/public/kbn_top_nav/kbn_top_nav_controller.js +++ b/src/ui/public/kbn_top_nav/kbn_top_nav_controller.js @@ -29,7 +29,7 @@ export default function ($compile) { } // change the current key and rerender - set(key) { + setCurrent(key) { if (key && !this.templates.hasOwnProperty(key)) { throw new TypeError(`KbnTopNav: unknown template key "${key}"`); } @@ -39,11 +39,11 @@ export default function ($compile) { } // little usability helpers - which() { return this.currentKey; } - is(key) { return this.which() === key; } - open(key) { this.set(key); } - close(key) { (!key || this.is(key)) && this.set(null); } - toggle(key) { this.set(this.is(key) ? null : key); } + getCurrent() { return this.currentKey; } + isCurrent(key) { return this.getCurrent() === key; } + open(key) { this.setCurrent(key); } + close(key) { (!key || this.isCurrent(key)) && this.setCurrent(null); } + toggle(key) { this.setCurrent(this.isCurrent(key) ? null : key); } // apply the defaults to individual options _applyOptDefault(opt = {}) { From 653e83f1c34838fb1de8f4a468b74f489bcbf1ee Mon Sep 17 00:00:00 2001 From: Joe Fleming Date: Fri, 10 Jun 2016 16:38:27 -0700 Subject: [PATCH 03/14] allow top-nav controls to pass label --- src/ui/public/kbn_top_nav/kbn_top_nav_controller.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ui/public/kbn_top_nav/kbn_top_nav_controller.js b/src/ui/public/kbn_top_nav/kbn_top_nav_controller.js index 01cefc32b1385..01509b0d769d8 100644 --- a/src/ui/public/kbn_top_nav/kbn_top_nav_controller.js +++ b/src/ui/public/kbn_top_nav/kbn_top_nav_controller.js @@ -48,7 +48,7 @@ export default function ($compile) { // apply the defaults to individual options _applyOptDefault(opt = {}) { return defaults({}, opt, { - label: capitalize(opt.key), + label: opt.label || capitalize(opt.key), hasFunction: !!opt.run, description: opt.run ? opt.key : `Toggle ${opt.key} view`, hideButton: !!opt.hideButton, From d69d60f02dd09a55a7aa9c78804190dd4745d1a6 Mon Sep 17 00:00:00 2001 From: Joe Fleming Date: Fri, 10 Jun 2016 16:38:42 -0700 Subject: [PATCH 04/14] remove unused $compile dependency --- src/ui/public/kbn_top_nav/kbn_top_nav.js | 2 +- src/ui/public/navbar_extensions/navbar_extensions.js | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/ui/public/kbn_top_nav/kbn_top_nav.js b/src/ui/public/kbn_top_nav/kbn_top_nav.js index cb0ade8f3073f..3bd119d7a02e5 100644 --- a/src/ui/public/kbn_top_nav/kbn_top_nav.js +++ b/src/ui/public/kbn_top_nav/kbn_top_nav.js @@ -74,7 +74,7 @@ module.directive('kbnTopNav', function (Private) { `; }, - controller($scope, $compile, $attrs, $element) { + controller($scope, $attrs, $element) { const KbnTopNavController = Private(KbnTopNavControllerProvider); const navbarExtensions = Private(RegistryNavbarExtensionsProvider); const getNavbarExtensions = _.memoize(function (name) { diff --git a/src/ui/public/navbar_extensions/navbar_extensions.js b/src/ui/public/navbar_extensions/navbar_extensions.js index b86fae8972c60..362e6f38b2b32 100644 --- a/src/ui/public/navbar_extensions/navbar_extensions.js +++ b/src/ui/public/navbar_extensions/navbar_extensions.js @@ -5,8 +5,7 @@ import RegistryNavbarExtensionsProvider from 'ui/registry/navbar_extensions'; import uiModules from 'ui/modules'; const navbar = uiModules.get('kibana/navbar'); - -navbar.directive('navbarExtensions', function (Private, $compile) { +navbar.directive('navbarExtensions', function (Private) { const navbarExtensions = Private(RegistryNavbarExtensionsProvider); const getExtensions = _.memoize(function (name) { if (!name) throw new Error('navbar directive requires a name attribute'); From a16e7203967ba5cb69bf11f54aa4d06e8ec697f5 Mon Sep 17 00:00:00 2001 From: Joe Fleming Date: Mon, 20 Jun 2016 11:15:21 -0400 Subject: [PATCH 05/14] remove references to navbar_extensions --- src/plugins/kibana/public/dashboard/index.js | 1 - src/plugins/kibana/public/discover/index.js | 1 - src/plugins/kibana/public/visualize/editor/editor.js | 1 - src/ui/public/kbn_top_nav/__tests__/kbn_top_nav.js | 1 - src/ui/public/kbn_top_nav/kbn_top_nav.js | 4 ---- .../public/navbar_extensions/__tests__/navbar_extensions.js | 1 - 6 files changed, 9 deletions(-) diff --git a/src/plugins/kibana/public/dashboard/index.js b/src/plugins/kibana/public/dashboard/index.js index e8322ea212c16..40fbd0934ebce 100644 --- a/src/plugins/kibana/public/dashboard/index.js +++ b/src/plugins/kibana/public/dashboard/index.js @@ -6,7 +6,6 @@ import 'ui/courier'; import 'ui/config'; import 'ui/notify'; import 'ui/typeahead'; -import 'ui/navbar_extensions'; import 'ui/share'; import 'plugins/kibana/dashboard/directives/grid'; import 'plugins/kibana/dashboard/components/panel/panel'; diff --git a/src/plugins/kibana/public/discover/index.js b/src/plugins/kibana/public/discover/index.js index 2888306ae454f..0a11459aa5602 100644 --- a/src/plugins/kibana/public/discover/index.js +++ b/src/plugins/kibana/public/discover/index.js @@ -1,7 +1,6 @@ import 'plugins/kibana/discover/saved_searches/saved_searches'; import 'plugins/kibana/discover/directives/no_results'; import 'plugins/kibana/discover/directives/timechart'; -import 'ui/navbar_extensions'; import 'ui/collapsible_sidebar'; import 'plugins/kibana/discover/components/field_chooser/field_chooser'; import 'plugins/kibana/discover/controllers/discover'; diff --git a/src/plugins/kibana/public/visualize/editor/editor.js b/src/plugins/kibana/public/visualize/editor/editor.js index 580bbb0aec632..552f6c01c4b8f 100644 --- a/src/plugins/kibana/public/visualize/editor/editor.js +++ b/src/plugins/kibana/public/visualize/editor/editor.js @@ -2,7 +2,6 @@ import _ from 'lodash'; import 'plugins/kibana/visualize/saved_visualizations/saved_visualizations'; import 'plugins/kibana/visualize/editor/sidebar'; import 'plugins/kibana/visualize/editor/agg_filter'; -import 'ui/navbar_extensions'; import 'ui/visualize'; import 'ui/collapsible_sidebar'; import 'ui/share'; diff --git a/src/ui/public/kbn_top_nav/__tests__/kbn_top_nav.js b/src/ui/public/kbn_top_nav/__tests__/kbn_top_nav.js index 57b23e6a5b4af..1f6250da2cc84 100644 --- a/src/ui/public/kbn_top_nav/__tests__/kbn_top_nav.js +++ b/src/ui/public/kbn_top_nav/__tests__/kbn_top_nav.js @@ -7,7 +7,6 @@ import '../kbn_top_nav'; import KbnTopNavControllerProvider from '../kbn_top_nav_controller'; import navbarExtensionsRegistry from 'ui/registry/navbar_extensions'; import Registry from 'ui/registry/_registry'; -import 'ui/navbar_extensions'; describe('kbnTopNav directive', function () { let build; diff --git a/src/ui/public/kbn_top_nav/kbn_top_nav.js b/src/ui/public/kbn_top_nav/kbn_top_nav.js index 3bd119d7a02e5..adc695dc97a1f 100644 --- a/src/ui/public/kbn_top_nav/kbn_top_nav.js +++ b/src/ui/public/kbn_top_nav/kbn_top_nav.js @@ -46,10 +46,6 @@ module.directive('kbnTopNav', function (Private) { restrict: 'E', transclude: true, template($el, $attrs) { - // This is ugly - // This is necessary because of navbar-extensions - // It will no accept any programatic way of setting its name - // besides this because it happens so early in the digest cycle return `
diff --git a/src/ui/public/navbar_extensions/__tests__/navbar_extensions.js b/src/ui/public/navbar_extensions/__tests__/navbar_extensions.js index 04518f44d5387..3bc6c8c4b930d 100644 --- a/src/ui/public/navbar_extensions/__tests__/navbar_extensions.js +++ b/src/ui/public/navbar_extensions/__tests__/navbar_extensions.js @@ -6,7 +6,6 @@ import _ from 'lodash'; import navbarExtensionsRegistry from 'ui/registry/navbar_extensions'; import Registry from 'ui/registry/_registry'; -import 'ui/navbar_extensions'; const defaultMarkup = ` `; From 41a84d2e9f2706c0cb80390df25bf2272baa102c Mon Sep 17 00:00:00 2001 From: Joe Fleming Date: Mon, 20 Jun 2016 11:15:45 -0400 Subject: [PATCH 06/14] remove the navbar_extensions directive --- .../__tests__/navbar_extensions.js | 105 ------------------ .../navbar_extensions/navbar_extensions.js | 41 ------- 2 files changed, 146 deletions(-) delete mode 100644 src/ui/public/navbar_extensions/__tests__/navbar_extensions.js delete mode 100644 src/ui/public/navbar_extensions/navbar_extensions.js diff --git a/src/ui/public/navbar_extensions/__tests__/navbar_extensions.js b/src/ui/public/navbar_extensions/__tests__/navbar_extensions.js deleted file mode 100644 index 3bc6c8c4b930d..0000000000000 --- a/src/ui/public/navbar_extensions/__tests__/navbar_extensions.js +++ /dev/null @@ -1,105 +0,0 @@ -import ngMock from 'ng_mock'; -import sinon from 'sinon'; -import expect from 'expect.js'; -import angular from 'angular'; -import _ from 'lodash'; - -import navbarExtensionsRegistry from 'ui/registry/navbar_extensions'; -import Registry from 'ui/registry/_registry'; - -const defaultMarkup = ` - `; - - -describe('navbar-extensions directive', function () { - let $rootScope; - let $compile; - let stubRegistry; - - beforeEach(function () { - ngMock.module('kibana', function (PrivateProvider) { - stubRegistry = new Registry({ - index: ['name'], - group: ['appName'], - order: ['order'] - }); - - PrivateProvider.swap(navbarExtensionsRegistry, stubRegistry); - }); - - ngMock.module('kibana/navbar'); - - // Create the scope - ngMock.inject(function ($injector) { - $rootScope = $injector.get('$rootScope'); - $compile = $injector.get('$compile'); - }); - }); - - function init(markup = defaultMarkup) { - // Give us a scope - const $el = angular.element(markup); - $compile($el)($rootScope); - $el.scope().$digest(); - return $el; - } - - describe('incorrect use', function () { - it('should throw if missing a name property', function () { - const markup = ``; - expect(() => init(markup)).to.throwException(/requires a name attribute/); - }); - }); - - describe('injecting extensions', function () { - function registerExtension(def = {}) { - stubRegistry.register(function () { - return _.defaults(def, { - name: 'exampleButton', - appName: 'testing', - order: 0, - template: ` - ` - }); - }); - } - - it('should append to end then order == 0', function () { - registerExtension({ order: 0 }); - let $el = init(); - - expect($el.find('button').last().hasClass('test-button')).to.be.ok(); - }); - - it('should enforce the order prop', function () { - registerExtension({ - order: 1, - template: ` - ` - }); - registerExtension({ - order: 2, - template: ` - ` - }); - registerExtension({ - order: 0, - template: ` - ` - }); - let $el = init(); - - expect($el.find('button').length).to.equal(3); - expect($el.find('button').last().hasClass('test-button-2')).to.be.ok(); - expect($el.find('button').first().hasClass('test-button-0')).to.be.ok(); - }); - }); -}); diff --git a/src/ui/public/navbar_extensions/navbar_extensions.js b/src/ui/public/navbar_extensions/navbar_extensions.js deleted file mode 100644 index 362e6f38b2b32..0000000000000 --- a/src/ui/public/navbar_extensions/navbar_extensions.js +++ /dev/null @@ -1,41 +0,0 @@ -import _ from 'lodash'; -import $ from 'jquery'; -import 'ui/render_directive'; -import RegistryNavbarExtensionsProvider from 'ui/registry/navbar_extensions'; -import uiModules from 'ui/modules'; -const navbar = uiModules.get('kibana/navbar'); - -navbar.directive('navbarExtensions', function (Private) { - const navbarExtensions = Private(RegistryNavbarExtensionsProvider); - const getExtensions = _.memoize(function (name) { - if (!name) throw new Error('navbar directive requires a name attribute'); - return _.sortBy(navbarExtensions.byAppName[name], 'order'); - }); - - return { - restrict: 'E', - template: function ($el, $attrs) { - const extensions = getExtensions($attrs.name); - const controls = extensions.map(function (extension, i) { - return { - order: extension.order, - index: i, - extension: extension, - }; - }); - - _.sortBy(controls, 'order').forEach(function (control) { - const { extension, index } = control; - const $ext = $(``); - $ext.html(extension.template); - $el.append($ext); - }); - - return $el.html(); - }, - controllerAs: 'navbar', - controller: function ($attrs) { - this.extensions = getExtensions($attrs.name); - } - }; -}); From a0630a63b69219d4105a76a73f56449fcf9a5825 Mon Sep 17 00:00:00 2001 From: Joe Fleming Date: Mon, 20 Jun 2016 12:18:27 -0400 Subject: [PATCH 07/14] update tests for method renaming --- .../kbn_top_nav/__tests__/kbn_top_nav.js | 2 +- .../__tests__/kbn_top_nav_controller.js | 78 +++++++++---------- 2 files changed, 40 insertions(+), 40 deletions(-) diff --git a/src/ui/public/kbn_top_nav/__tests__/kbn_top_nav.js b/src/ui/public/kbn_top_nav/__tests__/kbn_top_nav.js index 1f6250da2cc84..8d3b213db66db 100644 --- a/src/ui/public/kbn_top_nav/__tests__/kbn_top_nav.js +++ b/src/ui/public/kbn_top_nav/__tests__/kbn_top_nav.js @@ -30,7 +30,7 @@ describe('kbnTopNav directive', function () { const { $scope } = build(); expect($scope.kbnTopNav.open).to.be.a(Function); expect($scope.kbnTopNav.close).to.be.a(Function); - expect($scope.kbnTopNav.is).to.be.a(Function); + expect($scope.kbnTopNav.getCurrent).to.be.a(Function); expect($scope.kbnTopNav.toggle).to.be.a(Function); }); diff --git a/src/ui/public/kbn_top_nav/__tests__/kbn_top_nav_controller.js b/src/ui/public/kbn_top_nav/__tests__/kbn_top_nav_controller.js index 5b4f86333ef08..8821246d75073 100644 --- a/src/ui/public/kbn_top_nav/__tests__/kbn_top_nav_controller.js +++ b/src/ui/public/kbn_top_nav/__tests__/kbn_top_nav_controller.js @@ -94,11 +94,11 @@ describe('KbnTopNavController', function () { const opt = controller.opts[0]; expect(opt.run).to.be.a('function'); - expect(controller.which()).to.not.be(opt.key); + expect(controller.getCurrent()).to.not.be(opt.key); opt.run(opt); - expect(controller.which()).to.be(opt.key); + expect(controller.getCurrent()).to.be(opt.key); opt.run(opt); - expect(controller.which()).to.not.be(opt.key); + expect(controller.getCurrent()).to.not.be(opt.key); }); it('uses the supplied run function otherwise', function (done) { // eslint-disable-line mocha/handle-done-callback @@ -119,8 +119,8 @@ describe('KbnTopNavController', function () { { key: 'bar', template: 'Whisper Bar' }, ]); const render = sinon.spy(controller, '_render'); - const set = sinon.spy(controller, 'set'); - const is = sinon.spy(controller, 'is'); + const set = sinon.spy(controller, 'setCurrent'); + const is = sinon.spy(controller, 'getCurrent'); return { controller, render, set }; }; @@ -128,41 +128,41 @@ describe('KbnTopNavController', function () { describe('#set([key])', function () { it('assigns the passed key to the current key', function () { const { controller } = init(); - expect(controller.which()).to.not.be('foo'); - controller.set('foo'); - expect(controller.which()).to.be('foo'); + expect(controller.getCurrent()).to.not.be('foo'); + controller.setCurrent('foo'); + expect(controller.getCurrent()).to.be('foo'); }); it('throws if the key does not match a known template', function () { const { controller } = init(); expect(function () { - controller.set('june'); + controller.setCurrent('june'); }).to.throwError(/unknown template key/); }); it('sets to "null" for falsy values', function () { const { controller } = init(); - controller.set(); - expect(controller.which()).to.be(null); + controller.setCurrent(); + expect(controller.getCurrent()).to.be(null); - controller.set(false); - expect(controller.which()).to.be(null); + controller.setCurrent(false); + expect(controller.getCurrent()).to.be(null); - controller.set(null); - expect(controller.which()).to.be(null); + controller.setCurrent(null); + expect(controller.getCurrent()).to.be(null); - controller.set(''); - expect(controller.which()).to.be(null); + controller.setCurrent(''); + expect(controller.getCurrent()).to.be(null); }); it('rerenders after setting', function () { const { controller, render } = init(); sinon.assert.notCalled(render); - controller.set('bar'); + controller.setCurrent('bar'); sinon.assert.calledOnce(render); - controller.set('bar'); + controller.setCurrent('bar'); sinon.assert.calledTwice(render); }); }); @@ -171,13 +171,13 @@ describe('KbnTopNavController', function () { it('returns true when key matches', function () { const { controller } = init(); - controller.set('foo'); - expect(controller.is('foo')).to.be(true); - expect(controller.is('bar')).to.be(false); + controller.setCurrent('foo'); + expect(controller.isCurrent('foo')).to.be(true); + expect(controller.isCurrent('bar')).to.be(false); - controller.set('bar'); - expect(controller.is('bar')).to.be(true); - expect(controller.is('foo')).to.be(false); + controller.setCurrent('bar'); + expect(controller.isCurrent('bar')).to.be(true); + expect(controller.isCurrent('foo')).to.be(false); }); }); @@ -197,7 +197,7 @@ describe('KbnTopNavController', function () { controller.open('foo'); controller.close(); - expect(controller.which()).to.be(null); + expect(controller.getCurrent()).to.be(null); }); }); @@ -205,23 +205,23 @@ describe('KbnTopNavController', function () { it('sets to null if key is open', function () { const { controller } = init(); - expect(controller.which()).to.be(null); + expect(controller.getCurrent()).to.be(null); controller.close('foo'); - expect(controller.which()).to.be(null); + expect(controller.getCurrent()).to.be(null); controller.open('foo'); - expect(controller.which()).to.be('foo'); + expect(controller.getCurrent()).to.be('foo'); controller.close('foo'); - expect(controller.which()).to.be(null); + expect(controller.getCurrent()).to.be(null); }); it('ignores if other key is open', function () { const { controller } = init(); - expect(controller.which()).to.be(null); + expect(controller.getCurrent()).to.be(null); controller.open('foo'); - expect(controller.which()).to.be('foo'); + expect(controller.getCurrent()).to.be('foo'); controller.close('bar'); - expect(controller.which()).to.be('foo'); + expect(controller.getCurrent()).to.be('foo'); }); }); @@ -229,27 +229,27 @@ describe('KbnTopNavController', function () { it('opens if closed', function () { const { controller } = init(); - expect(controller.which()).to.be(null); + expect(controller.getCurrent()).to.be(null); controller.toggle('foo'); - expect(controller.which()).to.be('foo'); + expect(controller.getCurrent()).to.be('foo'); }); it('opens if other is open', function () { const { controller } = init(); controller.open('bar'); - expect(controller.which()).to.be('bar'); + expect(controller.getCurrent()).to.be('bar'); controller.toggle('foo'); - expect(controller.which()).to.be('foo'); + expect(controller.getCurrent()).to.be('foo'); }); it('closes if open', function () { const { controller } = init(); controller.open('bar'); - expect(controller.which()).to.be('bar'); + expect(controller.getCurrent()).to.be('bar'); controller.toggle('bar'); - expect(controller.which()).to.be(null); + expect(controller.getCurrent()).to.be(null); }); }); }); From 8c6cdd91b1c6798272e39fd4f72e9420dc9bac9a Mon Sep 17 00:00:00 2001 From: Joe Fleming Date: Mon, 20 Jun 2016 12:25:56 -0400 Subject: [PATCH 08/14] add addItems method, handle passing in KbnTopNavController instance --- src/ui/public/kbn_top_nav/kbn_top_nav.js | 7 ++++++- src/ui/public/kbn_top_nav/kbn_top_nav_controller.js | 10 ++++++++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/ui/public/kbn_top_nav/kbn_top_nav.js b/src/ui/public/kbn_top_nav/kbn_top_nav.js index adc695dc97a1f..9961977bbcf5d 100644 --- a/src/ui/public/kbn_top_nav/kbn_top_nav.js +++ b/src/ui/public/kbn_top_nav/kbn_top_nav.js @@ -79,7 +79,12 @@ module.directive('kbnTopNav', function (Private) { }); const extensions = getNavbarExtensions($attrs.name); - const controls = _.get($scope, $attrs.config, []).concat(extensions); + const controls = _.get($scope, $attrs.config, []); + if (controls instanceof KbnTopNavController) { + controls.addItems(extensions); + } else { + controls.concat(extensions); + } $scope.kbnTopNav = new KbnTopNavController(controls); $scope.kbnTopNav._link($scope, $element); diff --git a/src/ui/public/kbn_top_nav/kbn_top_nav_controller.js b/src/ui/public/kbn_top_nav/kbn_top_nav_controller.js index 01509b0d769d8..364c8d96b4984 100644 --- a/src/ui/public/kbn_top_nav/kbn_top_nav_controller.js +++ b/src/ui/public/kbn_top_nav/kbn_top_nav_controller.js @@ -1,4 +1,4 @@ -import { defaults, capitalize } from 'lodash'; +import { defaults, capitalize, isArray } from 'lodash'; import uiModules from 'ui/modules'; import filterTemplate from 'ui/chrome/config/filter.html'; @@ -19,7 +19,13 @@ export default function ($compile) { filter: filterTemplate, }; - opts.forEach(rawOpt => { + this.addItems(opts); + } + + addItems(rawOpts) { + if (!isArray(rawOpts)) rawOpts = [rawOpts]; + + rawOpts.forEach((rawOpt) => { const opt = this._applyOptDefault(rawOpt); if (!opt.key) throw new TypeError('KbnTopNav: menu items must have a key'); this.opts.push(opt); From 8ee0a4b1e8601b1c53d76e50705f59faad023dd9 Mon Sep 17 00:00:00 2001 From: Joe Fleming Date: Mon, 20 Jun 2016 12:57:22 -0400 Subject: [PATCH 09/14] additional renaming --- .../__tests__/kbn_top_nav_controller.js | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/ui/public/kbn_top_nav/__tests__/kbn_top_nav_controller.js b/src/ui/public/kbn_top_nav/__tests__/kbn_top_nav_controller.js index 8821246d75073..52da893e47dc7 100644 --- a/src/ui/public/kbn_top_nav/__tests__/kbn_top_nav_controller.js +++ b/src/ui/public/kbn_top_nav/__tests__/kbn_top_nav_controller.js @@ -112,20 +112,20 @@ describe('KbnTopNavController', function () { }); }); - describe('', function () { + describe('methods', function () { const init = function () { const controller = new KbnTopNavController([ { key: 'foo', template: 'Say Foo!' }, { key: 'bar', template: 'Whisper Bar' }, ]); const render = sinon.spy(controller, '_render'); - const set = sinon.spy(controller, 'setCurrent'); - const is = sinon.spy(controller, 'getCurrent'); + const setCurrent = sinon.spy(controller, 'setCurrent'); + const getCurrent = sinon.spy(controller, 'getCurrent'); - return { controller, render, set }; + return { controller, render, setCurrent, getCurrent }; }; - describe('#set([key])', function () { + describe('#setCurrent([key])', function () { it('assigns the passed key to the current key', function () { const { controller } = init(); expect(controller.getCurrent()).to.not.be('foo'); @@ -167,7 +167,7 @@ describe('KbnTopNavController', function () { }); }); - describe('#is(key)', function () { + describe('#isCurrent(key)', function () { it('returns true when key matches', function () { const { controller } = init(); @@ -183,11 +183,11 @@ describe('KbnTopNavController', function () { describe('#open(key)', function () { it('alias for set', function () { - const { controller, set } = init(); + const { controller, setCurrent } = init(); controller.open('foo'); - sinon.assert.calledOnce(set); - sinon.assert.calledWithExactly(set, 'foo'); + sinon.assert.calledOnce(setCurrent); + sinon.assert.calledWithExactly(setCurrent, 'foo'); }); }); From 654ddd737a1f8c5a42ea3792ff757b7cac04461c Mon Sep 17 00:00:00 2001 From: Joe Fleming Date: Mon, 20 Jun 2016 12:57:45 -0400 Subject: [PATCH 10/14] add tests for addItems method --- .../__tests__/kbn_top_nav_controller.js | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/src/ui/public/kbn_top_nav/__tests__/kbn_top_nav_controller.js b/src/ui/public/kbn_top_nav/__tests__/kbn_top_nav_controller.js index 52da893e47dc7..7a72d3e3754f4 100644 --- a/src/ui/public/kbn_top_nav/__tests__/kbn_top_nav_controller.js +++ b/src/ui/public/kbn_top_nav/__tests__/kbn_top_nav_controller.js @@ -252,5 +252,47 @@ describe('KbnTopNavController', function () { expect(controller.getCurrent()).to.be(null); }); }); + + describe('#addItems(opts)', function () { + it('should append to existing menu items', function () { + const { controller } = init(); + const newItems = [ + { key: 'green', template: 'Green means go' }, + { key: 'red', template: 'Red means stop' }, + ]; + + expect(controller.menuItems).to.have.length(2); + controller.addItems(newItems); + expect(controller.menuItems).to.have.length(4); + + // check that the items were added + var matches = controller.menuItems.reduce((acc, item) => { + if (item.key === 'green' || item.key === 'red') { + acc[item.key] = item; + } + return acc; + }, {}); + expect(matches).to.have.property('green'); + expect(matches.green).to.have.property('run'); + expect(matches).to.have.property('red'); + expect(matches.red).to.have.property('run'); + }); + + it('should take a single menu item object', function () { + const { controller } = init(); + const newItem = { key: 'green', template: 'Green means go' }; + + expect(controller.menuItems).to.have.length(2); + controller.addItems(newItem); + expect(controller.menuItems).to.have.length(3); + + // check that the items were added + var match = controller.menuItems.filter((item) => { + return item.key === 'green'; + }); + expect(match[0]).to.have.property('run'); + }); + }); + }); }); From eacb9484ddbc79f6c79b0b23dd008645da41c28b Mon Sep 17 00:00:00 2001 From: Joe Fleming Date: Mon, 20 Jun 2016 14:25:00 -0400 Subject: [PATCH 11/14] fix concatenating the extension controls --- src/ui/public/kbn_top_nav/kbn_top_nav.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ui/public/kbn_top_nav/kbn_top_nav.js b/src/ui/public/kbn_top_nav/kbn_top_nav.js index 9961977bbcf5d..78ce0e2445bae 100644 --- a/src/ui/public/kbn_top_nav/kbn_top_nav.js +++ b/src/ui/public/kbn_top_nav/kbn_top_nav.js @@ -79,11 +79,11 @@ module.directive('kbnTopNav', function (Private) { }); const extensions = getNavbarExtensions($attrs.name); - const controls = _.get($scope, $attrs.config, []); + let controls = _.get($scope, $attrs.config, []); if (controls instanceof KbnTopNavController) { controls.addItems(extensions); } else { - controls.concat(extensions); + controls = controls.concat(extensions); } $scope.kbnTopNav = new KbnTopNavController(controls); From 5985252beec0a86ea0b083d3b5a14b596b74dcc6 Mon Sep 17 00:00:00 2001 From: Joe Fleming Date: Tue, 21 Jun 2016 10:12:55 -0400 Subject: [PATCH 12/14] remove redundant prop definition --- src/ui/public/kbn_top_nav/kbn_top_nav_controller.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ui/public/kbn_top_nav/kbn_top_nav_controller.js b/src/ui/public/kbn_top_nav/kbn_top_nav_controller.js index 364c8d96b4984..80916038e1c90 100644 --- a/src/ui/public/kbn_top_nav/kbn_top_nav_controller.js +++ b/src/ui/public/kbn_top_nav/kbn_top_nav_controller.js @@ -54,7 +54,7 @@ export default function ($compile) { // apply the defaults to individual options _applyOptDefault(opt = {}) { return defaults({}, opt, { - label: opt.label || capitalize(opt.key), + label: capitalize(opt.key), hasFunction: !!opt.run, description: opt.run ? opt.key : `Toggle ${opt.key} view`, hideButton: !!opt.hideButton, From 173c8bc2c9e76c0be1bfb6dab68b3ba464906b77 Mon Sep 17 00:00:00 2001 From: Joe Fleming Date: Tue, 21 Jun 2016 10:14:36 -0400 Subject: [PATCH 13/14] move top nav template to its own file now that we no longer need to inject values from the code --- src/ui/public/kbn_top_nav/kbn_top_nav.html | 21 +++++++++++++++++ src/ui/public/kbn_top_nav/kbn_top_nav.js | 27 ++-------------------- 2 files changed, 23 insertions(+), 25 deletions(-) create mode 100644 src/ui/public/kbn_top_nav/kbn_top_nav.html diff --git a/src/ui/public/kbn_top_nav/kbn_top_nav.html b/src/ui/public/kbn_top_nav/kbn_top_nav.html new file mode 100644 index 0000000000000..5bfc1cfaf7ae1 --- /dev/null +++ b/src/ui/public/kbn_top_nav/kbn_top_nav.html @@ -0,0 +1,21 @@ + +
+ + + +
+
+
+ +
+
\ No newline at end of file diff --git a/src/ui/public/kbn_top_nav/kbn_top_nav.js b/src/ui/public/kbn_top_nav/kbn_top_nav.js index 78ce0e2445bae..29ceb7572d259 100644 --- a/src/ui/public/kbn_top_nav/kbn_top_nav.js +++ b/src/ui/public/kbn_top_nav/kbn_top_nav.js @@ -3,6 +3,7 @@ import 'ui/watch_multi'; import angular from 'angular'; import 'ui/directives/input_focus'; import uiModules from 'ui/modules'; +import template from './kbn_top_nav.html'; import KbnTopNavControllerProvider from './kbn_top_nav_controller'; import RegistryNavbarExtensionsProvider from 'ui/registry/navbar_extensions'; @@ -45,31 +46,7 @@ module.directive('kbnTopNav', function (Private) { return { restrict: 'E', transclude: true, - template($el, $attrs) { - return ` - -
- - - -
-
-
- -
-
- `; - }, + template, controller($scope, $attrs, $element) { const KbnTopNavController = Private(KbnTopNavControllerProvider); const navbarExtensions = Private(RegistryNavbarExtensionsProvider); From 895aabc3a3d842f0699d3b4de7a4db4ff2d43eb7 Mon Sep 17 00:00:00 2001 From: Joe Fleming Date: Tue, 21 Jun 2016 10:18:18 -0400 Subject: [PATCH 14/14] move modules and helpers out of controller makes the include happen once, and the memoize actually work as expected --- src/ui/public/kbn_top_nav/kbn_top_nav.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/ui/public/kbn_top_nav/kbn_top_nav.js b/src/ui/public/kbn_top_nav/kbn_top_nav.js index 29ceb7572d259..e0ba3cba16b34 100644 --- a/src/ui/public/kbn_top_nav/kbn_top_nav.js +++ b/src/ui/public/kbn_top_nav/kbn_top_nav.js @@ -43,18 +43,18 @@ const module = uiModules.get('kibana'); * Programatic control of the navbar can be acheived one of two ways */ module.directive('kbnTopNav', function (Private) { + const KbnTopNavController = Private(KbnTopNavControllerProvider); + const navbarExtensions = Private(RegistryNavbarExtensionsProvider); + const getNavbarExtensions = _.memoize(function (name) { + if (!name) throw new Error('navbar directive requires a name attribute'); + return _.sortBy(navbarExtensions.byAppName[name], 'order'); + }); + return { restrict: 'E', transclude: true, template, controller($scope, $attrs, $element) { - const KbnTopNavController = Private(KbnTopNavControllerProvider); - const navbarExtensions = Private(RegistryNavbarExtensionsProvider); - const getNavbarExtensions = _.memoize(function (name) { - if (!name) throw new Error('navbar directive requires a name attribute'); - return _.sortBy(navbarExtensions.byAppName[name], 'order'); - }); - const extensions = getNavbarExtensions($attrs.name); let controls = _.get($scope, $attrs.config, []); if (controls instanceof KbnTopNavController) {