[5.x] Use better confirm button text. rename safe confirm. (#9984) (#10048)

* Use better confirm button text. rename safe confirm. (#9984)

* Use better confirm button text. rename safe confirm.

Use confirm_modal_promise only when it’s easier than using
confirm_modal directly.

change all native confirmations to the new dialog

* address code review comments

* Be consistent with the confirm button text pattern

Address other code review comments

* remove default option for confirmButtonText

Require onConfirm and confirmButtonText for confirmModal

* fix tests for no longer using alertText

* address code review comments Round 2

* fix bad merge

* fix bad merge take 2
This commit is contained in:
Stacey Gammon 2017-01-25 11:17:34 -05:00 committed by GitHub
parent 7aca11968e
commit 930cf02f30
23 changed files with 206 additions and 106 deletions

View file

@ -5,7 +5,7 @@
<kbn-management-index-header
index-pattern="indexPattern"
set-default="setDefaultPattern()"
refresh-fields="indexPattern.refreshFields()"
refresh-fields="refreshFields()"
delete="removePattern()">
</kbn-management-index-header>

View file

@ -40,7 +40,8 @@ uiRoutes
});
uiModules.get('apps/management')
.controller('managementIndicesEdit', function ($scope, $location, $route, config, courier, Notifier, Private, AppState, docTitle) {
.controller('managementIndicesEdit', function (
$scope, $location, $route, config, courier, Notifier, Private, AppState, docTitle, confirmModal) {
const notify = new Notifier();
const $state = $scope.state = new AppState();
@ -70,23 +71,38 @@ uiModules.get('apps/management')
});
$scope.refreshFields = function () {
$scope.indexPattern.refreshFields();
const confirmModalOptions = {
confirmButtonText: 'Refresh fields',
onConfirm: () => { $scope.indexPattern.refreshFields(); }
};
confirmModal(
'This will reset the field popularity counters. Are you sure you want to refresh your fields?',
confirmModalOptions
);
};
$scope.removePattern = function () {
if ($scope.indexPattern.id === config.get('defaultIndex')) {
config.remove('defaultIndex');
if (otherIds.length) {
config.set('defaultIndex', otherIds[0]);
function doRemove() {
if ($scope.indexPattern.id === config.get('defaultIndex')) {
config.remove('defaultIndex');
if (otherIds.length) {
config.set('defaultIndex', otherIds[0]);
}
}
courier.indexPatterns.delete($scope.indexPattern)
.then(refreshKibanaIndex)
.then(function () {
$location.url('/management/kibana/index');
})
.catch(notify.fatal);
}
courier.indexPatterns.delete($scope.indexPattern)
.then(refreshKibanaIndex)
.then(function () {
$location.url('/management/kibana/index');
})
.catch(notify.fatal);
const confirmModalOptions = {
confirmButtonText: 'Remove index pattern',
onConfirm: doRemove
};
confirmModal('Are you sure you want to remove this index pattern?', confirmModalOptions);
};
$scope.setDefaultPattern = function () {

View file

@ -6,8 +6,7 @@
<button
ng-if="field.scripted"
confirm-click="remove(field)"
confirmation="Are you sure want to delete '{{field.name}}'? This action is irreversible!"
ng-click="remove(field)"
class="btn btn-xs btn-danger"
aria-label="Delete">
<i aria-hidden="true" class="fa fa-trash"></i>

View file

@ -14,8 +14,7 @@
</button>
<button
ng-if="refreshFields"
confirm-click="refreshFields()"
confirmation="This will reset the field popularity counters. Are you sure you want to reload your fields?"
ng-click="refreshFields()"
tooltip="Refresh field list"
class="btn btn-warning">
<span class="sr-only">Reload field list</span>
@ -23,9 +22,8 @@
</button>
<button
ng-if="delete"
confirm-click="delete()"
ng-click="delete()"
aria-label="Remove index pattern"
confirmation="Are you sure you want to remove this index pattern?"
tooltip="Remove index pattern"
class="btn btn-danger">
<span class="sr-only">Remove index pattern</span>

View file

@ -9,7 +9,7 @@ import { getSupportedScriptingLangs } from 'ui/scripting_langs';
import { scriptedFields as docLinks } from 'ui/documentation_links/documentation_links';
uiModules.get('apps/management')
.directive('scriptedFields', function (kbnUrl, Notifier, $filter) {
.directive('scriptedFields', function (kbnUrl, Notifier, $filter, confirmModal) {
const rowScopes = []; // track row scopes, so they can be destroyed as needed
const filter = $filter('filter');
@ -100,7 +100,11 @@ uiModules.get('apps/management')
};
$scope.remove = function (field) {
$scope.indexPattern.removeScriptedField(field.name);
const confirmModalOptions = {
confirmButtonText: 'Delete field',
onConfirm: () => { $scope.indexPattern.removeScriptedField(field.name); }
};
confirmModal(`Are you sure want to delete ${field.name}? This action is irreversible!`, confirmModalOptions);
};
$scope.getDeprecatedLanguagesInUse = function () {

View file

@ -12,7 +12,7 @@ import './source_filters.less';
const notify = new Notifier();
uiModules.get('kibana')
.directive('sourceFilters', function (Private, $filter) {
.directive('sourceFilters', function (Private, $filter, confirmModal) {
const angularFilter = $filter('filter');
const { fieldWildcardMatcher } = Private(FieldWildcardProvider);
const rowScopes = []; // track row scopes, so they can be destroyed as needed
@ -92,12 +92,20 @@ uiModules.get('kibana')
}
delete(filter) {
if (this.editing === filter) {
this.editing = null;
}
const doDelete = () => {
if (this.editing === filter) {
this.editing = null;
}
this.$scope.indexPattern.sourceFilters = without(this.all(), filter);
return this.save();
this.$scope.indexPattern.sourceFilters = without(this.all(), filter);
return this.save();
};
const confirmModalOptions = {
confirmButtonText: 'Delete filter',
onConfirm: doDelete
};
confirmModal(`Are you sure want to delete this filter?`, confirmModalOptions);
}
create() {

View file

@ -80,8 +80,7 @@
<!-- Bulk delete button -->
<button
class="kuiButton kuiButton--danger kuiButton--iconText"
confirm-click="bulkDelete()"
confirmation="Are you sure you want to delete the selected {{currentTab.title}}? This action is irreversible!"
ng-click="bulkDelete()"
aria-label="Delete selected objects"
ng-disabled="selectedItems.length == 0"
>

View file

@ -15,7 +15,7 @@ uiRoutes
});
uiModules.get('apps/management')
.directive('kbnManagementObjects', function (kbnIndex, Notifier, Private, kbnUrl, Promise) {
.directive('kbnManagementObjects', function (kbnIndex, Notifier, Private, kbnUrl, Promise, confirmModal) {
return {
restrict: 'E',
controllerAs: 'managementObjectsController',
@ -100,12 +100,23 @@ uiModules.get('apps/management')
// TODO: Migrate all scope methods to the controller.
$scope.bulkDelete = function () {
$scope.currentTab.service.delete(pluck($scope.selectedItems, 'id'))
.then(refreshData)
.then(function () {
$scope.selectedItems.length = 0;
})
.catch(error => notify.error(error));
function doBulkDelete() {
$scope.currentTab.service.delete(pluck($scope.selectedItems, 'id'))
.then(refreshData)
.then(function () {
$scope.selectedItems.length = 0;
})
.catch(error => notify.error(error));
}
const confirmModalOptions = {
confirmButtonText: `Delete ${$scope.currentTab.title}`,
onConfirm: doBulkDelete
};
confirmModal(
`Are you sure you want to delete the selected ${$scope.currentTab.title}? This action is irreversible!`,
confirmModalOptions
);
};
// TODO: Migrate all scope methods to the controller.

View file

@ -19,7 +19,7 @@
<button
class="kuiButton kuiButton--danger kuiButton--iconText"
confirm-click="delete()"
ng-click="delete()"
>
<span class="kuiButton__icon kuiIcon fa-trash-o"></span>
Delete {{ title }}

View file

@ -13,7 +13,7 @@ uiRoutes
});
uiModules.get('apps/management')
.directive('kbnManagementObjectsView', function (kbnIndex, Notifier) {
.directive('kbnManagementObjectsView', function (kbnIndex, Notifier, confirmModal) {
return {
restrict: 'E',
controller: function ($scope, $injector, $routeParams, $location, $window, $rootScope, esAdmin, Private) {
@ -163,15 +163,25 @@ uiModules.get('apps/management')
* @returns {type} description
*/
$scope.delete = function () {
esAdmin.delete({
index: kbnIndex,
type: service.type,
id: $routeParams.id
})
.then(function (resp) {
return redirectHandler('deleted');
})
.catch(notify.fatal);
function doDelete() {
esAdmin.delete({
index: kbnIndex,
type: service.type,
id: $routeParams.id
})
.then(function (resp) {
return redirectHandler('deleted');
})
.catch(notify.fatal);
}
const confirmModalOptions = {
onConfirm: doDelete,
confirmButtonText: 'Delete object'
};
confirmModal(
'Are you sure want to delete this object? This action is irreversible!',
confirmModalOptions
);
};
$scope.submit = function () {

View file

@ -47,7 +47,7 @@ require('ui/routes')
app.controller('timelion', function (
$scope, $http, timefilter, AppState, courier, $route, $routeParams,
kbnUrl, Notifier, config, $timeout, Private, savedVisualizations, safeConfirm) {
kbnUrl, Notifier, config, $timeout, Private, savedVisualizations, confirmModal) {
// TODO: For some reason the Kibana core doesn't correctly do this for all apps.
moment.tz.setDefault(config.get('dateFormat:tz'));
@ -85,13 +85,20 @@ app.controller('timelion', function (
return !savedSheet.id;
},
run: function () {
var title = savedSheet.title;
safeConfirm('Are you sure you want to delete the sheet ' + title + ' ?').then(function () {
const title = savedSheet.title;
function doDelete() {
savedSheet.delete().then(() => {
notify.info('Deleted ' + title);
kbnUrl.change('/');
}).catch(notify.fatal);
});},
}
const confirmModalOptions = {
onConfirm: doDelete,
confirmButtonText: 'Delete sheet'
};
confirmModal(`Are you sure you want to delete the sheet ${title}?`, confirmModalOptions);
},
testId: 'timelionDeleteButton',
}, {
key: 'open',

View file

@ -2,6 +2,7 @@
* Tests functionality in ui/public/courier/saved_object/saved_object.js
*/
import angular from 'angular';
import ngMock from 'ng_mock';
import expect from 'expect.js';
import sinon from 'auto-release-sinon';
@ -88,14 +89,14 @@ describe('Saved Object', function () {
}
beforeEach(ngMock.module('kibana',
// The default implementation of safeConfirm uses $timeout which will cause
// the test environment to hang.
// Use the native window.confirm instead of our specialized version to make testing
// this easier.
function ($provide) {
const overrideSafeConfirm = message => window.confirm(message) ? Promise.resolve() : Promise.reject();
$provide.decorator('safeConfirm', () => overrideSafeConfirm);
const overrideConfirm = message => window.confirm(message) ? Promise.resolve() : Promise.reject();
$provide.decorator('confirmModalPromise', () => overrideConfirm);
})
);
beforeEach(ngMock.inject(function (es, esAdmin, Private, $window) {
SavedObject = Private(SavedObjectFactory);
IndexPattern = Private(IndexPatternFactory);

View file

@ -19,7 +19,7 @@ import MappingSetupProvider from 'ui/utils/mapping_setup';
import DocSourceProvider from '../data_source/admin_doc_source';
import SearchSourceProvider from '../data_source/search_source';
export default function SavedObjectFactory(esAdmin, kbnIndex, Promise, Private, Notifier, safeConfirm, indexPatterns) {
export default function SavedObjectFactory(esAdmin, kbnIndex, Promise, Private, Notifier, confirmModalPromise, indexPatterns) {
let DocSource = Private(DocSourceProvider);
let SearchSource = Private(SearchSourceProvider);
@ -282,7 +282,7 @@ export default function SavedObjectFactory(esAdmin, kbnIndex, Promise, Private,
if (_.get(err, 'origError.status') === 409) {
const confirmMessage = `Are you sure you want to overwrite ${this.title}?`;
return safeConfirm(confirmMessage)
return confirmModalPromise(confirmMessage, { confirmButtonText: `Overwrite ${this.getDisplayName()}` })
.then(() => docSource.doIndex(source))
.catch(() => Promise.reject(new Error(OVERWRITE_REJECTED)));
}

View file

@ -228,8 +228,7 @@
<button
type="button"
ng-if="editor.field.scripted && !editor.creating"
confirm-click="editor.delete()"
confirmation="Are you sure want to delete '{{ editor.field.name }}'? This action is irreversible!"
ng-click="editor.delete()"
aria-label="Delete"
class="btn btn-danger">
Delete Field

View file

@ -14,10 +14,10 @@ import { GetEnabledScriptingLangsProvider, getSupportedScriptingLangs } from '..
uiModules
.get('kibana', ['colorpicker.module'])
.directive('fieldEditor', function (Private, $sce) {
let fieldFormats = Private(RegistryFieldFormatsProvider);
let Field = Private(IndexPatternsFieldProvider);
let getEnabledScriptingLangs = Private(GetEnabledScriptingLangsProvider);
.directive('fieldEditor', function (Private, $sce, confirmModal) {
const fieldFormats = Private(RegistryFieldFormatsProvider);
const Field = Private(IndexPatternsFieldProvider);
const getEnabledScriptingLangs = Private(GetEnabledScriptingLangsProvider);
const fieldTypesByLang = {
painless: ['number', 'string', 'date', 'boolean'],
@ -78,15 +78,25 @@ uiModules
};
self.delete = function () {
let indexPattern = self.indexPattern;
let field = self.field;
function doDelete() {
const indexPattern = self.indexPattern;
const field = self.field;
indexPattern.fields.remove({ name: field.name });
return indexPattern.save()
.then(function () {
notify.info('Deleted Field "' + field.name + '"');
redirectAway();
});
indexPattern.fields.remove({ name: field.name });
return indexPattern.save()
.then(function () {
notify.info('Deleted Field "' + field.name + '"');
redirectAway();
});
}
const confirmModalOptions = {
confirmButtonText: 'Delete field',
onConfirm: doDelete
};
confirmModal(
`Are you sure want to delete '${self.field.name}'? This action is irreversible!`,
confirmModalOptions
);
};
self.isSupportedLang = function (lang) {

View file

@ -14,7 +14,7 @@ import IndexPatternsFlattenHitProvider from 'ui/index_patterns/_flatten_hit';
import IndexPatternsCalculateIndicesProvider from 'ui/index_patterns/_calculate_indices';
import IndexPatternsPatternCacheProvider from 'ui/index_patterns/_pattern_cache';
export default function IndexPatternFactory(Private, Notifier, config, kbnIndex, Promise, safeConfirm) {
export default function IndexPatternFactory(Private, Notifier, config, kbnIndex, Promise, confirmModalPromise) {
const fieldformats = Private(RegistryFieldFormatsProvider);
const getIds = Private(IndexPatternsGetIdsProvider);
const mapper = Private(IndexPatternsMapperProvider);
@ -337,7 +337,7 @@ export default function IndexPatternFactory(Private, Notifier, config, kbnIndex,
}
const confirmMessage = 'Are you sure you want to overwrite this?';
return safeConfirm(confirmMessage)
return confirmModalPromise(confirmMessage, { confirmButtonText: 'Overwrite' })
.then(() => Promise
.try(() => {
const cached = patternCache.get(this.id);

View file

@ -3,23 +3,23 @@ import expect from 'expect.js';
import ngMock from 'ng_mock';
import sinon from 'sinon';
describe('ui/modals/safe_confirm', function () {
describe('ui/modals/confirm_modal_promise', function () {
let $rootScope;
let message;
let safeConfirm;
let confirmModalPromise;
let promise;
beforeEach(function () {
ngMock.module('kibana');
ngMock.inject(function ($injector) {
safeConfirm = $injector.get('safeConfirm');
confirmModalPromise = $injector.get('confirmModalPromise');
$rootScope = $injector.get('$rootScope');
});
message = 'woah';
promise = safeConfirm(message);
promise = confirmModalPromise(message, { confirmButtonText: 'click me' });
});
afterEach(function () {
@ -81,5 +81,16 @@ describe('ui/modals/safe_confirm', function () {
expect(confirmCallback.called).to.be(false);
});
});
context('error is thrown', function () {
it('when no confirm button text is used', function () {
try {
confirmModalPromise(message);
expect(false).to.be(true);
} catch (error) {
expect(error).to.not.be(undefined);
}
});
});
});
});

View file

@ -1,6 +1,9 @@
<div class="kuiModal" style="width: 450px" data-test-subj="confirmModal">
<div class="kuiModalBody">
<div class="kuiModalBodyText">
<div
class="kuiModalBodyText"
data-test-subj="confirmModalBodyText"
>
{{message}}
</div>
</div>

View file

@ -5,17 +5,31 @@ import { ModalOverlay } from './modal_overlay';
const module = uiModules.get('kibana');
/**
* @typedef {Object} ConfirmModalOptions
* @property {String} confirmButtonText
* @property {String=} cancelButtonText
* @property {function} onConfirm
* @property {function=} onCancel
*/
module.factory('confirmModal', function ($rootScope, $compile) {
let modalPopover;
return function confirmModal(message, customOptions = {}) {
/**
* @param {String} message - the message to show in the body of the confirmation dialog.
* @param {ConfirmModalOptions} - Options to further customize the dialog.
*/
return function confirmModal(message, customOptions) {
const defaultOptions = {
onConfirm: noop,
onCancel: noop,
confirmButtonText: 'Confirm',
cancelButtonText: 'Cancel'
};
if (!customOptions.confirmButtonText || !customOptions.onConfirm) {
throw new Error('Please specify confirmation button text and onConfirm action');
}
const options = Object.assign(defaultOptions, customOptions);
if (modalPopover) {
throw new Error('You\'ve called confirmModal but there\'s already a modal open. ' +

View file

@ -0,0 +1,29 @@
import uiModules from 'ui/modules';
import 'ui/modals';
const module = uiModules.get('kibana');
/**
* @typedef {Object} PromisifiedConfirmOptions
* @property {String} confirmButtonText
* @property {String=} cancelButtonText
*/
/**
* A "promisified" version of ConfirmModal that binds onCancel and onConfirm to
* Resolve and Reject methods.
*/
module.factory('confirmModalPromise', function (Promise, confirmModal) {
/**
* @param {String} message
* @param {PromisifiedConfirmOptions} customOptions
*/
return (message, customOptions) => new Promise((resolve, reject) => {
const defaultOptions = {
onConfirm: resolve,
onCancel: reject
};
const confirmOptions = Object.assign(defaultOptions, customOptions);
confirmModal(message, confirmOptions);
});
});

View file

@ -1,3 +1,2 @@
import './confirm_modal';
import './safe_confirm';
import './confirm_modal_promise';

View file

@ -1,16 +0,0 @@
import uiModules from 'ui/modules';
import 'ui/modals';
const module = uiModules.get('kibana');
module.factory('safeConfirm', function ($timeout, Promise, confirmModal) {
return (message) => new Promise((resolve, reject) => {
const confirmOptions = {
onConfirm: resolve,
onCancel: reject,
confirmButtonText: 'Okay',
cancelButtonText: 'Cancel'
};
confirmModal(message, confirmOptions);
});
});

View file

@ -286,12 +286,10 @@ export default class SettingsPage {
await this.clickDeletePattern();
});
await PageObjects.common.try(async () => {
PageObjects.common.debug('getAlertText');
alertText = await this.remote.getAlertText();
});
await PageObjects.common.try(async () => {
PageObjects.common.debug('acceptAlert');
await this.remote.acceptAlert();
PageObjects.common.debug('acceptConfirmation');
alertText = await PageObjects.common.findTestSubject('confirmModalBodyText').getVisibleText();
await PageObjects.common.findTestSubject('confirmModalConfirmButton')
.click();
});
await PageObjects.common.try(async () => {
const currentUrl = await this.remote.getCurrentUrl();