Skip to content

Commit 1da1728

Browse files
committed
Improve modals
1 parent 7550390 commit 1da1728

File tree

9 files changed

+58
-37
lines changed

9 files changed

+58
-37
lines changed

x-pack/plugins/security_solution/public/cases/components/connectors/case/existing_case.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ const ExistingCaseComponent: React.FC<ExistingCaseProps> = ({ onCaseChanged, sel
1919

2020
const onCaseCreated = useCallback(() => refetchCases(), [refetchCases]);
2121

22-
const { Modal: CreateCaseModal, openModal } = useCreateCaseModal({ onCaseCreated });
22+
const { modal, openModal } = useCreateCaseModal({ onCaseCreated });
2323

2424
const onChange = useCallback(
2525
(id: string) => {
@@ -46,7 +46,7 @@ const ExistingCaseComponent: React.FC<ExistingCaseProps> = ({ onCaseChanged, sel
4646
selectedCase={selectedCase ?? undefined}
4747
onCaseChanged={onChange}
4848
/>
49-
<CreateCaseModal />
49+
{modal}
5050
</>
5151
);
5252
};

x-pack/plugins/security_solution/public/cases/components/timeline_actions/add_to_case_action.tsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ const AddToCaseActionComponent: React.FC<AddToCaseActionProps> = ({ ecsRowData,
5454
[postComment, eventId, eventIndex, dispatchToaster]
5555
);
5656

57-
const { Modal: CreateCaseModal, openModal: openCreateCaseModal } = useCreateCaseModal({
57+
const { modal: createCaseModal, openModal: openCreateCaseModal } = useCreateCaseModal({
5858
onCaseCreated: attachAlertToCase,
5959
});
6060

@@ -75,7 +75,7 @@ const AddToCaseActionComponent: React.FC<AddToCaseActionProps> = ({ ecsRowData,
7575
[attachAlertToCase, openCreateCaseModal]
7676
);
7777

78-
const { Modal: AllCasesModal, openModal: openAllCaseModal } = useAllCasesModal({
78+
const { modal: allCasesModal, openModal: openAllCaseModal } = useAllCasesModal({
7979
onRowClick: onCaseClicked,
8080
});
8181

@@ -141,8 +141,8 @@ const AddToCaseActionComponent: React.FC<AddToCaseActionProps> = ({ ecsRowData,
141141
<EuiContextMenuPanel items={items} />
142142
</EuiPopover>
143143
</ActionIconItem>
144-
<CreateCaseModal />
145-
<AllCasesModal />
144+
{createCaseModal}
145+
{allCasesModal}
146146
</>
147147
);
148148
};

x-pack/plugins/security_solution/public/cases/components/use_all_cases_modal/all_cases_modal.test.tsx

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ jest.mock('../../../common/lib/kibana', () => {
2727
const onCloseCaseModal = jest.fn();
2828
const onRowClick = jest.fn();
2929
const defaultProps = {
30+
isModalOpen: true,
3031
onCloseCaseModal,
3132
onRowClick,
3233
};
@@ -46,6 +47,16 @@ describe('AllCasesModal', () => {
4647
expect(wrapper.find(`[data-test-subj='all-cases-modal']`).exists()).toBeTruthy();
4748
});
4849

50+
it('it does not render the modal isModalOpen=false ', () => {
51+
const wrapper = mount(
52+
<TestProviders>
53+
<AllCasesModal {...defaultProps} isModalOpen={false} />
54+
</TestProviders>
55+
);
56+
57+
expect(wrapper.find(`[data-test-subj='all-cases-modal']`).exists()).toBeFalsy();
58+
});
59+
4960
it('Closing modal calls onCloseCaseModal', () => {
5061
const wrapper = mount(
5162
<TestProviders>

x-pack/plugins/security_solution/public/cases/components/use_all_cases_modal/all_cases_modal.tsx

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,20 @@ import { AllCases } from '../all_cases';
1919
import * as i18n from './translations';
2020

2121
export interface AllCasesModalProps {
22+
isModalOpen: boolean;
2223
onCloseCaseModal: () => void;
2324
onRowClick: (theCase?: Case) => void;
2425
}
2526

26-
const AllCasesModalComponent: React.FC<AllCasesModalProps> = ({ onCloseCaseModal, onRowClick }) => {
27+
const AllCasesModalComponent: React.FC<AllCasesModalProps> = ({
28+
isModalOpen,
29+
onCloseCaseModal,
30+
onRowClick,
31+
}) => {
2732
const userPermissions = useGetUserSavedObjectPermissions();
2833
const userCanCrud = userPermissions?.crud ?? false;
2934

30-
return (
35+
return isModalOpen ? (
3136
<EuiOverlayMask data-test-subj="all-cases-modal">
3237
<EuiModal onClose={onCloseCaseModal}>
3338
<EuiModalHeader>
@@ -38,7 +43,7 @@ const AllCasesModalComponent: React.FC<AllCasesModalProps> = ({ onCloseCaseModal
3843
</EuiModalBody>
3944
</EuiModal>
4045
</EuiOverlayMask>
41-
);
46+
) : null;
4247
};
4348

4449
export const AllCasesModal = memo(AllCasesModalComponent);

x-pack/plugins/security_solution/public/cases/components/use_all_cases_modal/index.test.tsx

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,9 @@ jest.mock('react-redux', () => {
2929
jest.mock('../../../common/lib/kibana');
3030
jest.mock('../all_cases', () => {
3131
return {
32-
AllCases: ({ onRowClick }: { onRowClick: ({ id }: { id: string }) => void }) => {
32+
AllCases: ({ onRowClick }: { onRowClick: (id: string, title: string) => void }) => {
3333
return (
34-
<button type="button" onClick={() => onRowClick({ id: 'case-id' })}>
34+
<button type="button" onClick={() => onRowClick('case-id', 'case title')}>
3535
{'case-row'}
3636
</button>
3737
);
@@ -122,14 +122,14 @@ describe('useAllCasesModal', () => {
122122
result.current.openModal();
123123
});
124124

125-
const Modal = result.current.Modal;
126-
render(<Modal />);
125+
const modal = result.current.modal;
126+
render(<>{modal}</>);
127127

128128
act(() => {
129129
userEvent.click(screen.getByText('case-row'));
130130
});
131131

132132
expect(result.current.isModalOpen).toBe(false);
133-
expect(onRowClick).toHaveBeenCalledWith({ id: 'case-id' });
133+
expect(onRowClick).toHaveBeenCalledWith('case-id', 'case title');
134134
});
135135
});

x-pack/plugins/security_solution/public/cases/components/use_all_cases_modal/index.tsx

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ export interface UseAllCasesModalProps {
1313
}
1414

1515
export interface UseAllCasesModalReturnedValues {
16-
Modal: React.FC;
16+
modal: JSX.Element;
1717
isModalOpen: boolean;
1818
closeModal: () => void;
1919
openModal: () => void;
@@ -33,21 +33,21 @@ export const useAllCasesModal = ({
3333
[closeModal, onRowClick]
3434
);
3535

36-
const Modal: React.FC = useCallback(
37-
() =>
38-
isModalOpen ? <AllCasesModal onCloseCaseModal={closeModal} onRowClick={onClick} /> : null,
39-
[closeModal, onClick, isModalOpen]
40-
);
41-
4236
const state = useMemo(
4337
() => ({
44-
Modal,
38+
modal: (
39+
<AllCasesModal
40+
isModalOpen={isModalOpen}
41+
onCloseCaseModal={closeModal}
42+
onRowClick={onClick}
43+
/>
44+
),
4545
isModalOpen,
4646
closeModal,
4747
openModal,
4848
onRowClick,
4949
}),
50-
[isModalOpen, closeModal, openModal, onRowClick, Modal]
50+
[isModalOpen, closeModal, onClick, openModal, onRowClick]
5151
);
5252

5353
return state;

x-pack/plugins/security_solution/public/cases/components/use_create_case_modal/create_case_modal.tsx

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import { Case } from '../../containers/types';
2121
import * as i18n from '../../translations';
2222

2323
export interface CreateCaseModalProps {
24+
isModalOpen: boolean;
2425
onCloseCaseModal: () => void;
2526
onSuccess: (theCase: Case) => void;
2627
}
@@ -32,8 +33,12 @@ const Container = styled.div`
3233
`}
3334
`;
3435

35-
const CreateModalComponent: React.FC<CreateCaseModalProps> = ({ onCloseCaseModal, onSuccess }) => {
36-
return (
36+
const CreateModalComponent: React.FC<CreateCaseModalProps> = ({
37+
isModalOpen,
38+
onCloseCaseModal,
39+
onSuccess,
40+
}) => {
41+
return isModalOpen ? (
3742
<EuiOverlayMask data-test-subj="all-cases-modal">
3843
<EuiModal onClose={onCloseCaseModal}>
3944
<EuiModalHeader>
@@ -49,7 +54,7 @@ const CreateModalComponent: React.FC<CreateCaseModalProps> = ({ onCloseCaseModal
4954
</EuiModalBody>
5055
</EuiModal>
5156
</EuiOverlayMask>
52-
);
57+
) : null;
5358
};
5459

5560
export const CreateCaseModal = memo(CreateModalComponent);

x-pack/plugins/security_solution/public/cases/components/use_create_case_modal/index.tsx

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ interface Props {
1212
onCaseCreated: (theCase: Case) => void;
1313
}
1414
export interface UseAllCasesModalReturnedValues {
15-
Modal: React.FC;
15+
modal: JSX.Element;
1616
isModalOpen: boolean;
1717
closeModal: () => void;
1818
openModal: () => void;
@@ -30,20 +30,20 @@ export const useCreateCaseModal = ({ onCaseCreated }: Props) => {
3030
[onCaseCreated, closeModal]
3131
);
3232

33-
const Modal: React.FC = useCallback(
34-
() =>
35-
isModalOpen ? <CreateCaseModal onCloseCaseModal={closeModal} onSuccess={onSuccess} /> : null,
36-
[closeModal, isModalOpen, onSuccess]
37-
);
38-
3933
const state = useMemo(
4034
() => ({
41-
Modal,
35+
modal: (
36+
<CreateCaseModal
37+
isModalOpen={isModalOpen}
38+
onCloseCaseModal={closeModal}
39+
onSuccess={onSuccess}
40+
/>
41+
),
4242
isModalOpen,
4343
closeModal,
4444
openModal,
4545
}),
46-
[isModalOpen, closeModal, openModal, Modal]
46+
[isModalOpen, closeModal, onSuccess, openModal]
4747
);
4848

4949
return state;

x-pack/plugins/security_solution/public/timelines/components/flyout/add_to_case_button/index.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ const AddToCaseButtonComponent: React.FC<Props> = ({ timelineId }) => {
6262
[dispatch, graphEventId, navigateToApp, savedObjectId, timelineId, timelineTitle]
6363
);
6464

65-
const { Modal: AllCasesModal, openModal: openCaseModal } = useAllCasesModal({ onRowClick });
65+
const { modal: allCasesModal, openModal: openCaseModal } = useAllCasesModal({ onRowClick });
6666

6767
const handleButtonClick = useCallback(() => {
6868
setPopover((currentIsOpen) => !currentIsOpen);
@@ -155,7 +155,7 @@ const AddToCaseButtonComponent: React.FC<Props> = ({ timelineId }) => {
155155
>
156156
<EuiContextMenuPanel items={items} />
157157
</EuiPopover>
158-
<AllCasesModal />
158+
{allCasesModal}
159159
</>
160160
);
161161
};

0 commit comments

Comments
 (0)