From 16de62a5c918bd72b4c4967baffbce2c8bdbf80d Mon Sep 17 00:00:00 2001 From: Tomohisa Igarashi Date: Fri, 11 Oct 2024 09:55:29 -0400 Subject: [PATCH] chore: Use string key instead of number for modal action key (#1537) --- .../CustomAutoFields.test.tsx.snap | 15 ------- .../Custom/ContextMenu/ItemDeleteGroup.tsx | 7 +++- .../ContextMenu/ItemDeleteStep.test.tsx | 7 +++- .../Custom/ContextMenu/ItemDeleteStep.tsx | 7 +++- .../Custom/ContextMenu/ItemReplaceStep.tsx | 9 ++-- ...n-confirmaton-modal.provider.test.tsx.snap | 4 +- .../action-confirmation-modal.provider.tsx | 41 +++++++++---------- ...action-confirmaton-modal.provider.test.tsx | 32 +++++++-------- 8 files changed, 59 insertions(+), 63 deletions(-) diff --git a/packages/ui/src/components/Form/__snapshots__/CustomAutoFields.test.tsx.snap b/packages/ui/src/components/Form/__snapshots__/CustomAutoFields.test.tsx.snap index 818637c84..0af9aadef 100644 --- a/packages/ui/src/components/Form/__snapshots__/CustomAutoFields.test.tsx.snap +++ b/packages/ui/src/components/Form/__snapshots__/CustomAutoFields.test.tsx.snap @@ -583,21 +583,6 @@ exports[`CustomAutoFields renders \`AutoFields\` for common fields 1`] = ` value="" /> -
-
-
- -
-
-
{ @@ -22,7 +25,7 @@ export const ItemDeleteGroup: FunctionComponent = (props) text: 'All steps will be lost.', }); - if (!isDeleteConfirmed) return; + if (isDeleteConfirmed !== ACTION_ID_CONFIRM) return; entitiesContext?.camelResource.removeEntity(flowId); entitiesContext?.updateEntitiesFromCamelResource(); diff --git a/packages/ui/src/components/Visualization/Custom/ContextMenu/ItemDeleteStep.test.tsx b/packages/ui/src/components/Visualization/Custom/ContextMenu/ItemDeleteStep.test.tsx index 3c389dfa5..eab866681 100644 --- a/packages/ui/src/components/Visualization/Custom/ContextMenu/ItemDeleteStep.test.tsx +++ b/packages/ui/src/components/Visualization/Custom/ContextMenu/ItemDeleteStep.test.tsx @@ -1,7 +1,10 @@ import { fireEvent, render, waitFor } from '@testing-library/react'; import { createVisualizationNode, IVisualizationNode } from '../../../../models'; import { ItemDeleteStep } from './ItemDeleteStep'; -import { ActionConfirmationModalContext } from '../../../../providers/action-confirmation-modal.provider'; +import { + ACTION_ID_CONFIRM, + ActionConfirmationModalContext, +} from '../../../../providers/action-confirmation-modal.provider'; describe('ItemDeleteStep', () => { const vizNode = createVisualizationNode('test', {}); @@ -39,7 +42,7 @@ describe('ItemDeleteStep', () => { }); it('should call removechild if deletion is confirmed', async () => { - mockDeleteModalContext.actionConfirmation.mockResolvedValueOnce(true); + mockDeleteModalContext.actionConfirmation.mockResolvedValueOnce(ACTION_ID_CONFIRM); const wrapper = render( diff --git a/packages/ui/src/components/Visualization/Custom/ContextMenu/ItemDeleteStep.tsx b/packages/ui/src/components/Visualization/Custom/ContextMenu/ItemDeleteStep.tsx index e4c7c4efe..e5251946b 100644 --- a/packages/ui/src/components/Visualization/Custom/ContextMenu/ItemDeleteStep.tsx +++ b/packages/ui/src/components/Visualization/Custom/ContextMenu/ItemDeleteStep.tsx @@ -4,7 +4,10 @@ import { FunctionComponent, PropsWithChildren, useCallback, useContext } from 'r import { IDataTestID } from '../../../../models'; import { IVisualizationNode } from '../../../../models/visualization/base-visual-entity'; import { EntitiesContext } from '../../../../providers/entities.provider'; -import { ActionConfirmationModalContext } from '../../../../providers/action-confirmation-modal.provider'; +import { + ACTION_ID_CONFIRM, + ActionConfirmationModalContext, +} from '../../../../providers/action-confirmation-modal.provider'; interface ItemDeleteStepProps extends PropsWithChildren { vizNode: IVisualizationNode; @@ -23,7 +26,7 @@ export const ItemDeleteStep: FunctionComponent = (props) => text: 'Step and its children will be lost.', }); - if (!isDeleteConfirmed) return; + if (isDeleteConfirmed !== ACTION_ID_CONFIRM) return; } props.vizNode?.removeChild(); diff --git a/packages/ui/src/components/Visualization/Custom/ContextMenu/ItemReplaceStep.tsx b/packages/ui/src/components/Visualization/Custom/ContextMenu/ItemReplaceStep.tsx index ba58475d5..d45decd53 100644 --- a/packages/ui/src/components/Visualization/Custom/ContextMenu/ItemReplaceStep.tsx +++ b/packages/ui/src/components/Visualization/Custom/ContextMenu/ItemReplaceStep.tsx @@ -5,7 +5,10 @@ import { IDataTestID } from '../../../../models'; import { AddStepMode, IVisualizationNode } from '../../../../models/visualization/base-visual-entity'; import { CatalogModalContext } from '../../../../providers/catalog-modal.provider'; import { EntitiesContext } from '../../../../providers/entities.provider'; -import { ActionConfirmationModalContext } from '../../../../providers/action-confirmation-modal.provider'; +import { + ACTION_ID_CONFIRM, + ActionConfirmationModalContext, +} from '../../../../providers/action-confirmation-modal.provider'; interface ItemReplaceStepProps extends PropsWithChildren { vizNode: IVisualizationNode; @@ -27,7 +30,7 @@ export const ItemReplaceStep: FunctionComponent = (props) text: 'Step and its children will be lost.', }); - if (!isReplaceConfirmed) return; + if (isReplaceConfirmed !== ACTION_ID_CONFIRM) return; } /** Find compatible components */ @@ -45,7 +48,7 @@ export const ItemReplaceStep: FunctionComponent = (props) /** Update entity */ entitiesContext.updateEntitiesFromCamelResource(); - }, [replaceModalContext, catalogModalContext, entitiesContext, props.vizNode]); + }, [props.vizNode, props.loadActionConfirmationModal, entitiesContext, catalogModalContext, replaceModalContext]); return ( diff --git a/packages/ui/src/providers/__snapshots__/action-confirmaton-modal.provider.test.tsx.snap b/packages/ui/src/providers/__snapshots__/action-confirmaton-modal.provider.test.tsx.snap index 06009139c..76d27264e 100644 --- a/packages/ui/src/providers/__snapshots__/action-confirmaton-modal.provider.test.tsx.snap +++ b/packages/ui/src/providers/__snapshots__/action-confirmaton-modal.provider.test.tsx.snap @@ -90,7 +90,7 @@ exports[`ActionConfirmationModalProvider should allow consumers to update the mo data-ouia-component-id="OUIA-Generated-Button-danger-3" data-ouia-component-type="PF5/Button" data-ouia-safe="true" - data-testid="action-confirmation-modal-btn-1" + data-testid="action-confirmation-modal-btn-confirm" type="button" > Confirm @@ -101,7 +101,7 @@ exports[`ActionConfirmationModalProvider should allow consumers to update the mo data-ouia-component-id="OUIA-Generated-Button-link-3" data-ouia-component-type="PF5/Button" data-ouia-safe="true" - data-testid="action-confirmation-modal-btn-0" + data-testid="action-confirmation-modal-btn-cancel" type="button" > Cancel diff --git a/packages/ui/src/providers/action-confirmation-modal.provider.tsx b/packages/ui/src/providers/action-confirmation-modal.provider.tsx index 598a2b7f6..b2f734f45 100644 --- a/packages/ui/src/providers/action-confirmation-modal.provider.tsx +++ b/packages/ui/src/providers/action-confirmation-modal.provider.tsx @@ -1,10 +1,9 @@ import { Button, ButtonVariant, Modal, ModalVariant } from '@patternfly/react-core'; import { FunctionComponent, PropsWithChildren, createContext, useCallback, useMemo, useRef, useState } from 'react'; -export const ACTION_INDEX_CANCEL = 0; -export const ACTION_INDEX_CONFIRM = 1; +export const ACTION_ID_CANCEL = 'cancel'; +export const ACTION_ID_CONFIRM = 'confirm'; export interface ActionConfirmationButtonOption { - index: number; buttonText: string; variant: ButtonVariant; isDanger?: boolean; @@ -14,9 +13,9 @@ interface ActionConfirmationModalContextValue { actionConfirmation: (options: { title?: string; text?: string; - buttonOptions?: ActionConfirmationButtonOption[]; + buttonOptions?: Record; additionalModalText?: string; - }) => Promise; + }) => Promise; } export const ActionConfirmationModalContext = createContext(undefined); @@ -29,20 +28,20 @@ export const ActionConfirmationModalContextProvider: FunctionComponent([]); - const [buttonOptions, setButtonOptions] = useState([]); + const [buttonOptions, setButtonOptions] = useState>({}); const actionConfirmationRef = useRef<{ - resolve: (index: number) => void; + resolve: (actionId: string) => void; reject: (error: unknown) => unknown; }>(); const handleCloseModal = useCallback(() => { setIsModalOpen(false); - actionConfirmationRef.current?.resolve(ACTION_INDEX_CANCEL); + actionConfirmationRef.current?.resolve(ACTION_ID_CANCEL); }, []); - const handleAction = useCallback((index: number) => { + const handleAction = useCallback((actionId: string) => { setIsModalOpen(false); - actionConfirmationRef.current?.resolve(index); + actionConfirmationRef.current?.resolve(actionId); }, []); const actionConfirmation = useCallback( @@ -51,10 +50,10 @@ export const ActionConfirmationModalContextProvider: FunctionComponent; } = {}, ) => { - const actionConfirmationPromise = new Promise((resolve, reject) => { + const actionConfirmationPromise = new Promise((resolve, reject) => { /** Set both resolve and reject functions to be used once the user choose an action */ actionConfirmationRef.current = { resolve, reject }; }); @@ -67,7 +66,7 @@ export const ActionConfirmationModalContextProvider: FunctionComponent ( + ...Object.entries(buttonOptions).map(([actionId, option]) => ( )), , diff --git a/packages/ui/src/providers/action-confirmaton-modal.provider.test.tsx b/packages/ui/src/providers/action-confirmaton-modal.provider.test.tsx index b2b4ed642..34974d2cc 100644 --- a/packages/ui/src/providers/action-confirmaton-modal.provider.test.tsx +++ b/packages/ui/src/providers/action-confirmaton-modal.provider.test.tsx @@ -1,13 +1,15 @@ import { act, fireEvent, render, screen, waitFor } from '@testing-library/react'; import { FunctionComponent, useContext } from 'react'; import { + ACTION_ID_CANCEL, + ACTION_ID_CONFIRM, ActionConfirmationButtonOption, ActionConfirmationModalContext, ActionConfirmationModalContextProvider, } from './action-confirmation-modal.provider'; import { ButtonVariant } from '@patternfly/react-core'; -let actionConfirmationResult: number | undefined; +let actionConfirmationResult: string | undefined; describe('ActionConfirmationModalProvider', () => { beforeEach(() => { @@ -28,7 +30,7 @@ describe('ActionConfirmationModalProvider', () => { fireEvent.click(confirmButton); // Wait for actionConfirmation promise to resolve - await waitFor(() => expect(actionConfirmationResult).toEqual(1)); + await waitFor(() => expect(actionConfirmationResult).toEqual(ACTION_ID_CONFIRM)); }); it('calls actionConfirmation with false when Cancel button is clicked', async () => { @@ -45,7 +47,7 @@ describe('ActionConfirmationModalProvider', () => { fireEvent.click(cancelButton); // Wait for actionConfirmation promise to resolve - await waitFor(() => expect(actionConfirmationResult).toEqual(0)); + await waitFor(() => expect(actionConfirmationResult).toEqual(ACTION_ID_CANCEL)); }); it('should allow consumers to update the modal title and text', () => { @@ -74,19 +76,17 @@ describe('ActionConfirmationModalProvider', () => { title="Custom title" text="Custom text" additionalModalText="Additional text is added in the modal description" - buttonOptions={[ - { - index: 1, + buttonOptions={{ + 'del-step-and-file': { buttonText: 'Delete the step, and delete the file(s)', variant: ButtonVariant.danger, }, - { - index: 2, + 'del-step-only': { buttonText: 'Delete the step, but keep the file(s)', variant: ButtonVariant.secondary, isDanger: true, }, - ]} + }} /> , ); @@ -98,12 +98,12 @@ describe('ActionConfirmationModalProvider', () => { const modalDialog = wrapper.getByRole('dialog'); expect(modalDialog.textContent).toContain('Additional text is added in the modal description'); act(() => { - const cancelButton = wrapper.getByTestId('action-confirmation-modal-btn-0'); + const cancelButton = wrapper.getByTestId('action-confirmation-modal-btn-cancel'); expect(cancelButton.textContent).toEqual('Cancel'); fireEvent.click(cancelButton); }); await waitFor(() => { - expect(actionConfirmationResult).toEqual(0); + expect(actionConfirmationResult).toEqual(ACTION_ID_CANCEL); }); act(() => { @@ -111,12 +111,12 @@ describe('ActionConfirmationModalProvider', () => { fireEvent.click(deleteButton); }); act(() => { - const deleteStepAndFileButton = wrapper.getByTestId('action-confirmation-modal-btn-1'); + const deleteStepAndFileButton = wrapper.getByTestId('action-confirmation-modal-btn-del-step-and-file'); expect(deleteStepAndFileButton.textContent).toEqual('Delete the step, and delete the file(s)'); fireEvent.click(deleteStepAndFileButton); }); await waitFor(() => { - expect(actionConfirmationResult).toEqual(1); + expect(actionConfirmationResult).toEqual('del-step-and-file'); }); act(() => { @@ -124,12 +124,12 @@ describe('ActionConfirmationModalProvider', () => { fireEvent.click(deleteButton); }); act(() => { - const deleteStepOnlyButton = wrapper.getByTestId('action-confirmation-modal-btn-2'); + const deleteStepOnlyButton = wrapper.getByTestId('action-confirmation-modal-btn-del-step-only'); expect(deleteStepOnlyButton.textContent).toEqual('Delete the step, but keep the file(s)'); fireEvent.click(deleteStepOnlyButton); }); await waitFor(() => { - expect(actionConfirmationResult).toEqual(2); + expect(actionConfirmationResult).toEqual('del-step-only'); }); }); }); @@ -138,7 +138,7 @@ interface TestComponentProps { title: string; text: string; additionalModalText?: string; - buttonOptions?: ActionConfirmationButtonOption[]; + buttonOptions?: Record; } const TestComponent: FunctionComponent = (props) => {