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

Dashboard: Load a default rather than starting new - Against master - Closing #5820 #7626

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
142 changes: 104 additions & 38 deletions src/core_plugins/kibana/public/dashboard/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import _ from 'lodash';
import $ from 'jquery';
import angular from 'angular';
import chrome from 'ui/chrome';
import 'ui/courier';
Expand Down Expand Up @@ -29,34 +28,75 @@ const app = uiModules.get('app/dashboard', [
]);

uiRoutes
.defaults(/dashboard/, {
requireDefaultIndex: true
})
.when('/dashboard', {
template: indexTemplate,
resolve: {
dash: function (savedDashboards, config) {
return savedDashboards.get();
.defaults(/dashboard/, {
requireDefaultIndex: true
})
.when('/dashboard', {
template: function ($location) {
if ($location.new === true) {
return indexTemplate;
}

return false;
},
resolve: {
dash: function (savedDashboards, config, kbnUrl, Notifier, $location, $route) {
let defaultDashboard = config.get('dashboard:defaultDashboard', '');
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be const since the reference is never mutated.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why bother with the empty string fallback?


let notify = new Notifier({
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be const since the reference is never mutated.

location: 'Dashboard'
});

if ($location.search().new === true) {
return savedDashboards.get();
}

function forceNew() {
$location.search('new', true);
$route.reload();
}

if (defaultDashboard !== '' && typeof defaultDashboard !== 'undefined') {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just if (defaultDashboard)? I suspect we wouldn't want to try to get a null, NaN or false dashboard or anything of that sort.

return savedDashboards.get(defaultDashboard)
.then(function (result) {
let dashboardUrl = savedDashboards.urlFor(result.id).substring(1);
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be const since the reference is never mutated.

kbnUrl.change(dashboardUrl);
})
.catch(function (error) {
notify.error(error);

forceNew();
});
}

forceNew();
}
}
}
})
.when('/dashboard/:id', {
template: indexTemplate,
resolve: {
dash: function (savedDashboards, Notifier, $route, $location, courier) {
return savedDashboards.get($route.current.params.id)
.catch(courier.redirectWhenMissing({
'dashboard' : '/dashboard'
}));
})
.when('/dashboard/:id', {
template: indexTemplate,
resolve: {
dash: function (savedDashboards, Notifier, $route, $location, courier, kbnUrl) {
if ($location.search().new === true) {
kbnUrl.change('/dashboard');
$location.search('new', true);
Copy link
Contributor

Choose a reason for hiding this comment

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

Under what circumstance would a user try to access a dashboard by id while simultaneously wanting a new dashboard?

}

return savedDashboards.get($route.current.params.id)
.catch(courier.redirectWhenMissing({
'dashboard': '/dashboard'
Copy link
Contributor

Choose a reason for hiding this comment

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

No reason for quotes on this property. I'm actually surprised the linter allowed this.

}));
}
}
}
});
});

app.directive('dashboardApp', function (Notifier, courier, AppState, timefilter, kbnUrl) {
return {
controller: function ($scope, $rootScope, $route, $routeParams, $location, Private, getAppState) {
controller: function ($scope, $rootScope, $route, $routeParams, $location, Private, getAppState, config, uiSettings) {

const queryFilter = Private(FilterBarQueryFilterProvider);
const configDefaults = uiSettings.defaults;
const configDefaultDashboard = config.get('dashboard:defaultDashboard', '');
Copy link
Contributor

Choose a reason for hiding this comment

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

Why bother with the empty string fallback?


const notify = new Notifier({
location: 'Dashboard'
Expand Down Expand Up @@ -99,11 +139,12 @@ app.directive('dashboardApp', function (Notifier, courier, AppState, timefilter,
if (!angular.equals(newVal, oldVal)) $state.save();
});
$scope.$watch('state.options.darkTheme', setDarkTheme);
$scope.$watch('opts.isDefaultDashboard', toggleDefaultDashboard);

$scope.topNavMenu = [{
key: 'new',
description: 'New Dashboard',
run: function () { kbnUrl.change('/dashboard', {}); },
run: newDashboard
}, {
key: 'add',
description: 'Add a panel to the dashboard',
Expand Down Expand Up @@ -183,6 +224,33 @@ app.directive('dashboardApp', function (Notifier, courier, AppState, timefilter,
chrome.addApplicationClass(theme);
}

// Returns whether this dashboard is now default
function toggleDefaultDashboard(isChecked) {
if (isChecked) {
setDefaultDashboard(dash.id);
return true;
}

if (dash.id === configDefaultDashboard) {
/* If the default option is turned off and the previous default
dashboard was this dashboard set no default dashboard */
setDefaultDashboard(configDefaults['dashboard:defaultDashboard'].value);
return false;
}

setDefaultDashboard(configDefaultDashboard);
return false;
}

function setDefaultDashboard(id) {
config.set('dashboard:defaultDashboard', id);
}

function newDashboard() {
$location.search('new', true);
$route.reload();
}

// update root source when filters update
$scope.$listen(queryFilter, 'update', function () {
updateQueryOnRootSource();
Expand All @@ -192,10 +260,6 @@ app.directive('dashboardApp', function (Notifier, courier, AppState, timefilter,
// update data when filters fire fetch event
$scope.$listen(queryFilter, 'fetch', $scope.refresh);

$scope.newDashboard = function () {
kbnUrl.change('/dashboard', {});
};

$scope.filterResults = function () {
updateQueryOnRootSource();
$state.save();
Expand All @@ -215,16 +279,16 @@ app.directive('dashboardApp', function (Notifier, courier, AppState, timefilter,
dash.optionsJSON = angular.toJson($state.options);

dash.save()
.then(function (id) {
$scope.kbnTopNav.close('save');
if (id) {
notify.info('Saved Dashboard as "' + dash.title + '"');
if (dash.id !== $routeParams.id) {
kbnUrl.change('/dashboard/{{id}}', {id: dash.id});
.then(function (id) {
$scope.kbnTopNav.close('save');
if (id) {
notify.info('Saved Dashboard as "' + dash.title + '"');
if (dash.id !== $routeParams.id) {
kbnUrl.change('/dashboard/{{id}}', {id: dash.id});
}
}
}
})
.catch(notify.fatal);
})
.catch(notify.fatal);
};

let pendingVis = _.size($state.panels);
Expand All @@ -245,17 +309,19 @@ app.directive('dashboardApp', function (Notifier, courier, AppState, timefilter,
// called by the saved-object-finder when a user clicks a vis
$scope.addVis = function (hit) {
pendingVis++;
$state.panels.push({ id: hit.id, type: 'visualization', panelIndex: getMaxPanelIndex() });
$state.panels.push({id: hit.id, type: 'visualization', panelIndex: getMaxPanelIndex()});
};

$scope.addSearch = function (hit) {
pendingVis++;
$state.panels.push({ id: hit.id, type: 'search', panelIndex: getMaxPanelIndex() });
$state.panels.push({id: hit.id, type: 'search', panelIndex: getMaxPanelIndex()});
};

// Setup configurable values for config directive, after objects are initialized
$scope.opts = {
dashboard: dash,
isDefaultDashboard: configDefaultDashboard === dash.id && dash.id !== '',
isNewDashboard: $location.search().new === true,
ui: $state.options,
save: $scope.save,
addVis: $scope.addVis,
Expand Down
25 changes: 19 additions & 6 deletions src/core_plugins/kibana/public/dashboard/partials/options.html
Original file line number Diff line number Diff line change
@@ -1,10 +1,23 @@
<form role="form" class="options">
<p>
<div class="input-group">
<ul class="input-list">
<li>
<label>
<input type="checkbox" ng-model="opts.ui.darkTheme" ng-checked="opts.ui.darkTheme">
Use dark theme
<input class="hidden" type="checkbox" ng-model="opts.ui.darkTheme" ng-checked="opts.ui.darkTheme">
<i class="fa fa-fw"
ng-class="{ 'fa-square': !opts.ui.darkTheme,
'fa-check-square': opts.ui.darkTheme }"></i>
<span>Use dark theme for this dashboard</span>
</label>
</div>
</p>
</li>
<li>
<label ng-class="{ 'text-muted': opts.isNewDashboard }">
<input class="hidden" type="checkbox" ng-model="opts.isDefaultDashboard" ng-checked="opts.isDefaultDashboard" ng-disabled="opts.isNewDashboard">
<i class="fa fa-fw"
ng-class="{ 'fa-square': !opts.isDefaultDashboard,
'fa-check-square': opts.isDefaultDashboard }"></i>
<span>Set this dashboard as default dashboard to load<br>
<small ng-if="opts.isNewDashboard" class="text-danger">You have to save the dashboard first to be able to set it as default.</small></span>
</label>
</li>
</ul>
</form>
18 changes: 18 additions & 0 deletions src/core_plugins/kibana/public/dashboard/styles/main.less
Original file line number Diff line number Diff line change
Expand Up @@ -156,3 +156,21 @@ dashboard-grid {
.dashboard-panel-picker > .list-group-item {
border-top: 0;
}

.dashboard-container {
.config {
.options {
padding: 10px 0;

ul.input-list {
list-style-type: none;
margin-bottom: 0;

label span {
display: inline-block;
vertical-align: top;
}
}
}
}
}