Fix modals in react (#11714)
* 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
This commit is contained in:
parent
f5f056c701
commit
a9ee082660
|
@ -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)"
|
||||
>
|
||||
|
|
|
@ -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
|
||||
});
|
||||
};
|
||||
|
||||
|
|
|
@ -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);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
@ -6,4 +6,5 @@
|
|||
cancel-button-text="cancelButtonText"
|
||||
message="message"
|
||||
title="title"
|
||||
default-focused-button="defaultFocusedButton"
|
||||
></confirm-modal>
|
||||
|
|
|
@ -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) {
|
||||
|
|
|
@ -1,2 +1,4 @@
|
|||
import './confirm_modal';
|
||||
import './confirm_modal_promise';
|
||||
|
||||
export { ConfirmationButtonTypes } from './confirm_modal';
|
||||
|
|
69
test/functional/apps/dashboard/_dashboard_listing.js
Normal file
69
test/functional/apps/dashboard/_dashboard_listing.js
Normal file
|
@ -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);
|
||||
});
|
||||
});
|
||||
});
|
||||
}
|
|
@ -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'));
|
||||
});
|
||||
}
|
||||
|
|
|
@ -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');
|
||||
|
|
|
@ -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');
|
||||
}
|
||||
|
@ -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('-',' '));
|
||||
|
|
|
@ -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)
|
||||
};
|
||||
|
|
|
@ -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,7 +44,8 @@ test('onConfirm', () => {
|
|||
sinon.assert.notCalled(onCancel);
|
||||
});
|
||||
|
||||
test('onCancel', () => {
|
||||
describe('onCancel', () => {
|
||||
test('triggerd by click', () => {
|
||||
const component = mount(<KuiConfirmModal
|
||||
message="This is a confirmation modal example"
|
||||
title="A confirmation modal"
|
||||
|
@ -57,5 +58,67 @@ test('onCancel', () => {
|
|||
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);
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
@ -1 +1,2 @@
|
|||
export { SortableProperties } from './sort';
|
||||
export { ESC_KEY_CODE } from './key_codes';
|
||||
|
|
1
ui_framework/services/key_codes.js
Normal file
1
ui_framework/services/key_codes.js
Normal file
|
@ -0,0 +1 @@
|
|||
export const ESC_KEY_CODE = 27;
|
Loading…
Reference in a new issue