Skip to content

Commit 1baeae8

Browse files
lukasolsonlukasolson
committed
Close timepicker when a filter/interval is selected (#9618)
* Close timepicker when a filter/interval is selected * Copy absolute variables before sending * Use & instead of = for directive binding * Fix timepicker tests * Fix timepicker tests and remove tests that no longer apply
1 parent 7e350f5 commit 1baeae8

7 files changed

Lines changed: 69 additions & 84 deletions

File tree

‎src/ui/public/chrome/config/filter.html‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,5 +3,7 @@
33
to="timefilter.time.to"
44
mode="timefilter.time.mode"
55
active-tab="'filter'"
6-
interval="timefilter.refreshInterval">
6+
interval="timefilter.refreshInterval"
7+
on-filter-select="updateFilter(from, to)"
8+
on-interval-select="updateInterval(interval)">
79
</kbn-timepicker>

‎src/ui/public/chrome/config/interval.html‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,5 +3,7 @@
33
to="timefilter.time.to"
44
mode="timefilter.time.mode"
55
active-tab="'interval'"
6-
interval="timefilter.refreshInterval">
6+
interval="timefilter.refreshInterval"
7+
on-filter-select="updateFilter(from, to)"
8+
on-interval-select="updateInterval(interval)">
79
</kbn-timepicker>

‎src/ui/public/directives/__tests__/timepicker.js‎

Lines changed: 28 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ let init = function () {
4646
}
4747
};
4848
$parentScope.timefilter = timefilter;
49+
$parentScope.updateInterval = sinon.spy();
50+
$parentScope.updateFilter = sinon.spy();
4951

5052
// Create the element
5153
$elem = angular.element(
@@ -54,7 +56,9 @@ let init = function () {
5456
' to="timefilter.time.to"' +
5557
' mode="timefilter.time.mode"' +
5658
' active-tab="timefilter.timepickerActiveTab"' +
57-
' interval="timefilter.refreshInterval">' +
59+
' interval="timefilter.refreshInterval"' +
60+
' on-interval-select="updateInterval(interval)"' +
61+
' on-filter-select="updateFilter(from, to)">' +
5862
'</kbn-timepicker>'
5963
);
6064

@@ -99,64 +103,34 @@ describe('timepicker directive', function () {
99103
done();
100104
});
101105

102-
it('should have a $scope.setRefreshInterval() that sets interval variable', function (done) {
106+
it('should have a $scope.setRefreshInterval() that calls handler', function (done) {
103107
$scope.setRefreshInterval({ value : 10000 });
104-
expect($scope.interval).to.have.property('value', 10000);
105-
done();
106-
});
107-
108-
it('should set the interval on the courier', function (done) {
109-
// Change refresh interval and digest
110-
$scope.setRefreshInterval({ value : 1000});
111-
$elem.scope().$digest();
112-
expect($courier.searchLooper.loopInterval()).to.be(1000);
113-
done();
114-
});
115-
116-
it('should disable the looper when paused', function (done) {
117-
$scope.setRefreshInterval({ value : 1000, pause: true});
118-
$elem.scope().$digest();
119-
expect($courier.searchLooper.loopInterval()).to.be(0);
120-
expect($scope.interval.value).to.be(1000);
121-
done();
122-
});
123-
124-
it('but keep interval.value set', function (done) {
125-
$scope.setRefreshInterval({ value : 1000, pause: true});
126-
$elem.scope().$digest();
127-
expect($scope.interval.value).to.be(1000);
108+
sinon.assert.calledOnce($parentScope.updateInterval);
109+
expect($parentScope.updateInterval.firstCall.args[0]).to.have.property('value', 10000);
128110
done();
129111
});
130112

131113
it('should unpause when setRefreshInterval is called without pause:true', function (done) {
132-
$scope.setRefreshInterval({ value : 1000, pause: true});
133-
expect($scope.interval.pause).to.be(true);
114+
$scope.setRefreshInterval({ value : 1000, pause: true });
115+
expect($parentScope.updateInterval.getCall(0).args[0]).to.have.property('pause', true);
134116

135-
$scope.setRefreshInterval({ value : 1000, pause: false});
136-
expect($scope.interval.pause).to.be(false);
117+
$scope.setRefreshInterval({ value : 1000, pause: false });
118+
expect($parentScope.updateInterval.getCall(1).args[0]).to.have.property('pause', false);
137119

138-
$scope.setRefreshInterval({ value : 1000});
139-
expect($scope.interval.pause).to.be(false);
120+
$scope.setRefreshInterval({ value : 1000 });
121+
expect($parentScope.updateInterval.getCall(2).args[0]).to.have.property('pause', false);
140122

141123
done();
142124
});
143125

144126

145127
it('should highlight the current active interval', function (done) {
146-
$scope.setRefreshInterval({ value: 300000 });
128+
$scope.interval = { value: 300000 };
147129
$elem.scope().$digest();
148130
expect($elem.find('.refresh-interval-active').length).to.be(1);
149131
expect($elem.find('.refresh-interval-active').text().trim()).to.be('5 minutes');
150132
done();
151133
});
152-
153-
it('should default the interval on the courier with incorrect values', function (done) {
154-
// Change refresh interval and digest
155-
$scope.setRefreshInterval();
156-
$elem.scope().$digest();
157-
expect($courier.searchLooper.loopInterval()).to.be(0);
158-
done();
159-
});
160134
});
161135

162136
describe('mode setting', function () {
@@ -198,10 +172,11 @@ describe('timepicker directive', function () {
198172
done();
199173
});
200174

201-
it('should have a $scope.setQuick() that sets the to and from variables to strings', function (done) {
175+
it('should have a $scope.setQuick() that calls handler', function (done) {
202176
$scope.setQuick('now', 'now');
203-
expect($scope.from).to.be('now');
204-
expect($scope.to).to.be('now');
177+
sinon.assert.calledOnce($parentScope.updateFilter);
178+
expect($parentScope.updateFilter.firstCall.args[0]).to.be('now');
179+
expect($parentScope.updateFilter.firstCall.args[1]).to.be('now');
205180
done();
206181
});
207182
});
@@ -312,24 +287,25 @@ describe('timepicker directive', function () {
312287
$scope.relative.count = 1;
313288
$scope.relative.unit = 's';
314289
$scope.applyRelative();
315-
expect($scope.from).to.be('now-1s');
290+
sinon.assert.calledOnce($parentScope.updateFilter);
291+
expect($parentScope.updateFilter.getCall(0).args[0]).to.be('now-1s');
316292

317293
$scope.relative.count = 2;
318294
$scope.relative.unit = 'm';
319295
$scope.applyRelative();
320-
expect($scope.from).to.be('now-2m');
296+
expect($parentScope.updateFilter.getCall(1).args[0]).to.be('now-2m');
321297

322298
$scope.relative.count = 3;
323299
$scope.relative.unit = 'h';
324300
$scope.applyRelative();
325-
expect($scope.from).to.be('now-3h');
301+
expect($parentScope.updateFilter.getCall(2).args[0]).to.be('now-3h');
326302

327303
// Enable rounding
328304
$scope.relative.round = true;
329305
$scope.relative.count = 7;
330306
$scope.relative.unit = 'd';
331307
$scope.applyRelative();
332-
expect($scope.from).to.be('now-7d/d');
308+
expect($parentScope.updateFilter.getCall(3).args[0]).to.be('now-7d/d');
333309

334310
done();
335311
});
@@ -398,16 +374,6 @@ describe('timepicker directive', function () {
398374
done();
399375
});
400376

401-
it('should parse the time of scope.from and scope.to to set its own variables', function (done) {
402-
$scope.setQuick('now-30m', 'now');
403-
$scope.setMode('absolute');
404-
$scope.$digest();
405-
406-
expect($scope.absolute.from.valueOf()).to.be(moment().subtract(30, 'minutes').valueOf());
407-
expect($scope.absolute.to.valueOf()).to.be(moment().valueOf());
408-
done();
409-
});
410-
411377
it('should update its own variables if timefilter time is updated', function (done) {
412378
$scope.setMode('absolute');
413379
$scope.$digest();
@@ -438,18 +404,17 @@ describe('timepicker directive', function () {
438404
});
439405

440406
it('should only copy its input to scope.from and scope.to when scope.applyAbsolute() is called', function (done) {
441-
$scope.setQuick('now-30m', 'now');
442-
expect($scope.from).to.be('now-30m');
443-
expect($scope.to).to.be('now');
407+
$scope.from = 'now-30m';
408+
$scope.to = 'now';
444409

445410
$scope.absolute.from = moment('2012-02-01');
446411
$scope.absolute.to = moment('2012-02-11');
447412
expect($scope.from).to.be('now-30m');
448413
expect($scope.to).to.be('now');
449414

450415
$scope.applyAbsolute();
451-
expect($scope.from.valueOf()).to.be(moment('2012-02-01').valueOf());
452-
expect($scope.to.valueOf()).to.be(moment('2012-02-11').valueOf());
416+
expect($parentScope.updateFilter.firstCall.args[0]).to.eql(moment('2012-02-01'));
417+
expect($parentScope.updateFilter.firstCall.args[1]).to.eql(moment('2012-02-11'));
453418

454419
$scope.$digest();
455420

‎src/ui/public/timepicker/__tests__/toggle.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,13 @@ import $ from 'jquery';
55

66
describe('kbnGlobalTimepicker', function () {
77
let compile;
8+
89
beforeEach(() => {
910
ngMock.module('kibana');
1011
ngMock.inject(($compile, $rootScope) => {
1112
compile = () => {
1213
const $el = $('<kbn-global-timepicker></kbn-global-timepicker>');
14+
$el.data('$kbnTopNavController', {}); // Mock the kbnTopNav
1315
$rootScope.$apply();
1416
$compile($el)($rootScope);
1517
return $el;

‎src/ui/public/timepicker/kbn_global_timepicker.js‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@ UiModules
1919
return {
2020
template: toggleHtml,
2121
replace: true,
22-
link: ($scope) => {
22+
require: '^kbnTopNav',
23+
link: ($scope, element, attributes, kbnTopNav) => {
2324
listenForUpdates($rootScope);
2425

2526
$rootScope.timefilter = timefilter;
@@ -34,6 +35,17 @@ UiModules
3435
$scope.back = function () {
3536
assign(timefilter.time, timeNavigation.stepBackward(timefilter.getBounds()));
3637
};
38+
39+
$scope.updateFilter = function (from, to) {
40+
timefilter.time.from = from;
41+
timefilter.time.to = to;
42+
kbnTopNav.close('filter');
43+
};
44+
45+
$scope.updateInterval = function (interval) {
46+
timefilter.refreshInterval = interval;
47+
kbnTopNav.close('interval');
48+
};
3749
},
3850
};
3951
});

‎src/ui/public/timepicker/timepicker.js‎

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,9 @@ module.directive('kbnTimepicker', function (quickRanges, timeUnits, refreshInter
2424
to: '=',
2525
mode: '=',
2626
interval: '=',
27-
activeTab: '='
27+
activeTab: '=',
28+
onFilterSelect: '&',
29+
onIntervalSelect: '&'
2830
},
2931
template: html,
3032
controller: function ($scope) {
@@ -52,13 +54,13 @@ module.directive('kbnTimepicker', function (quickRanges, timeUnits, refreshInter
5254
$scope.units = timeUnits;
5355

5456
$scope.relativeOptions = [
55-
{text: 'Seconds ago', value: 's'},
56-
{text: 'Minutes ago', value: 'm'},
57-
{text: 'Hours ago', value: 'h'},
58-
{text: 'Days ago', value: 'd'},
59-
{text: 'Weeks ago', value: 'w'},
60-
{text: 'Months ago', value: 'M'},
61-
{text: 'Years ago', value: 'y'},
57+
{ text: 'Seconds ago', value: 's' },
58+
{ text: 'Minutes ago', value: 'm' },
59+
{ text: 'Hours ago', value: 'h' },
60+
{ text: 'Days ago', value: 'd' },
61+
{ text: 'Weeks ago', value: 'w' },
62+
{ text: 'Months ago', value: 'M' },
63+
{ text: 'Years ago', value: 'y' },
6264
];
6365

6466
$scope.$watch('from', function (date) {
@@ -124,8 +126,7 @@ module.directive('kbnTimepicker', function (quickRanges, timeUnits, refreshInter
124126
};
125127

126128
$scope.setQuick = function (from, to) {
127-
$scope.from = from;
128-
$scope.to = to;
129+
$scope.onFilterSelect({ from, to });
129130
};
130131

131132
$scope.setToNow = function () {
@@ -139,17 +140,21 @@ module.directive('kbnTimepicker', function (quickRanges, timeUnits, refreshInter
139140
};
140141

141142
$scope.applyRelative = function () {
142-
$scope.from = getRelativeString();
143-
$scope.to = 'now';
143+
$scope.onFilterSelect({
144+
from: getRelativeString(),
145+
to: 'now'
146+
});
144147
};
145148

146149
function getRelativeString() {
147150
return 'now-' + $scope.relative.count + $scope.relative.unit + ($scope.relative.round ? '/' + $scope.relative.unit : '');
148151
}
149152

150153
$scope.applyAbsolute = function () {
151-
$scope.from = moment($scope.absolute.from);
152-
$scope.to = moment($scope.absolute.to);
154+
$scope.onFilterSelect({
155+
from: moment($scope.absolute.from),
156+
to: moment($scope.absolute.to)
157+
});
153158
};
154159

155160
$scope.setRefreshInterval = function (interval) {
@@ -159,7 +164,7 @@ module.directive('kbnTimepicker', function (quickRanges, timeUnits, refreshInter
159164

160165
notify.log('after: ' + interval.pause);
161166

162-
$scope.interval = interval;
167+
$scope.onIntervalSelect({ interval });
163168
};
164169

165170
$scope.setMode($scope.mode);

‎test/support/page_objects/header_page.js‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,9 +102,6 @@ export default class HeaderPage {
102102
})
103103
.then(() => {
104104
return this.isGlobalLoadingIndicatorHidden();
105-
})
106-
.then(() => {
107-
return this.clickTimepicker();
108105
});
109106
}
110107

0 commit comments

Comments
 (0)