Merge pull request elastic#7508 from w33ble/navbar-ext-config

Navbar Extensions in KbnTopNav with Config controls

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';

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';

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';

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;
@@ -31,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);
   });
 

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
@@ -112,82 +112,82 @@ 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, 'set');
-      const is = sinon.spy(controller, 'is');
+      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.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);
       });
     });
 
-    describe('#is(key)', function () {
+    describe('#isCurrent(key)', 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);
       });
     });
 
     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');
       });
     });
 
@@ -197,60 +197,102 @@ describe('KbnTopNavController', function () {
 
         controller.open('foo');
         controller.close();
-        expect(controller.which()).to.be(null);
+        expect(controller.getCurrent()).to.be(null);
       });
     });
 
     describe('#close(key)', 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');
       });
     });
 
     describe('#toggle()', 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);
       });
     });
+
+    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');
+      });
+    });
+
   });
 });

src/ui/public/kbn_top_nav/kbn_top_nav.html

@@ -0,0 +1,21 @@
+<navbar ng-show="chrome.getVisible()" class="kibana-nav-options">
+  <div ng-transclude></div>
+  <div class="button-group kibana-nav-actions" role="toolbar">
+    <button
+      ng-repeat="menuItem in kbnTopNav.menuItems"
+      aria-label="{{::menuItem.description}}"
+      aria-haspopup="{{!menuItem.hasFunction}}"
+      aria-expanded="{{kbnTopNav.isCurrent(menuItem.key)}}"
+      ng-class="{active: kbnTopNav.isCurrent(menuItem.key)}"
+      ng-click="menuItem.run(menuItem, kbnTopNav)"
+      ng-bind="menuItem.label">
+    </button>
+  </div>
+  <kbn-global-timepicker></kbn-global-timepicker>
+</navbar>
+<div class="config" ng-show="kbnTopNav.rendered">
+  <div id="template_wrapper" class="container-fluid"></div>
+  <div class="config-close remove">
+    <i class="fa fa-chevron-circle-up" ng-click="kbnTopNav.close()"></i>
+  </div>
+</div>

src/ui/public/kbn_top_nav/kbn_top_nav.js

@@ -3,7 +3,9 @@ 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';
 
 const module = uiModules.get('kibana');
 
@@ -41,43 +43,27 @@ 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($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 `
-        <navbar ng-show="chrome.getVisible()" class="kibana-nav-options">
-          <div ng-transclude></div>
-          <div class="button-group kibana-nav-actions" role="toolbar">
-            <button
-              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)"
-              ng-bind="menuItem.label">
-            </button>
-            <navbar-extensions name="${$attrs.name}"></navbar-extensions>
-          </div>
-          <kbn-global-timepicker></kbn-global-timepicker>
-        </navbar>
-        <div class="config" ng-show="kbnTopNav.rendered">
-          <div id="template_wrapper" class="container-fluid"></div>
-          <div class="config-close remove">
-            <i class="fa fa-chevron-circle-up" ng-click="kbnTopNav.close()"></i>
-          </div>
-        </div>
-      `;
-    },
-    controller($scope, $compile, $attrs, $element) {
-      const KbnTopNavController = Private(KbnTopNavControllerProvider);
+    template,
+    controller($scope, $attrs, $element) {
+      const extensions = getNavbarExtensions($attrs.name);
+      let controls = _.get($scope, $attrs.config, []);
+      if (controls instanceof KbnTopNavController) {
+        controls.addItems(extensions);
+      } else {
+        controls = controls.concat(extensions);
+      }
 
-      $scope.kbnTopNav = new KbnTopNavController(_.get($scope, $attrs.config));
+      $scope.kbnTopNav = new KbnTopNavController(controls);
       $scope.kbnTopNav._link($scope, $element);
       return $scope.kbnTopNav;
     }