Fix modals in react (#11714) (#11862)

* fix focused button not being set

Need to do it in a timeout because ng-react hasn’t loaded the react
code yet, so it can’t find the buttons.

* add a test

that would have caught the issue, and also change the default focused
button for the listing delete page since it’s a destructive action.

* Add a comment

* Push focusing of default button into react code

* Push ESC key handling into react as well

* Remove ESC tests in angular form

They don’t pass, I’m guessing because of the way it’s triggered, but
these test cases should now be handled in jest.  Also confirmed it
works as expected with the modal.

* Address code review comments

* Use autoFocus

src/core_plugins/kibana/public/dashboard/listing/dashboard_listing.html

@@ -43,6 +43,7 @@
           ng-if="listingController.getSelectedItemsCount() > 0"
           tooltip="Delete selected dashboards"
           tooltip-append-to-body="true"
+          data-test-subj="deleteSelectedDashboards"
         >
           <span aria-hidden="true" class="kuiButton__icon kuiIcon fa-trash"></span>
         </button>
@@ -100,6 +101,7 @@
         <div class="kuiPromptForItems__actions">
           <a
             class="kuiButton kuiButton--primary kuiButton--iconText"
+            data-test-subj="createDashboardPromptButton"
             href="{{listingController.getCreateDashboardHref()}}"
           >
             <span class="kuiButton__inner">
@@ -155,6 +157,7 @@
             <input
               type="checkbox"
               class="kuiCheckBox"
+              data-test-subj="dashboardListItemCheckbox"
               ng-click="listingController.toggleItem(item)"
               ng-checked="listingController.isItemChecked(item)"
             >

src/core_plugins/kibana/public/dashboard/listing/dashboard_listing.js

@@ -3,6 +3,7 @@ import 'ui/pager_control';
 import 'ui/pager';
 import { DashboardConstants, createDashboardEditUrl } from '../dashboard_constants';
 import { SortableProperties } from 'ui_framework/services';
+import { ConfirmationButtonTypes } from 'ui/modals';
 
 export function DashboardListingController($injector, $scope) {
   const $filter = $injector.get('$filter');
@@ -125,7 +126,8 @@ export function DashboardListingController($injector, $scope) {
       'Are you sure you want to delete the selected dashboards? This action is irreversible!',
       {
         confirmButtonText: 'Delete',
-        onConfirm: doDelete
+        onConfirm: doDelete,
+        defaultFocusedButton: ConfirmationButtonTypes.CANCEL
       });
   };
 

src/ui/public/modals/__tests__/confirm_modal.js

@@ -114,39 +114,5 @@ describe('ui/modals/confirm_modal', function () {
       expect(confirmCallback.called).to.be(true);
       expect(cancelCallback.called).to.be(false);
     });
-
-    it('onKeyDown detects ESC and calls onCancel', function () {
-      const confirmModalOptions = {
-        confirmButtonText: 'bye',
-        onConfirm: confirmCallback,
-        onCancel: cancelCallback,
-        title: 'hi'
-      };
-
-      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 () {
-      const confirmModalOptions = {
-        confirmButtonText: 'bye',
-        onConfirm: confirmCallback,
-        onCancel: cancelCallback,
-        title: 'hi'
-      };
-
-      confirmModal('hi', confirmModalOptions);
-
-      const e = angular.element.Event('keydown'); // eslint-disable-line new-cap
-      e.keyCode = 50;
-      angular.element(document.body).trigger(e);
-
-      expect(cancelCallback.called).to.be(false);
-    });
   });
 });

src/ui/public/modals/confirm_modal.html

@@ -6,4 +6,5 @@
   cancel-button-text="cancelButtonText"
   message="message"
   title="title"
+  default-focused-button="defaultFocusedButton"
 ></confirm-modal>

src/ui/public/modals/confirm_modal.js

@@ -6,9 +6,11 @@ import { ModalOverlay } from './modal_overlay';
 
 const module = uiModules.get('kibana');
 
+import { CONFIRM_BUTTON, CANCEL_BUTTON } from 'ui_framework/components/modal/confirm_modal';
+
 export const ConfirmationButtonTypes = {
-  CONFIRM: 'Confirm',
-  CANCEL: 'Cancel'
+  CONFIRM: CONFIRM_BUTTON,
+  CANCEL: CANCEL_BUTTON
 };
 
 /**
@@ -48,6 +50,7 @@ module.factory('confirmModal', function ($rootScope, $compile) {
     const confirmScope = $rootScope.$new();
 
     confirmScope.message = message;
+    confirmScope.defaultFocusedButton = options.defaultFocusedButton;
     confirmScope.confirmButtonText = options.confirmButtonText;
     confirmScope.cancelButtonText = options.cancelButtonText;
     confirmScope.title = options.title;
@@ -67,21 +70,6 @@ module.factory('confirmModal', function ($rootScope, $compile) {
     function showModal(confirmScope) {
       const modalInstance = $compile(template)(confirmScope);
       modalPopover = new ModalOverlay(modalInstance);
-      angular.element(document.body).on('keydown', (event) => {
-        if (event.keyCode === 27) {
-          confirmScope.onCancel();
-        }
-      });
-
-      switch (options.defaultFocusedButton) {
-        case ConfirmationButtonTypes.CONFIRM:
-          modalInstance.find('[data-test-subj=confirmModalConfirmButton]').focus();
-          break;
-        case ConfirmationButtonTypes.CANCEL:
-          modalInstance.find('[data-test-subj=confirmModalCancelButton]').focus();
-          break;
-        default:
-      }
     }
 
     if (modalPopover) {

src/ui/public/modals/index.js

@@ -1,2 +1,4 @@
 import './confirm_modal';
 import './confirm_modal_promise';
+
+export { ConfirmationButtonTypes } from './confirm_modal';

test/functional/apps/dashboard/_dashboard_listing.js

@@ -0,0 +1,69 @@
+import expect from 'expect.js';
+
+export default function ({ getPageObjects }) {
+  const PageObjects = getPageObjects(['dashboard', 'header', 'common']);
+
+  describe('dashboard listing page', function describeIndexTests() {
+    const dashboardName = 'Dashboard Listing Test';
+
+    before(async function () {
+      await PageObjects.dashboard.initTests();
+    });
+
+    describe('create prompt', async () => {
+      it('appears when there are no dashboards', async function () {
+        const promptExists = await PageObjects.dashboard.getCreateDashboardPromptExists();
+        expect(promptExists).to.be(true);
+      });
+
+      it('creates a new dashboard', async function () {
+        await PageObjects.dashboard.clickCreateDashboardPrompt();
+        await PageObjects.dashboard.saveDashboard(dashboardName);
+        await PageObjects.header.clickToastOK();
+
+        await PageObjects.dashboard.gotoDashboardLandingPage();
+        const countOfDashboards = await PageObjects.dashboard.getDashboardCountWithName(dashboardName);
+        expect(countOfDashboards).to.equal(1);
+      });
+
+      it('is not shown when there is a dashboard', async function () {
+        const promptExists = await PageObjects.dashboard.getCreateDashboardPromptExists();
+        expect(promptExists).to.be(false);
+      });
+
+      it('is not shown when there are no dashboards shown during a search', async function () {
+        const countOfDashboards = await PageObjects.dashboard.getDashboardCountWithName('gobeldeguck');
+        expect(countOfDashboards).to.equal(0);
+
+        const promptExists = await PageObjects.dashboard.getCreateDashboardPromptExists();
+        expect(promptExists).to.be(false);
+      });
+    });
+
+    describe('delete', async function () {
+      it('default confirm action is cancel', async function () {
+        await PageObjects.dashboard.searchForDashboardWithName('');
+        await PageObjects.dashboard.clickListItemCheckbox();
+        await PageObjects.dashboard.clickDeleteSelectedDashboards();
+
+        await PageObjects.common.pressEnterKey();
+
+        const isConfirmOpen = await PageObjects.common.isConfirmModalOpen();
+        expect(isConfirmOpen).to.be(false);
+
+        const countOfDashboards = await PageObjects.dashboard.getDashboardCountWithName(dashboardName);
+        expect(countOfDashboards).to.equal(1);
+      });
+
+      it('succeeds on confirmation press', async function () {
+        await PageObjects.dashboard.clickListItemCheckbox();
+        await PageObjects.dashboard.clickDeleteSelectedDashboards();
+
+        await PageObjects.common.clickConfirmOnModal();
+
+        const countOfDashboards = await PageObjects.dashboard.getDashboardCountWithName(dashboardName);
+        expect(countOfDashboards).to.equal(0);
+      });
+    });
+  });
+}

test/functional/apps/dashboard/index.js

@@ -8,5 +8,6 @@ export default function ({ getService, loadTestFile }) {
     loadTestFile(require.resolve('./_dashboard'));
     loadTestFile(require.resolve('./_dashboard_save'));
     loadTestFile(require.resolve('./_dashboard_time'));
+    loadTestFile(require.resolve('./_dashboard_listing'));
   });
 }

test/functional/page_objects/common_page.js

@@ -229,6 +229,10 @@ export function CommonPageProvider({ getService, getPageObjects }) {
       await this.ensureModalOverlayHidden();
     }
 
+    async pressEnterKey() {
+      await remote.pressKeys('\uE007');
+    }
+
     async clickCancelOnModal() {
       log.debug('Clicking modal cancel');
       await testSubjects.click('confirmModalCancelButton');

test/functional/page_objects/dashboard_page.js

@@ -93,6 +93,22 @@ export function DashboardPageProvider({ getService, getPageObjects }) {
       return testSubjects.click('newDashboardLink');
     }
 
+    async clickCreateDashboardPrompt() {
+      await retry.try(() => testSubjects.click('createDashboardPromptButton'));
+    }
+
+    async getCreateDashboardPromptExists() {
+      return await testSubjects.exists('createDashboardPromptButton');
+    }
+
+    async clickListItemCheckbox() {
+      await testSubjects.click('dashboardListItemCheckbox');
+    }
+
+    async clickDeleteSelectedDashboards() {
+      await testSubjects.click('deleteSelectedDashboards');
+    }
+
     clickAddVisualization() {
       return testSubjects.click('dashboardAddPanelButton');
     }
@@ -135,9 +151,9 @@ export function DashboardPageProvider({ getService, getPageObjects }) {
 
     filterVizNames(vizName) {
       return retry.try(() => getRemote()
-      .findByCssSelector('input[placeholder="Visualizations Filter..."]')
-      .click()
-      .pressKeys(vizName));
+        .findByCssSelector('input[placeholder="Visualizations Filter..."]')
+        .click()
+        .pressKeys(vizName));
     }
 
     clickVizNameLink(vizName) {
@@ -242,6 +258,7 @@ export function DashboardPageProvider({ getService, getPageObjects }) {
 
       await retry.try(async () => {
         const searchFilter = await testSubjects.find('searchFilter');
+        await searchFilter.clearValue();
         await searchFilter.click();
         // Note: this replacement of - to space is to preserve original logic but I'm not sure why or if it's needed.
         await searchFilter.type(dashName.replace('-',' '));

ui_framework/components/modal/confirm_modal.js

@@ -8,6 +8,15 @@ import { KuiModalHeaderTitle } from './modal_header_title';
 import { KuiModalBody } from './modal_body';
 import { KuiModalBodyText } from './modal_body_text';
 import { KuiButton } from '../index';
+import { ESC_KEY_CODE } from '../../services';
+
+export const CONFIRM_BUTTON = 'confirm';
+export const CANCEL_BUTTON = 'cancel';
+
+const CONFIRM_MODAL_BUTTONS = [
+  CONFIRM_BUTTON,
+  CANCEL_BUTTON,
+];
 
 export function KuiConfirmModal({
     message,
@@ -17,7 +26,17 @@ export function KuiConfirmModal({
     cancelButtonText,
     confirmButtonText,
     className,
-    ...rest }) {
+    defaultFocusedButton,
+    ...rest,
+  }) {
+
+  const onKeyDown = (event) => {
+    // Treat the 'esc' key as a cancel indicator.
+    if (event.keyCode === ESC_KEY_CODE) {
+      onCancel();
+    }
+  };
+
   const ariaLabel = rest['aria-label'];
   const dataTestSubj = rest['data-test-subj'];
   return (
@@ -26,6 +45,7 @@ export function KuiConfirmModal({
       data-tests-subj={ dataTestSubj }
       aria-label={ ariaLabel }
       className={ className }
+      onKeyDown={ onKeyDown }
     >
       {
         title ?
@@ -45,6 +65,7 @@ export function KuiConfirmModal({
       <KuiModalFooter>
         <KuiButton
           type="hollow"
+          autoFocus={ defaultFocusedButton === CANCEL_BUTTON }
           data-test-subj="confirmModalCancelButton"
           onClick={ onCancel }
         >
@@ -52,6 +73,7 @@ export function KuiConfirmModal({
         </KuiButton>
         <KuiButton
           type="primary"
+          autoFocus={ defaultFocusedButton === CONFIRM_BUTTON }
           data-test-subj="confirmModalConfirmButton"
           onClick={ onConfirm }
         >
@@ -72,4 +94,5 @@ KuiConfirmModal.propTypes = {
   dataTestSubj: PropTypes.string,
   ariaLabel: PropTypes.string,
   className: PropTypes.string,
+  defaultFocusedButton: PropTypes.oneOf(CONFIRM_MODAL_BUTTONS)
 };

ui_framework/components/modal/confirm_modal.test.js

@@ -5,7 +5,7 @@ import { mount, render } from 'enzyme';
 import { requiredProps } from '../../test/required_props';
 
 import {
-  KuiConfirmModal,
+  CANCEL_BUTTON, CONFIRM_BUTTON, KuiConfirmModal,
 } from './confirm_modal';
 
 let onConfirm;
@@ -44,18 +44,81 @@ test('onConfirm', () => {
   sinon.assert.notCalled(onCancel);
 });
 
-test('onCancel', () => {
-  const component = mount(<KuiConfirmModal
-    message="This is a confirmation modal example"
-    title="A confirmation modal"
-    onCancel={onCancel}
-    onConfirm={onConfirm}
-    cancelButtonText="Cancel"
-    confirmButtonText="Confirm"
-    { ...requiredProps }
-  />);
-  component.find('[data-test-subj="confirmModalCancelButton"]').simulate('click');
-  sinon.assert.notCalled(onConfirm);
-  sinon.assert.calledOnce(onCancel);
+describe('onCancel', () => {
+  test('triggerd by click', () => {
+    const component = mount(<KuiConfirmModal
+      message="This is a confirmation modal example"
+      title="A confirmation modal"
+      onCancel={onCancel}
+      onConfirm={onConfirm}
+      cancelButtonText="Cancel"
+      confirmButtonText="Confirm"
+      { ...requiredProps }
+    />);
+    component.find('[data-test-subj="confirmModalCancelButton"]').simulate('click');
+    sinon.assert.notCalled(onConfirm);
+    sinon.assert.calledOnce(onCancel);
+  });
+
+  test('triggered by esc key', () => {
+    const component = mount(<KuiConfirmModal
+      message="This is a confirmation modal example"
+      title="A confirmation modal"
+      onCancel={onCancel}
+      onConfirm={onConfirm}
+      cancelButtonText="Cancel"
+      confirmButtonText="Confirm"
+      { ...requiredProps }
+    />);
+    component.simulate('keydown', { keyCode: 27 });
+    sinon.assert.notCalled(onConfirm);
+    sinon.assert.calledOnce(onCancel);
+  });
+});
+
+describe('defaultFocusedButton', () => {
+  test('is cancel', () => {
+    const component = mount(<KuiConfirmModal
+      message="This is a confirmation modal example"
+      title="A confirmation modal"
+      onCancel={onCancel}
+      onConfirm={onConfirm}
+      cancelButtonText="Cancel"
+      confirmButtonText="Confirm"
+      defaultFocusedButton={ CANCEL_BUTTON }
+      { ...requiredProps }
+    />);
+    const button = component.find('[data-test-subj="confirmModalCancelButton"]').getDOMNode();
+    expect(document.activeElement).toEqual(button);
+  });
+
+  test('is confirm', () => {
+    const component = mount(<KuiConfirmModal
+      message="This is a confirmation modal example"
+      title="A confirmation modal"
+      onCancel={onCancel}
+      onConfirm={onConfirm}
+      cancelButtonText="Cancel"
+      confirmButtonText="Confirm"
+      defaultFocusedButton={ CONFIRM_BUTTON }
+      { ...requiredProps }
+    />);
+    const button = component.find('[data-test-subj="confirmModalConfirmButton"]').getDOMNode();
+    expect(document.activeElement).toEqual(button);
+  });
+
+  test('when not given focuses on the confirm button', () => {
+    const component = mount(<KuiConfirmModal
+      message="This is a confirmation modal example"
+      title="A confirmation modal"
+      onCancel={onCancel}
+      onConfirm={onConfirm}
+      cancelButtonText="Cancel"
+      confirmButtonText="Confirm"
+      { ...requiredProps }
+    />);
+    const button = component.find('[data-test-subj="confirmModalConfirmButton"]').getDOMNode();
+    expect(document.activeElement).toEqual(button);
+  });
 });
 

ui_framework/services/index.js

@@ -1 +1,2 @@
 export { SortableProperties } from './sort';
+export { ESC_KEY_CODE } from './key_codes';

ui_framework/services/key_codes.js

@@ -0,0 +1 @@
+export const ESC_KEY_CODE = 27;