Skip to content

Commit

Permalink
address code review comments
Browse files Browse the repository at this point in the history
and implement new angular style suggestions from Joe
  • Loading branch information
stacey-gammon committed Jan 17, 2017
1 parent 6b727df commit 5ca36df
Show file tree
Hide file tree
Showing 13 changed files with 146 additions and 156 deletions.
4 changes: 1 addition & 3 deletions src/ui/public/autoload/modules.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,7 @@ import 'ui/parse_query';
import 'ui/persisted_log';
import 'ui/private';
import 'ui/promises';
import 'ui/dialogues/modal_dialogue';
import 'ui/dialogues/confirm_dialogue';
import 'ui/dialogues/safe_confirm';
import 'ui/dialogues';
import 'ui/state_management/app_state';
import 'ui/state_management/global_state';
import 'ui/storage';
Expand Down
23 changes: 9 additions & 14 deletions src/ui/public/dialogues/__tests__/safe_confirm.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import angular from 'angular';
import expect from 'expect.js';
import ngMock from 'ng_mock';

describe.only('ui/dialogues/safe_confirm', function () {
describe('ui/dialogues/safe_confirm', function () {

let $rootScope;
let $window;
Expand All @@ -25,10 +25,8 @@ describe.only('ui/dialogues/safe_confirm', function () {
});

afterEach(function () {
const confirmationDialogElement = document.querySelector('.confirm-dialogue');
if (confirmationDialogElement) {
const okayButton = confirmationDialogElement.getElementsByTagName('button')[0];

const okayButton = document.body.find('[data-test-subj=confirm-dialog-okay-button]');
if (okayButton) {
$rootScope.$digest();
angular.element(okayButton).click();
}
Expand All @@ -47,11 +45,11 @@ describe.only('ui/dialogues/safe_confirm', function () {
});

context('after timeout completes', function () {
it('confirmation dialog is loaded to dom with message', function () {
it('confirmation dialogue is loaded to dom with message', function () {
$timeout.flush();
const confirmationDialogElement = document.querySelector('.confirm-dialogue');
expect(!!confirmationDialogElement).to.be(true);
const htmlString = confirmationDialogElement.innerHTML;
const confirmationDialogueElement = document.body.find('[data-test-subj=confirm-dialog]');
expect(!!confirmationDialogueElement).to.be(true);
const htmlString = confirmationDialogueElement.innerHTML;

expect(htmlString.indexOf(message)).to.be.greaterThan(0);
});
Expand All @@ -64,9 +62,7 @@ describe.only('ui/dialogues/safe_confirm', function () {
promise.then((v) => {
value = v;
});

const confirmationDialogElement = document.querySelector('.confirm-dialogue');
const okayButton = confirmationDialogElement.getElementsByTagName('button')[0];
const okayButton = document.body.find('[data-test-subj=confirm-dialog-okay-button]');

$rootScope.$digest();
angular.element(okayButton).click();
Expand All @@ -81,8 +77,7 @@ describe.only('ui/dialogues/safe_confirm', function () {

let value;
promise.then(null, (v) => { value = v; });
const confirmationDialogElement = document.querySelector('.confirm-dialogue');
const noButton = confirmationDialogElement.getElementsByTagName('button')[1];
const noButton = document.body.find('[data-test-subj=confirm-dialog-cancel-button]');

$rootScope.$digest();
angular.element(noButton).click();
Expand Down
10 changes: 0 additions & 10 deletions src/ui/public/dialogues/confirm_dialogue.html

This file was deleted.

63 changes: 0 additions & 63 deletions src/ui/public/dialogues/confirm_dialogue.js

This file was deleted.

6 changes: 6 additions & 0 deletions src/ui/public/dialogues/confirm_dialogue_container.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<confirm-dialogue on-confirm="onConfirm()"
on-cancel="onCancel()"
message="{{message}}"
confirm-button-text="{{confirmButtonText}}"
cancel-button-text="{{cancelButtonText}}">
</confirm-dialogue>
15 changes: 15 additions & 0 deletions src/ui/public/dialogues/directives/confirm_dialogue.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<div class="disable-app-layer">
<div class="confirm-dialogue" data-test-subj="confirm-dialog">
<div class="confirm-dialogue__message">
{{message}}
</div>
<div class="confirm-dialogue__actions">
<button class="kuiButton kuiButton--primary"
data-test-subj="confirm-dialog-okay-button"
ng-click="onConfirm()">{{confirmButtonText}}</button>
<button class="kuiButton kuiButton--basic"
data-test-subj="confirm-dialog-cancel-button"
ng-click="onCancel()">{{cancelButtonText}}</button>
</div>
</div>
</div>
18 changes: 18 additions & 0 deletions src/ui/public/dialogues/directives/confirm_dialogue.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import uiModules from 'ui/modules';
import confirmDialogueTemplate from './confirm_dialogue.html';

const module = uiModules.get('kibana');

module.directive('confirmDialogue', function () {
return {
restrict: 'E',
template: confirmDialogueTemplate,
scope: {
message: '@',
confirmButtonText: '@',
cancelButtonText: '@',
onConfirm: '&',
onCancel: '&'
}
};
});
1 change: 1 addition & 0 deletions src/ui/public/dialogues/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import './safe_confirm.factory';
12 changes: 12 additions & 0 deletions src/ui/public/dialogues/modal_dialogue.factory.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import uiModules from 'ui/modules';
import { ModalDialogue } from './modal_dialogue';

const module = uiModules.get('kibana');

module.factory('ModalDialogue', function ($rootScope, $compile) {
return class AngularModalDialogue extends ModalDialogue {
constructor(html, scopeObject) {
super(html, scopeObject, $rootScope, $compile);
}
};
});
46 changes: 22 additions & 24 deletions src/ui/public/dialogues/modal_dialogue.js
Original file line number Diff line number Diff line change
@@ -1,29 +1,27 @@
import uiModules from 'ui/modules';
import angular from 'angular';

const module = uiModules.get('kibana');

module.factory('ModalDialogue', function () {
export class ModalDialogue {
/**
* The only thing this class does is load an element onto the dom when instantiated,
* and removes it when destroy is called. Useful for the modal confirmation dialog.
* Long term, some of the shared modal complexity could be moved in here, hence the name.
*
* @param html
* @param scopeObject
* @param $rootScope
* @param $compile {function}
*/
return class ModalDialogue {
/**
*
* @param innerElement {HTMLElement} - an element that will be appended to the dom.
*/
constructor(innerElement) {
this.innerElement = innerElement;
angular.element(document.body).append(this.innerElement);
}
constructor(html, scopeObject, $rootScope, $compile) {
this.modalScope = $rootScope.$new();

Object.keys(scopeObject).forEach((key) => { this.modalScope[key] = scopeObject[key]; });

this.innerElement = $compile(html)(this.modalScope);
angular.element(document.body).append(this.innerElement);
}

/**
* Removes the element from the dom.
*/
destroy() {
this.innerElement.remove();
}
};
});
/**
* Removes the element from the dom.
*/
destroy() {
this.innerElement.remove();
this.modalScope.$destroy();
}
}
11 changes: 11 additions & 0 deletions src/ui/public/dialogues/safe_confirm.factory.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import uiModules from 'ui/modules';
import { safeConfirm } from './safe_confirm';
import './directives/confirm_dialogue';
import './modal_dialogue.factory';

const module = uiModules.get('kibana');

module.factory('safeConfirm', function ($timeout, ModalDialogue, Promise) {
return (message, confirmButtonText = 'Okay', cancelButtonText = 'Cancel') =>
$timeout(() => safeConfirm(message, confirmButtonText, cancelButtonText, Promise, ModalDialogue));
});
57 changes: 31 additions & 26 deletions src/ui/public/dialogues/safe_confirm.js
Original file line number Diff line number Diff line change
@@ -1,29 +1,34 @@
import uiModules from 'ui/modules';
uiModules.get('kibana')
import confirmDialogueContainerTemplate from './confirm_dialogue_container.html';

/*
* Angular doesn't play well with thread blocking calls such as
* unless those calls are specifically handled inside a call
* to $timeout(). Rather than litter the code with that implementation
* detail, safeConfirm() can be used.
/**
* Shows a modal confirmation dialogue with the given message.
*
* WARNING: safeConfirm only blocks the thread beginning on the next tick. For that reason, a
* promise is returned so consumers can handle the control flow.
*
* Usage:
* safeConfirm('This message will be shown in a modal dialog').then(
* function () {
* // user clicked the okay button
* },
* function () {
* // user canceled the confirmation
* }
* );
* @param {String} message
* @param {String} confirmButtonText - Text to show for the button that will resolve the returned promise
* @param {String} cancelButtonText - Text to show for the button that will reject the returned promise
* @param {Promise} Promise - a promise class.
* @param {ModalDialogue} ModalDialogue service
* @return {Promise<boolean>} Returns an angular promise that will be resolved to true if the user
* clicks the yes/okay button and rejected if the user clicks the no/cancel button.
*/
.factory('safeConfirm', function ($window, $timeout, $q, showConfirmDialogue) {
return function safeConfirm(message) {
return $timeout(function () {
return showConfirmDialogue(message) || $q.reject(false);
});
};
});
export function safeConfirm(message, confirmButtonText, cancelButtonText, Promise, ModalDialogue) {
return new Promise((resolve, reject) => {
let modalDialogue = undefined;

const dialogueScope = {
onConfirm: () => {
modalDialogue.destroy();
resolve(true);
},
onCancel: () => {
modalDialogue.destroy();
reject(false);
},
message,
confirmButtonText,
cancelButtonText
};

modalDialogue = new ModalDialogue(confirmDialogueContainerTemplate, dialogueScope);
});
}
36 changes: 20 additions & 16 deletions src/ui/public/styles/confirm-dialog.less
Original file line number Diff line number Diff line change
@@ -1,30 +1,34 @@
@import "~ui/styles/variables";

.confirm-dialogue {
z-index: 10;
position: absolute;
background-color: white;
left: 40%;
top: 30%;
width: 400px;
padding: 15px 15px 0px 15px;
padding: 15px;
}

.message {
padding: 0px 0px 5px 0px;
}
.confirm-dialogue__message {
padding: 0px 0px 5px 0px;
}

.confirm-dialogue__actions {
display: flex;
justify-content: flex-end;

.form-group {
float: right;
> * + * {
margin-left: 5px;
}
}

.disable-app-layer {
z-index: 9;
position: absolute;
opacity: .5;
background-color: @kibanaGray1;
height: 100%;
width: 100%;
display: flex;
align-items: center;
justify-content: center;
position: fixed;
z-index: 10;
background-color: rgba(0, 0, 0, 0.3);
top: 0;
left: 0;
bottom: 0;
right: 0;
padding-bottom: 10vh;
}

0 comments on commit 5ca36df

Please sign in to comment.