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

Fix interval drop-down for date histogram in discover #10384

Merged
merged 9 commits into from
Mar 28, 2017
12 changes: 2 additions & 10 deletions src/core_plugins/kibana/public/discover/controllers/discover.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,6 @@ function discoverController($scope, config, courier, $route, $window, Notifier,
return interval.val !== 'custom';
};

$scope.toggleInterval = function () {
$scope.showInterval = !$scope.showInterval;
};
$scope.topNavMenu = [{
key: 'new',
description: 'New Search',
Expand Down Expand Up @@ -236,10 +233,7 @@ function discoverController($scope, config, courier, $route, $window, Notifier,
timefilter.enabled = !!timefield;
});

$scope.$watch('state.interval', function (interval, oldInterval) {
if (interval !== oldInterval && interval === 'auto') {
$scope.showInterval = false;
}
$scope.$watch('state.interval', function () {
$scope.fetch();
});

Expand All @@ -250,9 +244,7 @@ function discoverController($scope, config, courier, $route, $window, Notifier,
const buckets = $scope.vis.aggs.bySchemaGroup.buckets;

if (buckets && buckets.length === 1) {
$scope.intervalName = 'by ' + buckets[0].buckets.getInterval().description;
} else {
$scope.intervalName = 'auto';
$scope.bucketInterval = buckets[0].buckets.getInterval();
}
});

Expand Down
19 changes: 11 additions & 8 deletions src/core_plugins/kibana/public/discover/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -90,20 +90,23 @@ <h2>Searching</h2>

&mdash;

<span class="results-interval" ng-hide="showInterval">
<a
ng-click="toggleInterval()">
{{ intervalName }}
</a>
</span>

<span ng-show="showInterval" class="results-interval form-inline">
<span class="results-interval form-inline">
<select
class="form-control"
ng-model="state.interval"
ng-options="interval.val as interval.display for interval in intervalOptions | filter: intervalEnabled"
ng-blur="toggleInterval()"
Copy link
Contributor

Choose a reason for hiding this comment

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

One small suggestion... can we remove this function, and add two to replace it: showInterval and hideInterval? I think this explicitness clarifies the logic behind the behavior.

data-test-subj="discoverIntervalSelect"
>
</select>
<span ng-show="bucketInterval.scaled">
<kbn-info
placement="right"
class="kuiIcon--info"
info="This interval creates {{ bucketInterval.scale > 1 ? 'buckets that are too large' : 'too many buckets' }} to show in the selected time range, so it has been scaled to {{ bucketInterval.description }}">
</kbn-info>
Scaled to {{ bucketInterval.description }}
</span>
</span>
</center>

Expand Down
8 changes: 4 additions & 4 deletions test/functional/apps/discover/_discover.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,10 @@ bdd.describe('discover app', function describeIndexTests() {
expect(actualTimeString).to.be(expectedTimeString);
});

bdd.it('should show correct initial chart interval of 3 hours', async function () {
bdd.it('should show correct initial chart interval of Auto', async function () {
const actualInterval = await PageObjects.discover.getChartInterval();

const expectedInterval = 'by 3 hours';
const expectedInterval = 'Auto';
expect(actualInterval).to.be(expectedInterval);
});

Expand Down Expand Up @@ -155,8 +155,8 @@ bdd.describe('discover app', function describeIndexTests() {
await verifyChartData(expectedBarChartData);
});

bdd.it('should show Auto chart interval of 3 hours', async function () {
const expectedChartInterval = 'by 3 hours';
bdd.it('should show Auto chart interval', async function () {
const expectedChartInterval = 'Auto';

const actualInterval = await PageObjects.discover.getChartInterval();
expect(actualInterval).to.be(expectedChartInterval);
Expand Down
33 changes: 7 additions & 26 deletions test/support/page_objects/discover_page.js
Original file line number Diff line number Diff line change
Expand Up @@ -131,38 +131,19 @@ export default class DiscoverPage {
}

getChartInterval() {
return this.findTimeout
.findByCssSelector('a[ng-click="toggleInterval()"]')
.getVisibleText()
.then(intervalText => {
if (intervalText.length > 0) {
return intervalText;
} else {
return this.findTimeout
.findByCssSelector('select[ng-model="state.interval"]')
.getProperty('value') // this gets 'string:d' for Daily
.then(selectedValue => {
return this.findTimeout
.findByCssSelector('option[value="' + selectedValue + '"]')
.getVisibleText();
});
}
return PageObjects.common.findTestSubject('discoverIntervalSelect')
.getProperty('value')
.then(selectedValue => {
return this.findTimeout
.findByCssSelector('option[value="' + selectedValue + '"]')
.getVisibleText();
});
}

setChartInterval(interval) {
return this.remote.setFindTimeout(5000)
.findByCssSelector('a[ng-click="toggleInterval()"]')
.findByCssSelector('option[label="' + interval + '"]')
.click()
.catch(() => {
// in some cases we have the link above, but after we've made a
// selection we just have a select list.
})
.then(() => {
return this.findTimeout
.findByCssSelector('option[label="' + interval + '"]')
.click();
})
.then(() => {
return PageObjects.header.waitUntilLoadingHasFinished();
});
Expand Down