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

Create "chromeConfigControls" for plugins to add chrome config sections #6151

Closed
wants to merge 9 commits into from
1 change: 0 additions & 1 deletion src/ui/public/autoload/modules.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import 'angular';
import 'ui/chrome';
import 'ui/chrome/context';
import 'ui/bind';
import 'ui/bound_to_config_obj';
import 'ui/config';
Expand Down
54 changes: 54 additions & 0 deletions src/ui/public/chrome/__tests__/config_controls.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import ngMock from 'ngMock';
import $ from 'jquery';
import expect from 'expect.js';

import uiModules from 'ui/modules';
import chromeConfigControlsRegistry from 'ui/registry/chrome_config_controls';
import Registry from 'ui/registry/_registry';
import 'ui/chrome/directives/config_controls';
import 'ui/directives/config';

describe('chrome config controls', function () {
let compile;
let stubRegistry;

beforeEach(ngMock.module('kibana', function (PrivateProvider) {
stubRegistry = new Registry({
order: ['order']
});

PrivateProvider.swap(chromeConfigControlsRegistry, stubRegistry);
}));

beforeEach(ngMock.inject(function ($compile, $rootScope) {
compile = function () {
const $el = $('<kbn-chrome-config-controls></kbn-chrome-config-controls>');
let $scope = $rootScope.$new();
$compile($el)($scope);
$scope.$digest();
return $el;
};
}));

it('injects configs from the ui/registry/chrome_config_controls registry', function () {
stubRegistry.register(function () {
return {
name: 'control1',
order: 1,
config: {
}
};
});
stubRegistry.register(function () {
return {
name: 'control2',
order: 2,
config: {
}
};
});

var $el = compile();
expect($el.find('config')).to.have.length(2);
});
});
41 changes: 33 additions & 8 deletions src/ui/public/chrome/__tests__/nav_controls.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,26 @@ import expect from 'expect.js';

import uiModules from 'ui/modules';
import chromeNavControlsRegistry from 'ui/registry/chrome_nav_controls';
import chromeConfigControlsRegistry from 'ui/registry/chrome_config_controls';
import Registry from 'ui/registry/_registry';

describe('chrome nav controls', function () {
let compile;
let stubRegistry;
let stubNavRegistry;
let stubConfigRegistry;

beforeEach(ngMock.module('kibana', function (PrivateProvider) {
stubRegistry = new Registry({
stubNavRegistry = new Registry({
order: ['order']
});

PrivateProvider.swap(chromeNavControlsRegistry, stubRegistry);
PrivateProvider.swap(chromeNavControlsRegistry, stubNavRegistry);

stubConfigRegistry = new Registry({
order: ['order']
});

PrivateProvider.swap(chromeConfigControlsRegistry, stubConfigRegistry);
}));

beforeEach(ngMock.inject(function ($compile, $rootScope) {
Expand All @@ -28,7 +36,7 @@ describe('chrome nav controls', function () {
}));

it('injects templates from the ui/registry/chrome_nav_controls registry', function () {
stubRegistry.register(function () {
stubNavRegistry.register(function () {
return {
name: 'control',
order: 100,
Expand All @@ -40,22 +48,39 @@ describe('chrome nav controls', function () {
expect($el.find('#testTemplateEl')).to.have.length(1);
});

it('injects templates from the ui/registry/chrome_config_controls registry', function () {
stubConfigRegistry.register(function () {
return {
name: 'control',
order: 100,
navbar: {
template: `<span id="testTemplateEl"></span>`
}
};
});

var $el = compile();
expect($el.find('#testTemplateEl')).to.have.length(1);
});

it('renders controls in reverse order, assuming that each control will float:right', function () {
stubRegistry.register(function () {
stubConfigRegistry.register(function () {
return {
name: 'control2',
order: 2,
template: `<span id="2", class="testControl"></span>`
navbar: {
template: `<span id="2", class="testControl"></span>`
}
};
});
stubRegistry.register(function () {
stubNavRegistry.register(function () {
return {
name: 'control1',
order: 1,
template: `<span id="1", class="testControl"></span>`
};
});
stubRegistry.register(function () {
stubNavRegistry.register(function () {
return {
name: 'control3',
order: 3,
Expand Down
10 changes: 2 additions & 8 deletions src/ui/public/chrome/chrome.html
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<kbn-notifications list="notifList"></kbn-notifications>
<div class="content" chrome-context >
<div class="content">
<nav
ng-style="::{ background: chrome.getNavBackground() }"
ng-class="{ show: chrome.getVisible() }"
Expand Down Expand Up @@ -56,19 +56,13 @@
<!-- /Full navbar -->
</nav>

<kbn-chrome-config-controls></kbn-chrome-config-controls>
<!-- TODO: These config dropdowns shouldn't be hard coded -->
<config
config-template="appSwitcherTemplate"
config-object="chrome"
config-close="appSwitcherTemplate.close">
</config>

<config
ng-show="timefilter.enabled"
config-template="pickerTemplate"
config-object="timefilter"
config-close="pickerTemplate.close">
</config>

<div class="application" ng-class="'tab-' + chrome.getActiveTabId('-none-') + ' ' + chrome.getApplicationClasses()" ng-view></div>
</div>
7 changes: 0 additions & 7 deletions src/ui/public/chrome/config/filter.html

This file was deleted.

7 changes: 0 additions & 7 deletions src/ui/public/chrome/config/interval.html

This file was deleted.

35 changes: 0 additions & 35 deletions src/ui/public/chrome/context.js

This file was deleted.

48 changes: 35 additions & 13 deletions src/ui/public/chrome/directives/append_nav_controls.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,10 @@
import $ from 'jquery';
import _ from 'lodash';

import chromeNavControlsRegistry from 'ui/registry/chrome_nav_controls';
import chromeConfigControlsRegistry from 'ui/registry/chrome_config_controls';
import UiModules from 'ui/modules';
import spinnerHtml from './active_http_spinner.html';

const spinner = {
name: 'active http requests',
template: spinnerHtml
};

export default function (chrome, internals) {

UiModules
Expand All @@ -17,16 +13,42 @@ export default function (chrome, internals) {
return {
template: function ($element) {
const parts = [$element.html()];
const controls = Private(chromeNavControlsRegistry);
const navs = Private(chromeNavControlsRegistry);
const configs = Private(chromeConfigControlsRegistry);

const controls = [
{
name: 'active http requests',
order: -100,
template: spinnerHtml
},

...navs.map(function (nav) {
return {
template: `<!-- nav control ${nav.name} -->${nav.template}`,
order: nav.order
};
}),

...configs.map(function (config) {
const configHtml = `<render-directive definition="configs['${config.name}'].navbar">
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure how I missed this the first time, but we should probably take a tad-bit more care with how we build configHtml

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you mean?

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, I guess we have to trust the template value, but I would feel better if we changed the injection of config.name to something like:

`<render-directive definition="configs.byName['${JSON.stringify(config.name)}'].navbar">

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The JSON.stringify didn't work since it add double quotes which messed up the html parsing. I instead add a regex piece in the registry that forces the names to only allow a-z, A-Z, or 0-9.

${config.navbar.template}
</render-directive>`;
return {
template: `<!-- nav control ${config.name} -->${configHtml}`,
order: config.order
};
}),
];

for (const control of [spinner, ...controls.inOrder]) {
parts.unshift(
`<!-- nav control ${control.name} -->`,
control.template
);
}
_.sortBy(controls, 'order').forEach(function (control) {
parts.unshift(control.template);
});

return parts.join('\n');
},
controller: function ($scope) {
$scope.configs = Private(chromeConfigControlsRegistry).byName;
}
};
});
Expand Down
8 changes: 8 additions & 0 deletions src/ui/public/chrome/directives/config_controls.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<div ng-repeat="control in controls">
<config
config-template="control.config.template"
config-object="control.config.object"
config-submit="::control.config.submit"
config-close="::control.config.close">
</config>
</div>
22 changes: 22 additions & 0 deletions src/ui/public/chrome/directives/config_controls.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import $ from 'jquery';

import chromeConfigControlsRegistry from 'ui/registry/chrome_config_controls';
import UiModules from 'ui/modules';
import configControlsHtml from './config_controls.html';

export default function (chrome, internals) {

UiModules
.get('kibana')
.directive('kbnChromeConfigControls', function (Private) {
const controls = Private(chromeConfigControlsRegistry);
return {
restrict: 'E',
template: configControlsHtml,
controller: function ($scope) {
$scope.controls = controls.inOrder;
}
};
});

}
2 changes: 2 additions & 0 deletions src/ui/public/chrome/directives/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ import 'ui/directives/config';
import './app_switcher';
import kbnChromeProv from './kbn_chrome';
import kbnChromeNavControlsProv from './append_nav_controls';
import kbnChromeConfigControlsProv from './config_controls';

export default function (chrome, internals) {
kbnChromeProv(chrome, internals);
kbnChromeNavControlsProv(chrome, internals);
kbnChromeConfigControlsProv(chrome, internals);
}
6 changes: 3 additions & 3 deletions src/ui/public/courier/courier.js
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,9 @@ uiModules.get('kibana/courier')
};

// Listen for refreshInterval changes
$rootScope.$watchCollection('timefilter.refreshInterval', function () {
var refreshValue = _.get($rootScope, 'timefilter.refreshInterval.value');
var refreshPause = _.get($rootScope, 'timefilter.refreshInterval.pause');
$rootScope.$watchCollection('$$timefilter.refreshInterval', function () {
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, we can't use the $$timefilter here. The $$ prefix is explicitly designed to prevent it's use. Maybe we can inject the timefilter and use a getter function in the watchCollection

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That gist worked for me so I committed it.

var refreshValue = _.get($rootScope, '$$timefilter.refreshInterval.value');
var refreshPause = _.get($rootScope, '$$timefilter.refreshInterval.pause');
if (_.isNumber(refreshValue) && !refreshPause) {
self.fetchInterval(refreshValue);
} else {
Expand Down
19 changes: 7 additions & 12 deletions src/ui/public/directives/__tests__/timepicker.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,23 +28,18 @@ var init = function () {
clock = sinon.useFakeTimers(moment(anchor).valueOf());

// Create the scope
ngMock.inject(function ($rootScope, $compile) {
ngMock.inject(function ($rootScope, $compile, timefilter) {

// Give us a scope
$parentScope = $rootScope;

// Add some parameters to it
var timefilter = {
time : {
from: moment().subtract(15, 'minutes'),
to: moment(),
mode: undefined
},
refreshInterval : {
value : 0,
display : 'Off'
}
};
timefilter.time.from = moment().subtract(15, 'minutes');
timefilter.time.to = moment();
timefilter.time.mode = undefined;
timefilter.refreshInterval.value = 0;
timefilter.refreshInterval.display = 'Off';

$parentScope.timefilter = timefilter;

// Create the element
Expand Down
8 changes: 8 additions & 0 deletions src/ui/public/registry/chrome_config_controls.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import _ from 'lodash';
define(function (require) {
return require('ui/registry/_registry')({
name: 'chromeConfigControls',
order: ['order'],
index: ['name']
});
});
7 changes: 7 additions & 0 deletions src/ui/public/timepicker/config/filter.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<kbn-timepicker
from="configObject.time.from"
to="configObject.time.to"
mode="configObject.time.mode"
active-tab="'filter'"
interval="configObject.refreshInterval">
</kbn-timepicker>
Loading