From 43330326637e9beaa2c28b60c1690bef5bc7939f Mon Sep 17 00:00:00 2001 From: Doug Penny Date: Sun, 19 Feb 2017 14:00:51 -0800 Subject: [PATCH 1/4] #10099 adds keyboard handling to close confirm_modal on ESC --- .../public/modals/__tests__/confirm_modal.js | 43 +++++++++++++++++++ src/ui/public/modals/confirm_modal.html | 1 - src/ui/public/modals/confirm_modal.js | 2 +- src/ui/public/modals/modal_overlay.html | 2 +- src/ui/public/modals/modal_overlay.js | 11 ++++- 5 files changed, 55 insertions(+), 4 deletions(-) diff --git a/src/ui/public/modals/__tests__/confirm_modal.js b/src/ui/public/modals/__tests__/confirm_modal.js index 3332028848e69..eb6a24b78477b 100644 --- a/src/ui/public/modals/__tests__/confirm_modal.js +++ b/src/ui/public/modals/__tests__/confirm_modal.js @@ -182,5 +182,48 @@ describe('ui/modals/confirm_modal', function () { expect(confirmCallback.called).to.be(false); expect(cancelCallback.called).to.be(true); }); + + it('onKeyDown detects ESC and calls onCancel', function () { + resetSpyCounts(); + const confirmModalOptions = { + confirmButtonText: 'bye', + onConfirm: confirmCallback, + onCancel: cancelCallback, + onClose: closeCallback, + title: 'hi', + showClose: false + }; + + confirmModal('hi', confirmModalOptions); + + const e = angular.element.Event('keydown'); // eslint-disable-line new-cap + e.keyCode = 27; + angular.element(document.body).trigger(e); + + expect(cancelCallback.called).to.be(true); + }); + + it('onKeyDown ignores keys other than ESC', function () { + resetSpyCounts(); + const confirmModalOptions = { + confirmButtonText: 'bye', + onConfirm: confirmCallback, + onCancel: cancelCallback, + onClose: closeCallback, + title: 'hi', + showClose: false + }; + + confirmModal('hi', confirmModalOptions); + + const e = angular.element.Event('keydown'); // eslint-disable-line new-cap + e.keyCode = 27; + while(e.keyCode === 27) { + e.keyCode = Math.floor((Math.random() * 100) + 1); + } + angular.element(document.body).trigger(e); + + expect(cancelCallback.called).to.be(false); + }); }); }); diff --git a/src/ui/public/modals/confirm_modal.html b/src/ui/public/modals/confirm_modal.html index ad1b05dcd5b11..34a4eaae99a05 100644 --- a/src/ui/public/modals/confirm_modal.html +++ b/src/ui/public/modals/confirm_modal.html @@ -3,7 +3,6 @@
{{title}}
-
+
diff --git a/src/ui/public/modals/modal_overlay.js b/src/ui/public/modals/modal_overlay.js index f7f4183f713f7..733c1452887ee 100644 --- a/src/ui/public/modals/modal_overlay.js +++ b/src/ui/public/modals/modal_overlay.js @@ -5,9 +5,17 @@ import modalOverlayTemplate from './modal_overlay.html'; * Appends the modal to the dom on instantiation, and removes it when destroy is called. */ export class ModalOverlay { - constructor(modalElement) { + constructor(modalElement, scope) { this.overlayElement = angular.element(modalOverlayTemplate); this.overlayElement.append(modalElement); + + const onKeyDown = (event) => { + if(event.keyCode === 27) { + scope.onCancel(); + } + }; + + angular.element(document.body).bind('keydown', onKeyDown); angular.element(document.body).append(this.overlayElement); } @@ -15,6 +23,7 @@ export class ModalOverlay { * Removes the overlay and modal from the dom. */ destroy() { + angular.element(document.body).off('keydown'); this.overlayElement.remove(); } } From dccafec61c10ad12ddeebf04ba9720f2b8523f77 Mon Sep 17 00:00:00 2001 From: Joe Fleming Date: Mon, 27 Feb 2017 14:54:23 -0700 Subject: [PATCH 2/4] remove resetSpyCounts, use beforeEach --- src/ui/public/modals/__tests__/confirm_modal.js | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/ui/public/modals/__tests__/confirm_modal.js b/src/ui/public/modals/__tests__/confirm_modal.js index eb6a24b78477b..b97010f64b405 100644 --- a/src/ui/public/modals/__tests__/confirm_modal.js +++ b/src/ui/public/modals/__tests__/confirm_modal.js @@ -125,14 +125,13 @@ describe('ui/modals/confirm_modal', function () { showClose: true }; - function resetSpyCounts() { + beforeEach(() => { confirmCallback.reset(); closeCallback.reset(); cancelCallback.reset(); - } + }); it('onClose', function () { - resetSpyCounts(); confirmModal('hi', confirmModalOptions); $rootScope.$digest(); findByDataTestSubj('confirmModalCloseButton').click(); @@ -143,7 +142,6 @@ describe('ui/modals/confirm_modal', function () { }); it('onCancel', function () { - resetSpyCounts(); confirmModal('hi', confirmModalOptions); $rootScope.$digest(); findByDataTestSubj('confirmModalCancelButton').click(); @@ -154,7 +152,6 @@ describe('ui/modals/confirm_modal', function () { }); it('onConfirm', function () { - resetSpyCounts(); confirmModal('hi', confirmModalOptions); $rootScope.$digest(); findByDataTestSubj('confirmModalConfirmButton').click(); @@ -166,7 +163,6 @@ describe('ui/modals/confirm_modal', function () { it('onClose defaults to onCancel if not specified', function () { - resetSpyCounts(); const confirmModalOptions = { confirmButtonText: 'bye', onConfirm: confirmCallback, @@ -184,7 +180,6 @@ describe('ui/modals/confirm_modal', function () { }); it('onKeyDown detects ESC and calls onCancel', function () { - resetSpyCounts(); const confirmModalOptions = { confirmButtonText: 'bye', onConfirm: confirmCallback, @@ -204,7 +199,6 @@ describe('ui/modals/confirm_modal', function () { }); it('onKeyDown ignores keys other than ESC', function () { - resetSpyCounts(); const confirmModalOptions = { confirmButtonText: 'bye', onConfirm: confirmCallback, From 6761493e80d7a96106acd38b72080d3f5ca7f1cc Mon Sep 17 00:00:00 2001 From: Doug Penny Date: Mon, 27 Feb 2017 21:01:27 -0800 Subject: [PATCH 3/4] #10099 change from .bind to .on and remove unnecessary onKeyDown call --- src/ui/public/modals/__tests__/confirm_modal.js | 13 +++---------- src/ui/public/modals/modal_overlay.html | 2 +- src/ui/public/modals/modal_overlay.js | 6 ++---- 3 files changed, 6 insertions(+), 15 deletions(-) diff --git a/src/ui/public/modals/__tests__/confirm_modal.js b/src/ui/public/modals/__tests__/confirm_modal.js index b97010f64b405..7cc8904f157c5 100644 --- a/src/ui/public/modals/__tests__/confirm_modal.js +++ b/src/ui/public/modals/__tests__/confirm_modal.js @@ -184,9 +184,7 @@ describe('ui/modals/confirm_modal', function () { confirmButtonText: 'bye', onConfirm: confirmCallback, onCancel: cancelCallback, - onClose: closeCallback, - title: 'hi', - showClose: false + title: 'hi' }; confirmModal('hi', confirmModalOptions); @@ -203,18 +201,13 @@ describe('ui/modals/confirm_modal', function () { confirmButtonText: 'bye', onConfirm: confirmCallback, onCancel: cancelCallback, - onClose: closeCallback, - title: 'hi', - showClose: false + title: 'hi' }; confirmModal('hi', confirmModalOptions); const e = angular.element.Event('keydown'); // eslint-disable-line new-cap - e.keyCode = 27; - while(e.keyCode === 27) { - e.keyCode = Math.floor((Math.random() * 100) + 1); - } + e.keyCode = 50; angular.element(document.body).trigger(e); expect(cancelCallback.called).to.be(false); diff --git a/src/ui/public/modals/modal_overlay.html b/src/ui/public/modals/modal_overlay.html index 495e807560104..b9de7e5ce45e1 100644 --- a/src/ui/public/modals/modal_overlay.html +++ b/src/ui/public/modals/modal_overlay.html @@ -1 +1 @@ -
+
diff --git a/src/ui/public/modals/modal_overlay.js b/src/ui/public/modals/modal_overlay.js index 733c1452887ee..0543d532c4fe9 100644 --- a/src/ui/public/modals/modal_overlay.js +++ b/src/ui/public/modals/modal_overlay.js @@ -9,13 +9,11 @@ export class ModalOverlay { this.overlayElement = angular.element(modalOverlayTemplate); this.overlayElement.append(modalElement); - const onKeyDown = (event) => { + angular.element(document.body).on('keydown', (event) => { if(event.keyCode === 27) { scope.onCancel(); } - }; - - angular.element(document.body).bind('keydown', onKeyDown); + }); angular.element(document.body).append(this.overlayElement); } From 13366a576707e3ab4678f541e7141ea457e380f2 Mon Sep 17 00:00:00 2001 From: Doug Penny Date: Tue, 28 Feb 2017 19:26:51 -0800 Subject: [PATCH 4/4] #10099 move ESC handling to confirm_modal.js --- src/ui/public/modals/confirm_modal.js | 11 ++++++++++- src/ui/public/modals/modal_overlay.js | 8 +------- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/ui/public/modals/confirm_modal.js b/src/ui/public/modals/confirm_modal.js index a934696868c10..b1529f242802a 100644 --- a/src/ui/public/modals/confirm_modal.js +++ b/src/ui/public/modals/confirm_modal.js @@ -1,3 +1,4 @@ +import angular from 'angular'; import { noop } from 'lodash'; import uiModules from 'ui/modules'; import template from './confirm_modal.html'; @@ -72,12 +73,20 @@ module.factory('confirmModal', function ($rootScope, $compile) { }; const modalInstance = $compile(template)(confirmScope); - modalPopover = new ModalOverlay(modalInstance, confirmScope); + modalPopover = new ModalOverlay(modalInstance); + + angular.element(document.body).on('keydown', (event) => { + if(event.keyCode === 27) { + confirmScope.onCancel(); + } + }); + modalInstance.find('[data-test-subj=confirmModalConfirmButton]').focus(); function destroy() { modalPopover.destroy(); modalPopover = undefined; + angular.element(document.body).off('keydown'); confirmScope.$destroy(); } }; diff --git a/src/ui/public/modals/modal_overlay.js b/src/ui/public/modals/modal_overlay.js index 0543d532c4fe9..43cd5cda1f159 100644 --- a/src/ui/public/modals/modal_overlay.js +++ b/src/ui/public/modals/modal_overlay.js @@ -5,15 +5,10 @@ import modalOverlayTemplate from './modal_overlay.html'; * Appends the modal to the dom on instantiation, and removes it when destroy is called. */ export class ModalOverlay { - constructor(modalElement, scope) { + constructor(modalElement) { this.overlayElement = angular.element(modalOverlayTemplate); this.overlayElement.append(modalElement); - angular.element(document.body).on('keydown', (event) => { - if(event.keyCode === 27) { - scope.onCancel(); - } - }); angular.element(document.body).append(this.overlayElement); } @@ -21,7 +16,6 @@ export class ModalOverlay { * Removes the overlay and modal from the dom. */ destroy() { - angular.element(document.body).off('keydown'); this.overlayElement.remove(); } }