Skip to content

Commit

Permalink
Improve modals
Browse files Browse the repository at this point in the history
  • Loading branch information
cnasikas committed Dec 8, 2020
1 parent 7550390 commit 1da1728
Show file tree
Hide file tree
Showing 9 changed files with 58 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const ExistingCaseComponent: React.FC<ExistingCaseProps> = ({ onCaseChanged, sel

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

const { Modal: CreateCaseModal, openModal } = useCreateCaseModal({ onCaseCreated });
const { modal, openModal } = useCreateCaseModal({ onCaseCreated });

const onChange = useCallback(
(id: string) => {
Expand All @@ -46,7 +46,7 @@ const ExistingCaseComponent: React.FC<ExistingCaseProps> = ({ onCaseChanged, sel
selectedCase={selectedCase ?? undefined}
onCaseChanged={onChange}
/>
<CreateCaseModal />
{modal}
</>
);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ const AddToCaseActionComponent: React.FC<AddToCaseActionProps> = ({ ecsRowData,
[postComment, eventId, eventIndex, dispatchToaster]
);

const { Modal: CreateCaseModal, openModal: openCreateCaseModal } = useCreateCaseModal({
const { modal: createCaseModal, openModal: openCreateCaseModal } = useCreateCaseModal({
onCaseCreated: attachAlertToCase,
});

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

const { Modal: AllCasesModal, openModal: openAllCaseModal } = useAllCasesModal({
const { modal: allCasesModal, openModal: openAllCaseModal } = useAllCasesModal({
onRowClick: onCaseClicked,
});

Expand Down Expand Up @@ -141,8 +141,8 @@ const AddToCaseActionComponent: React.FC<AddToCaseActionProps> = ({ ecsRowData,
<EuiContextMenuPanel items={items} />
</EuiPopover>
</ActionIconItem>
<CreateCaseModal />
<AllCasesModal />
{createCaseModal}
{allCasesModal}
</>
);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ jest.mock('../../../common/lib/kibana', () => {
const onCloseCaseModal = jest.fn();
const onRowClick = jest.fn();
const defaultProps = {
isModalOpen: true,
onCloseCaseModal,
onRowClick,
};
Expand All @@ -46,6 +47,16 @@ describe('AllCasesModal', () => {
expect(wrapper.find(`[data-test-subj='all-cases-modal']`).exists()).toBeTruthy();
});

it('it does not render the modal isModalOpen=false ', () => {
const wrapper = mount(
<TestProviders>
<AllCasesModal {...defaultProps} isModalOpen={false} />
</TestProviders>
);

expect(wrapper.find(`[data-test-subj='all-cases-modal']`).exists()).toBeFalsy();
});

it('Closing modal calls onCloseCaseModal', () => {
const wrapper = mount(
<TestProviders>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,20 @@ import { AllCases } from '../all_cases';
import * as i18n from './translations';

export interface AllCasesModalProps {
isModalOpen: boolean;
onCloseCaseModal: () => void;
onRowClick: (theCase?: Case) => void;
}

const AllCasesModalComponent: React.FC<AllCasesModalProps> = ({ onCloseCaseModal, onRowClick }) => {
const AllCasesModalComponent: React.FC<AllCasesModalProps> = ({
isModalOpen,
onCloseCaseModal,
onRowClick,
}) => {
const userPermissions = useGetUserSavedObjectPermissions();
const userCanCrud = userPermissions?.crud ?? false;

return (
return isModalOpen ? (
<EuiOverlayMask data-test-subj="all-cases-modal">
<EuiModal onClose={onCloseCaseModal}>
<EuiModalHeader>
Expand All @@ -38,7 +43,7 @@ const AllCasesModalComponent: React.FC<AllCasesModalProps> = ({ onCloseCaseModal
</EuiModalBody>
</EuiModal>
</EuiOverlayMask>
);
) : null;
};

export const AllCasesModal = memo(AllCasesModalComponent);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ jest.mock('react-redux', () => {
jest.mock('../../../common/lib/kibana');
jest.mock('../all_cases', () => {
return {
AllCases: ({ onRowClick }: { onRowClick: ({ id }: { id: string }) => void }) => {
AllCases: ({ onRowClick }: { onRowClick: (id: string, title: string) => void }) => {
return (
<button type="button" onClick={() => onRowClick({ id: 'case-id' })}>
<button type="button" onClick={() => onRowClick('case-id', 'case title')}>
{'case-row'}
</button>
);
Expand Down Expand Up @@ -122,14 +122,14 @@ describe('useAllCasesModal', () => {
result.current.openModal();
});

const Modal = result.current.Modal;
render(<Modal />);
const modal = result.current.modal;
render(<>{modal}</>);

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

expect(result.current.isModalOpen).toBe(false);
expect(onRowClick).toHaveBeenCalledWith({ id: 'case-id' });
expect(onRowClick).toHaveBeenCalledWith('case-id', 'case title');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export interface UseAllCasesModalProps {
}

export interface UseAllCasesModalReturnedValues {
Modal: React.FC;
modal: JSX.Element;
isModalOpen: boolean;
closeModal: () => void;
openModal: () => void;
Expand All @@ -33,21 +33,21 @@ export const useAllCasesModal = ({
[closeModal, onRowClick]
);

const Modal: React.FC = useCallback(
() =>
isModalOpen ? <AllCasesModal onCloseCaseModal={closeModal} onRowClick={onClick} /> : null,
[closeModal, onClick, isModalOpen]
);

const state = useMemo(
() => ({
Modal,
modal: (
<AllCasesModal
isModalOpen={isModalOpen}
onCloseCaseModal={closeModal}
onRowClick={onClick}
/>
),
isModalOpen,
closeModal,
openModal,
onRowClick,
}),
[isModalOpen, closeModal, openModal, onRowClick, Modal]
[isModalOpen, closeModal, onClick, openModal, onRowClick]
);

return state;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { Case } from '../../containers/types';
import * as i18n from '../../translations';

export interface CreateCaseModalProps {
isModalOpen: boolean;
onCloseCaseModal: () => void;
onSuccess: (theCase: Case) => void;
}
Expand All @@ -32,8 +33,12 @@ const Container = styled.div`
`}
`;

const CreateModalComponent: React.FC<CreateCaseModalProps> = ({ onCloseCaseModal, onSuccess }) => {
return (
const CreateModalComponent: React.FC<CreateCaseModalProps> = ({
isModalOpen,
onCloseCaseModal,
onSuccess,
}) => {
return isModalOpen ? (
<EuiOverlayMask data-test-subj="all-cases-modal">
<EuiModal onClose={onCloseCaseModal}>
<EuiModalHeader>
Expand All @@ -49,7 +54,7 @@ const CreateModalComponent: React.FC<CreateCaseModalProps> = ({ onCloseCaseModal
</EuiModalBody>
</EuiModal>
</EuiOverlayMask>
);
) : null;
};

export const CreateCaseModal = memo(CreateModalComponent);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ interface Props {
onCaseCreated: (theCase: Case) => void;
}
export interface UseAllCasesModalReturnedValues {
Modal: React.FC;
modal: JSX.Element;
isModalOpen: boolean;
closeModal: () => void;
openModal: () => void;
Expand All @@ -30,20 +30,20 @@ export const useCreateCaseModal = ({ onCaseCreated }: Props) => {
[onCaseCreated, closeModal]
);

const Modal: React.FC = useCallback(
() =>
isModalOpen ? <CreateCaseModal onCloseCaseModal={closeModal} onSuccess={onSuccess} /> : null,
[closeModal, isModalOpen, onSuccess]
);

const state = useMemo(
() => ({
Modal,
modal: (
<CreateCaseModal
isModalOpen={isModalOpen}
onCloseCaseModal={closeModal}
onSuccess={onSuccess}
/>
),
isModalOpen,
closeModal,
openModal,
}),
[isModalOpen, closeModal, openModal, Modal]
[isModalOpen, closeModal, onSuccess, openModal]
);

return state;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ const AddToCaseButtonComponent: React.FC<Props> = ({ timelineId }) => {
[dispatch, graphEventId, navigateToApp, savedObjectId, timelineId, timelineTitle]
);

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

const handleButtonClick = useCallback(() => {
setPopover((currentIsOpen) => !currentIsOpen);
Expand Down Expand Up @@ -155,7 +155,7 @@ const AddToCaseButtonComponent: React.FC<Props> = ({ timelineId }) => {
>
<EuiContextMenuPanel items={items} />
</EuiPopover>
<AllCasesModal />
{allCasesModal}
</>
);
};
Expand Down

0 comments on commit 1da1728

Please sign in to comment.