Remove showClose and onClose options in confirm modal

The reason they were added is no longer necessary (something for view edit mode which didn’t pan out). It was a point of contention: See elastic#10104 and elastic#10069 (review) for background. Since it’s no longer needed anyway, and this is going to get moved to react anyhow, I’m removing the unused functionality.
stacey-gammon · Apr 18, 2017 · 9c05419 · 9c05419
1 parent 753dce8
commit 9c05419
Show file tree

Hide file tree

Showing 3 changed files with 2 additions and 82 deletions.
diff --git a/src/ui/public/modals/__tests__/confirm_modal.js b/src/ui/public/modals/__tests__/confirm_modal.js
@@ -81,72 +81,27 @@ describe('ui/modals/confirm_modal', function () {
     });
   });
 
-  describe('x icon', function () {
-    it('is visible when showClose is true', function () {
-      const confirmModalOptions = {
-        confirmButtonText: 'bye',
-        onConfirm: _.noop,
-        showClose: true,
-        title: 'hi'
-      };
-      confirmModal('hi', confirmModalOptions);
-
-      $rootScope.$digest();
-      const xIcon = findByDataTestSubj('confirmModalCloseButton');
-      expect(xIcon.length).to.be(1);
-    });
-
-    it('is not visible when showClose is false', function () {
-      const confirmModalOptions = {
-        confirmButtonText: 'bye',
-        onConfirm: _.noop,
-        title: 'hi',
-        showClose: false
-      };
-      confirmModal('hi', confirmModalOptions);
-
-      $rootScope.$digest();
-      const xIcon = findByDataTestSubj('confirmModalCloseButton');
-      expect(xIcon.length).to.be(0);
-    });
-  });
-
   describe('callbacks are called:', function () {
     const confirmCallback = sinon.spy();
-    const closeCallback = sinon.spy();
     const cancelCallback = sinon.spy();
 
     const confirmModalOptions = {
       confirmButtonText: 'bye',
       onConfirm: confirmCallback,
       onCancel: cancelCallback,
-      onClose: closeCallback,
-      title: 'hi',
-      showClose: true
+      title: 'hi'
     };
 
     beforeEach(() => {
       confirmCallback.reset();
-      closeCallback.reset();
       cancelCallback.reset();
     });
 
-    it('onClose', function () {
-      confirmModal('hi', confirmModalOptions);
-      $rootScope.$digest();
-      findByDataTestSubj('confirmModalCloseButton').click();
-
-      expect(closeCallback.called).to.be(true);
-      expect(confirmCallback.called).to.be(false);
-      expect(cancelCallback.called).to.be(false);
-    });
-
     it('onCancel', function () {
       confirmModal('hi', confirmModalOptions);
       $rootScope.$digest();
       findByDataTestSubj('confirmModalCancelButton').click();
 
-      expect(closeCallback.called).to.be(false);
       expect(confirmCallback.called).to.be(false);
       expect(cancelCallback.called).to.be(true);
     });
@@ -156,29 +111,10 @@ describe('ui/modals/confirm_modal', function () {
       $rootScope.$digest();
       findByDataTestSubj('confirmModalConfirmButton').click();
 
-      expect(closeCallback.called).to.be(false);
       expect(confirmCallback.called).to.be(true);
       expect(cancelCallback.called).to.be(false);
     });
 
-
-    it('onClose defaults to onCancel if not specified', function () {
-      const confirmModalOptions = {
-        confirmButtonText: 'bye',
-        onConfirm: confirmCallback,
-        onCancel: cancelCallback,
-        title: 'hi',
-        showClose: true
-      };
-
-      confirmModal('hi', confirmModalOptions);
-      $rootScope.$digest();
-      findByDataTestSubj('confirmModalCloseButton').click();
-
-      expect(confirmCallback.called).to.be(false);
-      expect(cancelCallback.called).to.be(true);
-    });
-
     it('onKeyDown detects ESC and calls onCancel', function () {
       const confirmModalOptions = {
         confirmButtonText: 'bye',

diff --git a/src/ui/public/modals/confirm_modal.html b/src/ui/public/modals/confirm_modal.html
@@ -3,12 +3,6 @@
     <div class="kuiModalHeader__title" data-test-subj="confirmModalTitleText">
       {{title}}
     </div>
-    <div
-      ng-if="showClose"
-      class="kuiModalHeaderCloseButton kuiIcon fa-times"
-      data-test-subj="confirmModalCloseButton"
-      ng-click="onClose()"
-    ></div>
   </div>
   <div class="kuiModalBody">
     <div

diff --git a/src/ui/public/modals/confirm_modal.js b/src/ui/public/modals/confirm_modal.js
@@ -17,11 +17,7 @@ export const ConfirmationButtonTypes = {
  * @property {String=} cancelButtonText
  * @property {function} onConfirm
  * @property {function=} onCancel
- * @property {String=} title - If given, shows a title on the confirm modal. A title must be given if
- * showClose is true, for aesthetic reasons.
- * @property {Boolean=} showClose - If true, shows an [x] icon close button which by default is a noop
- * @property {function=} onClose - Custom close button to call if showClose is true. If not supplied
- * but showClose is true, the function defaults to onCancel.
+ * @property {String=} title - If given, shows a title on the confirm modal.
  */
 
 module.factory('confirmModal', function ($rootScope, $compile) {
@@ -36,14 +32,9 @@ module.factory('confirmModal', function ($rootScope, $compile) {
     const defaultOptions = {
       onCancel: noop,
       cancelButtonText: 'Cancel',
-      showClose: false,
       defaultFocusedButton: ConfirmationButtonTypes.CONFIRM
     };
 
-    if (customOptions.showClose === true && !customOptions.title) {
-      throw new Error('A title must be supplied when a close icon is shown');
-    }
-
     if (!customOptions.confirmButtonText || !customOptions.onConfirm) {
       throw new Error('Please specify confirmation button text and onConfirm action');
     }
@@ -60,7 +51,6 @@ module.factory('confirmModal', function ($rootScope, $compile) {
     confirmScope.confirmButtonText = options.confirmButtonText;
     confirmScope.cancelButtonText = options.cancelButtonText;
     confirmScope.title = options.title;
-    confirmScope.showClose = options.showClose;
     confirmScope.onConfirm = () => {
       destroy();
       options.onConfirm();