From 9939be453fc4d593c8af1557d8ba803aef516198 Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Thu, 25 Aug 2016 16:16:52 -0700 Subject: [PATCH 1/2] Adding defaults and fixing unit tests --- .../__tests__/kbn_top_nav_controller.js | 32 +++++++++++++++---- .../kbn_top_nav/kbn_top_nav_controller.js | 4 +-- 2 files changed, 27 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 f72f13f2cf083..36614f1e9b7e9 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 @@ -64,16 +64,19 @@ describe('KbnTopNavController', function () { }); describe('hideButton:', function () { - it('defaults to false', function () { + it('defaults to a function that returns false', function () { const controller = new KbnTopNavController([ { key: 'foo' }, { key: '1234' }, ]); - expect(pluck(controller.opts, 'hideButton')).to.eql([false, false]); + pluck(controller.opts, 'hideButton').forEach(prop => { + expect(prop).to.be.a(Function); + expect(prop()).to.eql(false); + }); }); - it('excludes opts from opts when true', function () { + it('excludes opts from opts when set to true', function () { const controller = new KbnTopNavController([ { key: 'foo' }, { key: '1234', hideButton: true }, @@ -81,27 +84,42 @@ describe('KbnTopNavController', function () { expect(controller.menuItems).to.have.length(1); }); + + it('excludes opts from opts when set to a function that returns true', function () { + const controller = new KbnTopNavController([ + { key: 'foo' }, + { key: '1234', hideButton: () => true }, + ]); + + expect(controller.menuItems).to.have.length(1); + }); }); describe('disableButton:', function () { - it('defaults to false', function () { + it('defaults to a function that returns false', function () { const controller = new KbnTopNavController([ { key: 'foo' }, { key: '1234' }, ]); - expect(pluck(controller.opts, 'disableButton')).to.eql([false, false]); + pluck(controller.opts, 'disableButton').forEach(prop => { + expect(prop).to.be.a(Function); + expect(prop()).to.eql(false); + }); }); }); describe('tooltip:', function () { - it('defaults to empty string', function () { + it('defaults to a function that returns undefined', function () { const controller = new KbnTopNavController([ { key: 'foo' }, { key: '1234' }, ]); - expect(pluck(controller.opts, 'tooltip')).to.eql(['', '']); + pluck(controller.opts, 'tooltip').forEach(prop => { + expect(prop).to.be.a(Function); + expect(prop()).to.eql(undefined); + }); }); }); 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 3f475847fef86..2e44d6f0e5a60 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 @@ -65,8 +65,8 @@ export default function ($compile) { run: (item) => this.toggle(item.key) }, opt); - defaultedOpt.hideButton = isFunction(opt.hideButton) ? opt.hideButton : () => opt.hideButton; - defaultedOpt.disableButton = isFunction(opt.disableButton) ? opt.disableButton : () => opt.disableButton; + defaultedOpt.hideButton = isFunction(opt.hideButton) ? opt.hideButton : () => !!opt.hideButton || false; + defaultedOpt.disableButton = isFunction(opt.disableButton) ? opt.disableButton : () => !!opt.disableButton || false; defaultedOpt.tooltip = isFunction(opt.tooltip) ? opt.tooltip : () => opt.tooltip; return defaultedOpt; From 5ca6ad149ef6e2fc85fdaef13916fd82533872e6 Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Thu, 25 Aug 2016 16:30:14 -0700 Subject: [PATCH 2/2] Removing redundant defaults --- src/ui/public/kbn_top_nav/kbn_top_nav_controller.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 2e44d6f0e5a60..43eeaf13f4f5a 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 @@ -65,8 +65,8 @@ export default function ($compile) { run: (item) => this.toggle(item.key) }, opt); - defaultedOpt.hideButton = isFunction(opt.hideButton) ? opt.hideButton : () => !!opt.hideButton || false; - defaultedOpt.disableButton = isFunction(opt.disableButton) ? opt.disableButton : () => !!opt.disableButton || false; + defaultedOpt.hideButton = isFunction(opt.hideButton) ? opt.hideButton : () => !!opt.hideButton; + defaultedOpt.disableButton = isFunction(opt.disableButton) ? opt.disableButton : () => !!opt.disableButton; defaultedOpt.tooltip = isFunction(opt.tooltip) ? opt.tooltip : () => opt.tooltip; return defaultedOpt;