Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Use string key instead of number for modal action key (#1537) #1569

Merged
merged 1 commit into from
Oct 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ import { ContextMenuItem } from '@patternfly/react-topology';
import { FunctionComponent, PropsWithChildren, useCallback, useContext } from 'react';
import { IDataTestID } from '../../../../models';
import { IVisualizationNode } from '../../../../models/visualization/base-visual-entity';
import { ActionConfirmationModalContext } from '../../../../providers/action-confirmation-modal.provider';
import {
ACTION_ID_CONFIRM,
ActionConfirmationModalContext,
} from '../../../../providers/action-confirmation-modal.provider';
import { EntitiesContext } from '../../../../providers/entities.provider';

interface ItemDeleteGroupProps extends PropsWithChildren<IDataTestID> {
Expand All @@ -22,7 +25,7 @@ export const ItemDeleteGroup: FunctionComponent<ItemDeleteGroupProps> = (props)
text: 'All steps will be lost.',
});

if (!isDeleteConfirmed) return;
if (isDeleteConfirmed !== ACTION_ID_CONFIRM) return;

entitiesContext?.camelResource.removeEntity(flowId);
entitiesContext?.updateEntitiesFromCamelResource();
Expand Down
Original file line number Diff line number Diff line change
@@ -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', {});
Expand Down Expand Up @@ -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(
<ActionConfirmationModalContext.Provider value={mockDeleteModalContext}>
<ItemDeleteStep vizNode={mockVizNode} loadActionConfirmationModal={true} />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<IDataTestID> {
vizNode: IVisualizationNode;
Expand All @@ -23,7 +26,7 @@ export const ItemDeleteStep: FunctionComponent<ItemDeleteStepProps> = (props) =>
text: 'Step and its children will be lost.',
});

if (!isDeleteConfirmed) return;
if (isDeleteConfirmed !== ACTION_ID_CONFIRM) return;
}

props.vizNode?.removeChild();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<IDataTestID> {
vizNode: IVisualizationNode;
Expand All @@ -27,7 +30,7 @@ export const ItemReplaceStep: FunctionComponent<ItemReplaceStepProps> = (props)
text: 'Step and its children will be lost.',
});

if (!isReplaceConfirmed) return;
if (isReplaceConfirmed !== ACTION_ID_CONFIRM) return;
}

/** Find compatible components */
Expand All @@ -45,7 +48,7 @@ export const ItemReplaceStep: FunctionComponent<ItemReplaceStepProps> = (props)

/** Update entity */
entitiesContext.updateEntitiesFromCamelResource();
}, [replaceModalContext, catalogModalContext, entitiesContext, props.vizNode]);
}, [props.vizNode, props.loadActionConfirmationModal, entitiesContext, catalogModalContext, replaceModalContext]);

return (
<ContextMenuItem onClick={onReplaceNode} data-testid={props['data-testid']}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
41 changes: 20 additions & 21 deletions packages/ui/src/providers/action-confirmation-modal.provider.tsx
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -14,9 +13,9 @@ interface ActionConfirmationModalContextValue {
actionConfirmation: (options: {
title?: string;
text?: string;
buttonOptions?: ActionConfirmationButtonOption[];
buttonOptions?: Record<string, ActionConfirmationButtonOption>;
additionalModalText?: string;
}) => Promise<number>;
}) => Promise<string>;
}

export const ActionConfirmationModalContext = createContext<ActionConfirmationModalContextValue | undefined>(undefined);
Expand All @@ -29,20 +28,20 @@ export const ActionConfirmationModalContextProvider: FunctionComponent<PropsWith
const [isModalOpen, setIsModalOpen] = useState(false);
const [title, setTitle] = useState('');
const [textParagraphs, setTextParagraphs] = useState<string[]>([]);
const [buttonOptions, setButtonOptions] = useState<ActionConfirmationButtonOption[]>([]);
const [buttonOptions, setButtonOptions] = useState<Record<string, ActionConfirmationButtonOption>>({});
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(
Expand All @@ -51,10 +50,10 @@ export const ActionConfirmationModalContextProvider: FunctionComponent<PropsWith
title?: string;
text?: string;
additionalModalText?: string;
buttonOptions?: ActionConfirmationButtonOption[];
buttonOptions?: Record<string, ActionConfirmationButtonOption>;
} = {},
) => {
const actionConfirmationPromise = new Promise<number>((resolve, reject) => {
const actionConfirmationPromise = new Promise<string>((resolve, reject) => {
/** Set both resolve and reject functions to be used once the user choose an action */
actionConfirmationRef.current = { resolve, reject };
});
Expand All @@ -67,7 +66,7 @@ export const ActionConfirmationModalContextProvider: FunctionComponent<PropsWith
setTextParagraphs(textParagraphs);
options.buttonOptions
? setButtonOptions(options.buttonOptions)
: setButtonOptions([{ index: ACTION_INDEX_CONFIRM, buttonText: 'Confirm', variant: ButtonVariant.danger }]);
: setButtonOptions({ [ACTION_ID_CONFIRM]: { buttonText: 'Confirm', variant: ButtonVariant.danger } });
setIsModalOpen(true);

return actionConfirmationPromise;
Expand Down Expand Up @@ -95,22 +94,22 @@ export const ActionConfirmationModalContextProvider: FunctionComponent<PropsWith
onClose={handleCloseModal}
ouiaId="ActionConfirmationModal"
actions={[
...buttonOptions.map((op) => (
...Object.entries(buttonOptions).map(([actionId, option]) => (
<Button
key={op.index}
variant={op.variant}
onClick={() => handleAction(op.index)}
data-testid={`action-confirmation-modal-btn-${op.index}`}
isDanger={op.isDanger}
key={actionId}
variant={option.variant}
onClick={() => handleAction(actionId)}
data-testid={`action-confirmation-modal-btn-${actionId}`}
isDanger={option.isDanger}
>
{op.buttonText}
{option.buttonText}
</Button>
)),
<Button
key="cancel"
variant="link"
onClick={handleCloseModal}
data-testid={`action-confirmation-modal-btn-${ACTION_INDEX_CANCEL}`}
data-testid={`action-confirmation-modal-btn-${ACTION_ID_CANCEL}`}
>
Cancel
</Button>,
Expand Down
Original file line number Diff line number Diff line change
@@ -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(() => {
Expand All @@ -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 () => {
Expand All @@ -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', () => {
Expand Down Expand Up @@ -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,
},
]}
}}
/>
</ActionConfirmationModalContextProvider>,
);
Expand All @@ -98,38 +98,38 @@ 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(() => {
const deleteButton = wrapper.getByText('Delete');
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(() => {
const deleteButton = wrapper.getByText('Delete');
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');
});
});
});
Expand All @@ -138,7 +138,7 @@ interface TestComponentProps {
title: string;
text: string;
additionalModalText?: string;
buttonOptions?: ActionConfirmationButtonOption[];
buttonOptions?: Record<string, ActionConfirmationButtonOption>;
}

const TestComponent: FunctionComponent<TestComponentProps> = (props) => {
Expand Down
Loading