diff --git a/frontend/packages/console-shared/src/hooks/useWarningModal.tsx b/frontend/packages/console-shared/src/hooks/useWarningModal.tsx index 7e87c92821e..853d468a20e 100644 --- a/frontend/packages/console-shared/src/hooks/useWarningModal.tsx +++ b/frontend/packages/console-shared/src/hooks/useWarningModal.tsx @@ -1,23 +1,28 @@ -import type { FC } from 'react'; import { useState, useCallback } from 'react'; import type { WarningModalProps } from '@patternfly/react-component-groups'; import { WarningModal } from '@patternfly/react-component-groups'; +import type { OverlayComponent } from '@console/dynamic-plugin-sdk/src/app/modal-support/OverlayProvider'; import { useOverlay } from '@console/dynamic-plugin-sdk/src/app/modal-support/useOverlay'; /** * ControlledWarningModal is a wrapper around WarningModal that manages its open state. */ -const ControlledWarningModal: FC = (props) => { +const ControlledWarningModal: OverlayComponent = ({ + closeOverlay, + ...props +}) => { const [isOpen, setIsOpen] = useState(true); const onClose: WarningModalProps['onClose'] = (e) => { setIsOpen(false); props.onClose?.(e); + closeOverlay?.(); }; const onConfirm: WarningModalProps['onConfirm'] = () => { setIsOpen(false); props.onConfirm?.(); + closeOverlay?.(); }; return ; diff --git a/frontend/packages/console-shared/src/utils/__mocks__/error-modal-handler.tsx b/frontend/packages/console-shared/src/utils/__mocks__/error-modal-handler.tsx index 3c92f4646f8..a37ceb32119 100644 --- a/frontend/packages/console-shared/src/utils/__mocks__/error-modal-handler.tsx +++ b/frontend/packages/console-shared/src/utils/__mocks__/error-modal-handler.tsx @@ -1,11 +1,17 @@ /** * Mock implementation of error-modal-handler for Jest tests + * + * Note: This mock provides a simplified SyncModalLaunchers that doesn't call + * useSyncWarningModalLauncher. Tests that need warning modal functionality + * should explicitly mock warning-modal-handler or use the real implementation. */ export const mockLaunchErrorModal = jest.fn(); -export const SyncErrorModalLauncher = () => null; +export const useSyncErrorModalLauncher = jest.fn(); -export const useErrorModalLauncher = jest.fn(() => mockLaunchErrorModal); +// Simplified component that doesn't sync warning modals +// Tests needing both error and warning modals should not use this mock +export const SyncModalLaunchers = () => null; export const launchErrorModal = mockLaunchErrorModal; diff --git a/frontend/packages/console-shared/src/utils/__mocks__/warning-modal-handler.tsx b/frontend/packages/console-shared/src/utils/__mocks__/warning-modal-handler.tsx new file mode 100644 index 00000000000..6662d7cc69a --- /dev/null +++ b/frontend/packages/console-shared/src/utils/__mocks__/warning-modal-handler.tsx @@ -0,0 +1,13 @@ +/** + * Mock for warning-modal-handler + * Used in Jest tests to avoid rendering actual modals + */ + +export const mockLaunchWarningModal = jest.fn((props, onConfirm) => { + // Immediately call onConfirm by default to simulate user confirming + onConfirm?.(); +}); + +export const useSyncWarningModalLauncher = jest.fn(); + +export const launchWarningModal = mockLaunchWarningModal; diff --git a/frontend/packages/console-shared/src/utils/__tests__/error-modal-handler.spec.tsx b/frontend/packages/console-shared/src/utils/__tests__/error-modal-handler.spec.tsx index 25e452acdef..2fe92f0137c 100644 --- a/frontend/packages/console-shared/src/utils/__tests__/error-modal-handler.spec.tsx +++ b/frontend/packages/console-shared/src/utils/__tests__/error-modal-handler.spec.tsx @@ -1,9 +1,5 @@ import { render } from '@testing-library/react'; -import { - SyncErrorModalLauncher, - useErrorModalLauncher, - launchErrorModal, -} from '../error-modal-handler'; +import { SyncModalLaunchers, launchErrorModal } from '../error-modal-handler'; // Mock useOverlay const mockLauncher = jest.fn(); @@ -16,9 +12,9 @@ describe('error-modal-handler', () => { jest.clearAllMocks(); }); - describe('SyncErrorModalLauncher', () => { + describe('SyncModalLaunchers', () => { it('should sync the launcher on mount', () => { - render(); + render(); // Call the module-level function launchErrorModal({ error: 'Test error', title: 'Test' }); @@ -31,46 +27,20 @@ describe('error-modal-handler', () => { }); it('should cleanup launcher on unmount', () => { - const { unmount } = render(); + const { unmount } = render(); unmount(); - // Should log error instead of crashing - const consoleError = jest.spyOn(console, 'error').mockImplementation(); - launchErrorModal({ error: 'Test error' }); - - expect(consoleError).toHaveBeenCalledWith( - expect.stringContaining('Error modal launcher not initialized'), - expect.any(Object), - ); - - consoleError.mockRestore(); - }); - }); - - describe('useErrorModalLauncher', () => { - it('should return a function that launches error modals', () => { - let capturedLauncher: any; - - const TestComponent = () => { - capturedLauncher = useErrorModalLauncher(); - return null; - }; - - render(); - - capturedLauncher({ error: 'Test error', title: 'Test Title' }); - - expect(mockLauncher).toHaveBeenCalledWith(expect.anything(), { - error: 'Test error', - title: 'Test Title', - }); + // Should throw error when launcher is not initialized + expect(() => { + launchErrorModal({ error: 'Test error' }); + }).toThrow('Error modal launcher not initialized'); }); }); describe('launchErrorModal', () => { it('should launch error modal when launcher is initialized', () => { - render(); + render(); launchErrorModal({ error: 'Connection failed', @@ -83,17 +53,10 @@ describe('error-modal-handler', () => { }); }); - it('should log error when launcher is not initialized', () => { - const consoleError = jest.spyOn(console, 'error').mockImplementation(); - - launchErrorModal({ error: 'Test error' }); - - expect(consoleError).toHaveBeenCalledWith( - expect.stringContaining('Error modal launcher not initialized'), - { error: 'Test error' }, - ); - - consoleError.mockRestore(); + it('should throw error when launcher is not initialized', () => { + expect(() => { + launchErrorModal({ error: 'Test error' }); + }).toThrow('Error modal launcher not initialized'); }); }); }); diff --git a/frontend/packages/console-shared/src/utils/__tests__/warning-modal-handler.spec.tsx b/frontend/packages/console-shared/src/utils/__tests__/warning-modal-handler.spec.tsx new file mode 100644 index 00000000000..6f6fe0b9761 --- /dev/null +++ b/frontend/packages/console-shared/src/utils/__tests__/warning-modal-handler.spec.tsx @@ -0,0 +1,200 @@ +import { render } from '@testing-library/react'; +import { useSyncWarningModalLauncher, launchWarningModal } from '../warning-modal-handler'; + +// Mock useOverlay +const mockLauncher = jest.fn(); +jest.mock('@console/dynamic-plugin-sdk/src/app/modal-support/useOverlay', () => ({ + useOverlay: () => mockLauncher, +})); + +describe('warning-modal-handler', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + describe('useSyncWarningModalLauncher', () => { + it('should sync the launcher on mount', () => { + const TestComponent = () => { + useSyncWarningModalLauncher(); + return null; + }; + + render(); + + const onConfirm = jest.fn(); + const onCancel = jest.fn(); + + // Call the module-level function + launchWarningModal( + { + title: 'Test Warning', + children: 'Are you sure?', + confirmButtonLabel: 'Confirm', + }, + onConfirm, + onCancel, + ); + + // Should have called the mocked overlay launcher + expect(mockLauncher).toHaveBeenCalledWith(expect.anything(), { + title: 'Test Warning', + children: 'Are you sure?', + confirmButtonLabel: 'Confirm', + onConfirm: expect.any(Function), + onClose: expect.any(Function), + }); + }); + + it('should cleanup launcher on unmount', () => { + const TestComponent = () => { + useSyncWarningModalLauncher(); + return null; + }; + + const { unmount } = render(); + + unmount(); + + // Should throw error when launcher is not initialized + expect(() => { + launchWarningModal( + { + title: 'Test Warning', + children: 'Are you sure?', + }, + jest.fn(), + ); + }).toThrow('Warning modal launcher not initialized'); + }); + }); + + describe('launchWarningModal', () => { + it('should launch warning modal when launcher is initialized', () => { + const TestComponent = () => { + useSyncWarningModalLauncher(); + return null; + }; + + render(); + + const onConfirm = jest.fn(); + const onCancel = jest.fn(); + + launchWarningModal( + { + title: 'Delete Resource', + children: 'Are you sure you want to delete?', + confirmButtonLabel: 'Delete', + }, + onConfirm, + onCancel, + ); + + expect(mockLauncher).toHaveBeenCalledWith(expect.anything(), { + title: 'Delete Resource', + children: 'Are you sure you want to delete?', + confirmButtonLabel: 'Delete', + onConfirm: expect.any(Function), + onClose: expect.any(Function), + }); + }); + + it('should throw error when launcher is not initialized', () => { + expect(() => { + launchWarningModal( + { + title: 'Test', + children: 'Test message', + }, + jest.fn(), + ); + }).toThrow('Warning modal launcher not initialized'); + }); + + it('should call onConfirm callback when user confirms', () => { + const TestComponent = () => { + useSyncWarningModalLauncher(); + return null; + }; + + render(); + + const onConfirm = jest.fn(); + const onCancel = jest.fn(); + + launchWarningModal( + { + title: 'Confirm Action', + children: 'Proceed?', + }, + onConfirm, + onCancel, + ); + + // Get the onConfirm callback that was passed to the launcher + const launcherCall = mockLauncher.mock.calls[0]; + const launcherProps = launcherCall[1]; + launcherProps.onConfirm(); + + expect(onConfirm).toHaveBeenCalled(); + expect(onCancel).not.toHaveBeenCalled(); + }); + + it('should call onCancel callback when user cancels', () => { + const TestComponent = () => { + useSyncWarningModalLauncher(); + return null; + }; + + render(); + + const onConfirm = jest.fn(); + const onCancel = jest.fn(); + + launchWarningModal( + { + title: 'Confirm Action', + children: 'Proceed?', + }, + onConfirm, + onCancel, + ); + + // Get the onClose callback that was passed to the launcher + const launcherCall = mockLauncher.mock.calls[0]; + const launcherProps = launcherCall[1]; + launcherProps.onClose(); + + expect(onCancel).toHaveBeenCalled(); + expect(onConfirm).not.toHaveBeenCalled(); + }); + + it('should handle optional callbacks gracefully', () => { + const TestComponent = () => { + useSyncWarningModalLauncher(); + return null; + }; + + render(); + + // Call without callbacks + expect(() => { + launchWarningModal({ + title: 'Info', + children: 'Just showing info', + }); + }).not.toThrow(); + + expect(mockLauncher).toHaveBeenCalled(); + + // Verify calling the callbacks doesn't throw + const launcherCall = mockLauncher.mock.calls[0]; + const launcherProps = launcherCall[1]; + + expect(() => { + launcherProps.onConfirm(); + launcherProps.onClose(); + }).not.toThrow(); + }); + }); +}); diff --git a/frontend/packages/console-shared/src/utils/error-modal-handler.tsx b/frontend/packages/console-shared/src/utils/error-modal-handler.tsx index 7b52fdc6370..5a0218d4879 100644 --- a/frontend/packages/console-shared/src/utils/error-modal-handler.tsx +++ b/frontend/packages/console-shared/src/utils/error-modal-handler.tsx @@ -1,76 +1,54 @@ -import { useCallback, useEffect } from 'react'; -import { useOverlay } from '@console/dynamic-plugin-sdk/src/app/modal-support/useOverlay'; +import { useEffect } from 'react'; import type { ErrorModalProps } from '@console/internal/components/modals/error-modal'; -import { ErrorModal } from '@console/internal/components/modals/error-modal'; +import { useErrorModalLauncher } from '@console/internal/components/modals/error-modal'; +import { useSyncWarningModalLauncher } from './warning-modal-handler'; // Module-level reference for non-React contexts -// This is populated by SyncErrorModalLauncher and should not be set directly +// This is populated by useSyncErrorModalLauncher and should not be set directly let moduleErrorModalLauncher: ((props: ErrorModalProps) => void) | null = null; /** - * Component that syncs the error modal launcher to module-level for non-React contexts. - * This should be mounted once in the app root, after OverlayProvider. - * - * @example - * ```tsx - * const App = () => ( - * - * - * - * - * ); - * ``` + * Hook that syncs the error modal launcher to module-level for non-React contexts. + * Use SyncModalLaunchers component instead of calling this directly. */ -export const SyncErrorModalLauncher = () => { - const launcher = useOverlay(); +export const useSyncErrorModalLauncher = () => { + const errorModalLauncher = useErrorModalLauncher(); useEffect(() => { - moduleErrorModalLauncher = (props: ErrorModalProps) => { - launcher(ErrorModal, props); - }; + moduleErrorModalLauncher = errorModalLauncher; return () => { - moduleErrorModalLauncher = null; + // Only clear if we're still the active launcher + if (moduleErrorModalLauncher === errorModalLauncher) { + moduleErrorModalLauncher = null; + } }; - }, [launcher]); - - return null; + }, [errorModalLauncher]); }; /** - * Hook to launch error modals from React components. - * Must be used within an OverlayProvider. + * Component that syncs both error and warning modal launchers to module-level for non-React contexts. + * This should be mounted once in the app root, after OverlayProvider. * * @example * ```tsx - * const MyComponent = () => { - * const launchErrorModal = useErrorModalLauncher(); - * - * const handleError = (error: Error) => { - * launchErrorModal({ - * title: 'Operation Failed', - * error: error.message, - * }); - * }; - * - * // ... - * }; + * const App = () => ( + * + * + * + * + * ); * ``` */ -export const useErrorModalLauncher = (): ((props: ErrorModalProps) => void) => { - const launcher = useOverlay(); - - return useCallback( - (props: ErrorModalProps) => { - launcher(ErrorModal, props); - }, - [launcher], - ); +export const SyncModalLaunchers = () => { + useSyncErrorModalLauncher(); + useSyncWarningModalLauncher(); + return null; }; /** * Launch an error modal from non-React contexts (callbacks, promises, utilities). - * The SyncErrorModalLauncher component must be mounted in the app root. + * The SyncModalLaunchers component must be mounted in the app root. * * @deprecated Use React component modals within component code instead. * For new code, write modals directly within React components using useOverlay or other modal patterns. @@ -91,10 +69,8 @@ export const launchErrorModal = (props: ErrorModalProps): void => { if (moduleErrorModalLauncher) { moduleErrorModalLauncher(props); } else { - // eslint-disable-next-line no-console - console.error( - 'Error modal launcher not initialized. Ensure SyncErrorModalLauncher is mounted after OverlayProvider.', - props, + throw new Error( + 'Error modal launcher not initialized. Ensure SyncModalLaunchers is mounted after OverlayProvider.', ); } }; diff --git a/frontend/packages/console-shared/src/utils/warning-modal-handler.tsx b/frontend/packages/console-shared/src/utils/warning-modal-handler.tsx new file mode 100644 index 00000000000..35dab142c7e --- /dev/null +++ b/frontend/packages/console-shared/src/utils/warning-modal-handler.tsx @@ -0,0 +1,81 @@ +import { useEffect } from 'react'; +import type { ControlledWarningModalProps } from '../hooks/useWarningModal'; +import { useWarningModal } from '../hooks/useWarningModal'; + +// Module-level reference for non-React contexts +// This is populated by useSyncWarningModalLauncher and should not be set directly +let moduleWarningModalLauncher: ((props: ControlledWarningModalProps) => void) | null = null; + +/** + * Hook that syncs the warning modal launcher to module-level for non-React contexts. + * This should be called once in the app root, after OverlayProvider. + * Use SyncModalLaunchers component from error-modal-handler instead of calling this directly. + */ +export const useSyncWarningModalLauncher = () => { + const warningModalLauncher = useWarningModal(); + + useEffect(() => { + moduleWarningModalLauncher = warningModalLauncher; + + return () => { + // Only clear if we're still the active launcher + if (moduleWarningModalLauncher === warningModalLauncher) { + moduleWarningModalLauncher = null; + } + }; + }, [warningModalLauncher]); +}; + +/** + * Launch a warning modal from non-React contexts (callbacks, promises, utilities). + * The SyncModalLaunchers component must be mounted in the app root. + * + * @deprecated Use React component modals within component code instead. + * For new code, write modals directly within React components using useWarningModal. + * This function should only be used for legacy non-React contexts like promise callbacks. + * + * @param props - Warning modal properties (title, children, confirmButtonLabel, etc.) + * @param onConfirm - Optional callback invoked when user confirms the action + * @param onCancel - Optional callback invoked when user cancels or closes the modal + * + * @example + * ```tsx + * // In a utility function or non-React context + * launchWarningModal( + * { + * title: 'Delete Resource', + * children: 'Are you sure you want to delete this resource?', + * confirmButtonLabel: 'Delete', + * }, + * () => { + * // User confirmed - proceed with action + * deleteResource(); + * }, + * () => { + * // User canceled + * console.log('Action cancelled by user'); + * } + * ); + * ``` + */ +export const launchWarningModal = ( + props: Omit, + onConfirm?: () => void, + onCancel?: () => void, +): void => { + if (moduleWarningModalLauncher) { + moduleWarningModalLauncher({ + ...props, + onConfirm: () => { + onConfirm?.(); + }, + onClose: () => { + onCancel?.(); + }, + }); + } else { + throw new Error( + 'Warning modal launcher not initialized. Ensure SyncModalLaunchers is mounted after OverlayProvider.', + ); + } +}; diff --git a/frontend/packages/metal3-plugin/src/components/baremetal-hosts/host-actions-provider.ts b/frontend/packages/metal3-plugin/src/components/baremetal-hosts/host-actions-provider.ts index 163db208b7e..fd74f206f31 100644 --- a/frontend/packages/metal3-plugin/src/components/baremetal-hosts/host-actions-provider.ts +++ b/frontend/packages/metal3-plugin/src/components/baremetal-hosts/host-actions-provider.ts @@ -125,18 +125,20 @@ export const useRemoveNodeMaintenanceAction = ( ) => { const { t } = useTranslation(); const hidden = !nodeName || !hasNodeMaintenanceCapability || !nodeMaintenance; - const stopNodeMaintenanceModalLauncher = useStopNodeMaintenanceModal(nodeMaintenance); + const stopNodeMaintenanceModalLauncher = useStopNodeMaintenanceModal(); const factory = useMemo( () => ({ removeNodeMaintenance: () => ({ id: 'remove-node-maintenance', label: t('metal3-plugin~Stop Maintenance'), - cta: stopNodeMaintenanceModalLauncher, + cta: () => stopNodeMaintenanceModalLauncher(nodeMaintenance), accessReview: host && asAccessReview(maintenanceModel, host, 'delete'), }), }), + // Missing stopNodeMaintenanceModalLauncher dependency - intentionally excluded to prevent + // infinite re-renders when used in action hooks. // eslint-disable-next-line react-hooks/exhaustive-deps - [host, t], + [host, t, nodeMaintenance], ); const action = useMemo(() => (!hidden ? [factory.removeNodeMaintenance()] : []), [ factory, diff --git a/frontend/packages/metal3-plugin/src/components/maintenance/StartingMaintenancePopoverContent.tsx b/frontend/packages/metal3-plugin/src/components/maintenance/StartingMaintenancePopoverContent.tsx index 75580611700..ca49ea582fd 100644 --- a/frontend/packages/metal3-plugin/src/components/maintenance/StartingMaintenancePopoverContent.tsx +++ b/frontend/packages/metal3-plugin/src/components/maintenance/StartingMaintenancePopoverContent.tsx @@ -20,7 +20,7 @@ import { getNodeMaintenanceLastError, getNodeMaintenancePendingPods, } from '../../selectors'; -import stopNodeMaintenanceModal from '../modals/StopNodeMaintenanceModal'; +import { useStopNodeMaintenanceModal } from '../modals/StopNodeMaintenanceModal'; import MaintenancePopoverPodList from './MaintenancePopoverPodList'; type StartingMaintenancePopoverContentProps = { @@ -31,6 +31,7 @@ const StartingMaintenancePopoverContent: FC { const { t } = useTranslation(); + const stopNodeMaintenanceModalLauncher = useStopNodeMaintenanceModal(); const reason = getNodeMaintenanceReason(nodeMaintenance); const creationTimestamp = getNodeMaintenanceCreationTimestamp(nodeMaintenance); const lastError = getNodeMaintenanceLastError(nodeMaintenance); @@ -78,7 +79,11 @@ const StartingMaintenancePopoverContent: FC
- diff --git a/frontend/packages/metal3-plugin/src/components/maintenance/UnderMaintenancePopoverContent.tsx b/frontend/packages/metal3-plugin/src/components/maintenance/UnderMaintenancePopoverContent.tsx index c19b66070d4..8d178084668 100644 --- a/frontend/packages/metal3-plugin/src/components/maintenance/UnderMaintenancePopoverContent.tsx +++ b/frontend/packages/metal3-plugin/src/components/maintenance/UnderMaintenancePopoverContent.tsx @@ -10,7 +10,7 @@ import { useTranslation } from 'react-i18next'; import type { K8sResourceKind } from '@console/internal/module/k8s'; import { Timestamp } from '@console/shared/src/components/datetime/Timestamp'; import { getNodeMaintenanceReason, getNodeMaintenanceCreationTimestamp } from '../../selectors'; -import stopNodeMaintenanceModal from '../modals/StopNodeMaintenanceModal'; +import { useStopNodeMaintenanceModal } from '../modals/StopNodeMaintenanceModal'; type UnderMaintenancePopoverContentProps = { nodeMaintenance: K8sResourceKind; @@ -20,6 +20,7 @@ const UnderMaintenancePopoverContent: FC = nodeMaintenance, }) => { const { t } = useTranslation(); + const stopNodeMaintenanceModalLauncher = useStopNodeMaintenanceModal(); const reason = getNodeMaintenanceReason(nodeMaintenance); const creationTimestamp = getNodeMaintenanceCreationTimestamp(nodeMaintenance); @@ -43,7 +44,11 @@ const UnderMaintenancePopoverContent: FC =
- diff --git a/frontend/packages/metal3-plugin/src/components/maintenance/actions.tsx b/frontend/packages/metal3-plugin/src/components/maintenance/actions.tsx index 9e4ff59176d..b19f8139cbc 100644 --- a/frontend/packages/metal3-plugin/src/components/maintenance/actions.tsx +++ b/frontend/packages/metal3-plugin/src/components/maintenance/actions.tsx @@ -6,7 +6,7 @@ import type { NodeKind } from '@console/internal/module/k8s'; import { useMaintenanceCapability } from '../../hooks/useMaintenanceCapability'; import { findNodeMaintenance } from '../../selectors'; import { useStartNodeMaintenanceModalLauncher } from '../modals/StartNodeMaintenanceModal'; -import stopNodeMaintenanceModal from '../modals/StopNodeMaintenanceModal'; +import { useStopNodeMaintenanceModal } from '../modals/StopNodeMaintenanceModal'; export const useNodeMaintenanceActions: ExtensionHook = (resource) => { const { t } = useTranslation(); @@ -24,6 +24,8 @@ export const useNodeMaintenanceActions: ExtensionHook = (res namespaced: false, }); + const stopNodeMaintenanceModalLauncher = useStopNodeMaintenanceModal(); + const actions = useMemo(() => { const nodeMaintenance = findNodeMaintenance(maintenances, resource.metadata.name); @@ -38,11 +40,13 @@ export const useNodeMaintenanceActions: ExtensionHook = (res action = { id: 'stop-node-maintenance', label: t('metal3-plugin~Stop Maintenance'), - cta: () => stopNodeMaintenanceModal(nodeMaintenance, t), + cta: () => stopNodeMaintenanceModalLauncher(nodeMaintenance), insertBefore: 'edit-labels', }; } return [action]; + // missing startNodeMaintenanceModal and stopNodeMaintenanceModalLauncher dependencies, + // that causes max depth exceeded error // eslint-disable-next-line react-hooks/exhaustive-deps }, [maintenances, resource.metadata.name, t]); diff --git a/frontend/packages/metal3-plugin/src/components/modals/StopNodeMaintenanceModal.tsx b/frontend/packages/metal3-plugin/src/components/modals/StopNodeMaintenanceModal.tsx index 28d3421b346..f5503248075 100644 --- a/frontend/packages/metal3-plugin/src/components/modals/StopNodeMaintenanceModal.tsx +++ b/frontend/packages/metal3-plugin/src/components/modals/StopNodeMaintenanceModal.tsx @@ -1,6 +1,5 @@ -import type { TFunction } from 'i18next'; +import { useCallback } from 'react'; import { Trans, useTranslation } from 'react-i18next'; -import { confirmModal } from '@console/internal/components/modals/confirm-modal'; import type { K8sResourceKind } from '@console/internal/module/k8s'; import { k8sKill } from '@console/internal/module/k8s'; import { useWarningModal } from '@console/shared/src/hooks/useWarningModal'; @@ -27,40 +26,32 @@ const getMaintenanceModel = (nodeMaintenance: K8sResourceKind) => { return NodeMaintenanceKubevirtAlphaModel; }; -const stopNodeMaintenanceModal = (nodeMaintenance: K8sResourceKind, t: TFunction) => { - const reason = getNodeMaintenanceReason(nodeMaintenance); - const reasonLabel = reason ? `(${reason})` : ''; - const nodeName = getNodeMaintenanceNodeName(nodeMaintenance); - return confirmModal({ - title: t('metal3-plugin~Stop maintenance'), - message: ( - - Are you sure you want to stop maintenance {reasonLabel} on node{' '} - {nodeName}? - - ), - btnText: t('metal3-plugin~Stop maintenance'), - executeFn: () => k8sKill(getMaintenanceModel(nodeMaintenance), nodeMaintenance), - }); -}; - -export const useStopNodeMaintenanceModal = (nodeMaintenance: K8sResourceKind) => { +export const useStopNodeMaintenanceModal = () => { const { t } = useTranslation(); - const reason = getNodeMaintenanceReason(nodeMaintenance); - const reasonLabel = reason ? `(${reason})` : ''; - const nodeName = getNodeMaintenanceNodeName(nodeMaintenance); - const stopNodeMaintenanceModalLauncher = useWarningModal({ - title: t('metal3-plugin~Stop maintenance'), - children: ( - - Are you sure you want to stop maintenance {reasonLabel} on node{' '} - {nodeName}? - - ), - confirmButtonLabel: t('metal3-plugin~Stop maintenance'), - onConfirm: () => k8sKill(getMaintenanceModel(nodeMaintenance), nodeMaintenance), - }); - return stopNodeMaintenanceModalLauncher; -}; + const launchWarningModal = useWarningModal(); -export default stopNodeMaintenanceModal; + return useCallback( + (nodeMaintenance: K8sResourceKind) => { + const reason = getNodeMaintenanceReason(nodeMaintenance); + const reasonLabel = reason ? `(${reason})` : ''; + const nodeName = getNodeMaintenanceNodeName(nodeMaintenance); + + launchWarningModal({ + title: t('metal3-plugin~Stop maintenance'), + children: ( + + Are you sure you want to stop maintenance {reasonLabel} on node{' '} + {nodeName}? + + ), + confirmButtonLabel: t('metal3-plugin~Stop maintenance'), + onConfirm: () => k8sKill(getMaintenanceModel(nodeMaintenance), nodeMaintenance), + }); + }, + // Missing launchWarningModal dependency - intentionally excluded to prevent infinite re-renders. + // The modal launcher function reference changes when its props change, but we capture + // nodeMaintenance at call time via closure, so the stale reference is safe here. + // eslint-disable-next-line react-hooks/exhaustive-deps + [t], + ); +}; diff --git a/frontend/packages/shipwright-plugin/locales/en/shipwright-plugin.json b/frontend/packages/shipwright-plugin/locales/en/shipwright-plugin.json index e3362388c58..269033f39c5 100644 --- a/frontend/packages/shipwright-plugin/locales/en/shipwright-plugin.json +++ b/frontend/packages/shipwright-plugin/locales/en/shipwright-plugin.json @@ -96,7 +96,6 @@ "Started": "Started", "Unknown failure condition": "Unknown failure condition", "Failure on task {{taskName}} - check logs for details.": "Failure on task {{taskName}} - check logs for details.", - "Error downloading logs.": "Error downloading logs.", "An error occurred while retrieving the requested logs.": "An error occurred while retrieving the requested logs.", "Unknown error retrieving logs": "Unknown error retrieving logs", "Download": "Download", diff --git a/frontend/packages/shipwright-plugin/src/components/logs/logs-utils.ts b/frontend/packages/shipwright-plugin/src/components/logs/logs-utils.ts index fdbc4806e13..4bb21513235 100644 --- a/frontend/packages/shipwright-plugin/src/components/logs/logs-utils.ts +++ b/frontend/packages/shipwright-plugin/src/components/logs/logs-utils.ts @@ -1,21 +1,12 @@ -import { saveAs } from 'file-saver'; -import i18next from 'i18next'; import * as _ from 'lodash'; -import { coFetchText } from '@console/internal/co-fetch'; import { LOG_SOURCE_RESTARTING, LOG_SOURCE_RUNNING, LOG_SOURCE_TERMINATED, LOG_SOURCE_WAITING, - LineBuffer, } from '@console/internal/components/utils'; -import { PodModel } from '@console/internal/models'; -import type { PodKind, ContainerSpec, ContainerStatus } from '@console/internal/module/k8s'; -import { resourceURL, k8sGet } from '@console/internal/module/k8s'; -import { launchErrorModal } from '@console/shared/src/utils/error-modal-handler'; -import type { TaskRunKind } from '../../types'; +import type { ContainerSpec, ContainerStatus, PodKind } from '@console/internal/module/k8s'; import { ComputedStatus, SucceedConditionReason } from './log-snippet-types'; -import { getTaskRunLog } from './tekton-results'; export const containerToLogSourceStatus = (container: ContainerStatus): string => { if (!container) { @@ -67,131 +58,6 @@ export const getRenderContainers = ( }; }; -const getOrderedStepsFromPod = (name: string, ns: string): Promise => { - return k8sGet(PodModel, name, ns) - .then((pod: PodKind) => { - return getSortedContainerStatus( - pod.spec.containers ?? [], - pod.status?.containerStatuses ?? [], - ); - }) - .catch((err) => { - launchErrorModal({ - error: err.message || i18next.t('shipwright-plugin~Error downloading logs.'), - }); - return []; - }); -}; - -type StepsWatchUrl = { - [key: string]: { - name: string; - steps: { [step: string]: WatchURLStatus }; - taskRunPath: string; - }; -}; - -type WatchURLStatus = { - status: string; - url: string; -}; - -export const getDownloadAllLogsCallback = ( - sortedTaskRunNames: string[], - taskRuns: TaskRunKind[], - namespace: string, - pipelineRunName: string, -): (() => Promise) => { - const getWatchUrls = async (): Promise => { - const stepsList: ContainerStatus[][] = await Promise.all( - sortedTaskRunNames.map((currTask) => { - const { status } = taskRuns.find((t) => t.metadata.name === currTask) ?? {}; - return getOrderedStepsFromPod(status?.podName, namespace); - }), - ); - return sortedTaskRunNames.reduce((acc, currTask, i) => { - const taskRun = taskRuns.find((t) => t.metadata.name === currTask); - const pipelineTaskName = taskRun?.spec.taskRef?.name ?? taskRun?.metadata.name; - const { status } = taskRun; - const podName = status?.podName; - const steps = stepsList[i]; - const allStepUrls = steps.reduce((stepUrls, currentStep) => { - const { name } = currentStep; - const currentStatus = containerToLogSourceStatus(currentStep); - if (currentStatus === LOG_SOURCE_WAITING) return stepUrls; - const urlOpts = { - ns: namespace, - name: podName, - path: 'log', - queryParams: { - container: name, - follow: 'true', - }, - }; - return { - ...stepUrls, - [name]: { - status: currentStatus, - url: resourceURL(PodModel, urlOpts), - } as WatchURLStatus, - }; - }, {}); - acc[currTask] = { - name: pipelineTaskName, - steps: { ...allStepUrls }, - taskRunPath: taskRun.metadata?.annotations?.['results.tekton.dev/record'], - }; - return acc; - }, {}); - }; - - const fetchLogs = async (tasksPromise: Promise) => { - const tasks = await tasksPromise; - let allLogs = ''; - for (const currTask of sortedTaskRunNames) { - const task = tasks[currTask]; - const steps = Object.keys(task.steps); - allLogs += `${task.name}\n\n`; - if (steps.length > 0) { - for (const step of steps) { - const { url, status } = task.steps[step]; - const getContentPromise = coFetchText(url).then((logs) => { - return `${step.toUpperCase()}\n\n${logs}\n\n`; - }); - allLogs += - status === LOG_SOURCE_TERMINATED - ? // If we are done, we want this log content - // eslint-disable-next-line no-await-in-loop - await getContentPromise - : // If we are not done, let's not wait indefinitely - // eslint-disable-next-line no-await-in-loop - await Promise.race([ - getContentPromise, - new Promise((resolve) => { - setTimeout(() => resolve(''), 1000); - }), - ]); - } - } else { - // eslint-disable-next-line no-await-in-loop - allLogs += await getTaskRunLog(task.taskRunPath).then( - (log) => `${tasks[currTask].name.toUpperCase()}\n\n${log}\n\n`, - ); - } - } - const buffer = new LineBuffer(null); - buffer.ingest(allLogs); - const blob = buffer.getBlob({ - type: 'text/plain;charset=utf-8', - }); - saveAs(blob, `${pipelineRunName}.log`); - return null; - }; - return (): Promise => { - return fetchLogs(getWatchUrls()); - }; -}; - export const pipelineRunStatus = (pipelineRun): ComputedStatus => { const conditions = _.get(pipelineRun, ['status', 'conditions'], []); if (conditions.length === 0) return null; diff --git a/frontend/packages/topology/src/utils/moveNodeToGroup.tsx b/frontend/packages/topology/src/utils/moveNodeToGroup.tsx index ac238aa1a7b..657fb882482 100644 --- a/frontend/packages/topology/src/utils/moveNodeToGroup.tsx +++ b/frontend/packages/topology/src/utils/moveNodeToGroup.tsx @@ -1,7 +1,8 @@ import type { Node } from '@patternfly/react-topology'; +import i18next from 'i18next'; import { Trans } from 'react-i18next'; -import { confirmModal } from '@console/internal/components/modals'; import { launchErrorModal } from '@console/shared/src/utils/error-modal-handler'; +import { launchWarningModal } from '@console/shared/src/utils/warning-modal-handler'; import { updateTopologyResourceApplication } from './topology-utils'; export const moveNodeToGroup = ( @@ -37,23 +38,28 @@ export const moveNodeToGroup = ( // t('topology~Remove') const btnTextKey = targetGroup ? 'topology~Move' : 'topology~Remove'; - return new Promise((resolve, reject) => { - confirmModal({ - titleKey, - message, - btnTextKey, - close: () => { - reject(); - }, - cancel: () => { - reject(); + // Blur active element to prevent aria-hidden focus violations when modal opens + // This fixes the browser warning: "Blocked aria-hidden on an element because its + // descendant retained focus" when focus remains on SVG elements during drag-drop + if ( + document.activeElement instanceof HTMLElement || + document.activeElement instanceof SVGElement + ) { + document.activeElement.blur(); + } + + return new Promise((resolve, reject) => { + launchWarningModal( + { + title: i18next.t(titleKey), + children: message, + confirmButtonLabel: i18next.t(btnTextKey), + titleIconVariant: null, }, - executeFn: () => { - return updateTopologyResourceApplication( - node, - targetGroup ? targetGroup.getLabel() : null, - ) - .then(resolve) + () => { + // User confirmed - proceed with move/remove + updateTopologyResourceApplication(node, targetGroup ? targetGroup.getLabel() : null) + .then(() => resolve()) .catch((err) => { const error = err.message; if (onError) { @@ -64,7 +70,11 @@ export const moveNodeToGroup = ( reject(err); }); }, - }); + () => { + // User cancelled - reject to signal operation was cancelled + reject(new Error('User cancelled')); + }, + ); }); } diff --git a/frontend/public/components/app.tsx b/frontend/public/components/app.tsx index 31dcac1b84d..18a7f8a4365 100644 --- a/frontend/public/components/app.tsx +++ b/frontend/public/components/app.tsx @@ -53,7 +53,7 @@ import { QuickStartDrawer } from '@console/app/src/components/quick-starts/Quick import { ModalProvider } from '@console/dynamic-plugin-sdk/src/app/modal-support/ModalProvider'; import { OverlayProvider } from '@console/dynamic-plugin-sdk/src/app/modal-support/OverlayProvider'; import ToastProvider from '@console/shared/src/components/toast/ToastProvider'; -import { SyncErrorModalLauncher } from '@console/shared/src/utils/error-modal-handler'; +import { SyncModalLaunchers } from '@console/shared/src/utils/error-modal-handler'; import { useTelemetry } from '@console/shared/src/hooks/useTelemetry'; import { useDebounceCallback } from '@console/shared/src/hooks/debounce'; import { LOGIN_ERROR_PATH } from '@console/internal/module/auth'; @@ -319,7 +319,7 @@ const App: FC<{ - + }> {contextProviderExtensions.reduce( (children, e) => ( diff --git a/frontend/public/components/modals/confirm-modal.tsx b/frontend/public/components/modals/confirm-modal.tsx deleted file mode 100644 index 5601758848f..00000000000 --- a/frontend/public/components/modals/confirm-modal.tsx +++ /dev/null @@ -1,72 +0,0 @@ -import { Translation } from 'react-i18next'; -import { usePromiseHandler } from '@console/shared/src/hooks/promise-handler'; -import type { ReactNode, FormEvent, FC } from 'react'; -import { - createModalLauncher, - ModalTitle, - ModalBody, - ModalSubmitFooter, - ModalComponentProps, -} from '../factory/modal'; - -interface ConfirmModalProps extends ModalComponentProps { - btnText?: string | ReactNode; - btnTextKey?: string; - cancelText?: string | ReactNode; - cancelTextKey?: string; - executeFn: (e?: FormEvent, opts?: { supressNotifications: boolean }) => Promise; - message?: string | ReactNode; - messageKey?: string; - title?: string | ReactNode; - titleKey?: string; - submitDanger?: boolean; -} - -const ConfirmModal: FC = (props) => { - const [handlePromise, inProgress, errorMessage] = usePromiseHandler(); - - const submit = (event: FormEvent) => { - event.preventDefault(); - - handlePromise( - props.executeFn(null, { - supressNotifications: true, - }), - ).then(props.close); - }; - - const { - title, - titleKey, - message, - messageKey, - btnText, - btnTextKey, - cancelText, - cancelTextKey, - submitDanger, - cancel, - } = props; - - return ( - - {(t) => ( -
- {titleKey ? t(titleKey) : title} - {messageKey ? t(messageKey) : message} - - - )} -
- ); -}; - -/** @deprecated use `useWarningModal` instead */ -export const confirmModal = createModalLauncher(ConfirmModal); diff --git a/frontend/public/components/modals/error-modal.tsx b/frontend/public/components/modals/error-modal.tsx index 6ef082221c3..fb10bf1d49a 100644 --- a/frontend/public/components/modals/error-modal.tsx +++ b/frontend/public/components/modals/error-modal.tsx @@ -53,9 +53,21 @@ export const ErrorModal: OverlayComponent = (props) => { ); }; -export const useErrorModalLauncher = (props) => { +export const useErrorModalLauncher = ( + props?: Partial, +): ((overrides?: ErrorModalProps) => void) => { const launcher = useOverlay(); - return useCallback(() => launcher(ErrorModal, props), [launcher, props]); + return useCallback( + (overrides?: ErrorModalProps) => { + const mergedProps: ErrorModalProps = { + error: '', + ...(props || {}), + ...(overrides || {}), + }; + launcher(ErrorModal, mergedProps); + }, + [launcher, props], + ); }; export type ErrorModalProps = { diff --git a/frontend/public/components/modals/index.ts b/frontend/public/components/modals/index.ts index 801c5599513..4cf88b5cb79 100644 --- a/frontend/public/components/modals/index.ts +++ b/frontend/public/components/modals/index.ts @@ -11,12 +11,6 @@ export const configureJobParallelismModal = (props) => m.configureJobParallelismModal(props), ); -/** @deprecated use `useWarningModal` instead */ -export const confirmModal = (props) => - import('./confirm-modal' /* webpackChunkName: "confirm-modal" */).then((m) => - m.confirmModal(props), - ); - // Lazy-loaded OverlayComponent for Configure Namespace Pull Secret Modal export const LazyConfigureNamespacePullSecretModalOverlay = lazy(() => import( diff --git a/frontend/public/locales/en/public.json b/frontend/public/locales/en/public.json index 2884855fcee..3ce74d9ebe4 100644 --- a/frontend/public/locales/en/public.json +++ b/frontend/public/locales/en/public.json @@ -911,7 +911,6 @@ "Recreate": "Recreate", "Shut down all existing pods before creating new ones": "Shut down all existing pods before creating new ones", "Edit update strategy": "Edit update strategy", - "Confirm": "Confirm", "Delete {{kind}}?": "Delete {{kind}}?", "Are you sure you want to delete <2>{{resourceName}} in namespace <5>{{namespace}}?": "Are you sure you want to delete <2>{{resourceName}} in namespace <5>{{namespace}}?", "Are you sure you want to delete <2>{{resourceName}}?": "Are you sure you want to delete <2>{{resourceName}}?",