Skip to content

Commit 274b735

Browse files
rhamiltoclaude
andcommitted
CONSOLE-5012: Migrate confirmModal to overlay pattern
This commit migrates all instances of confirmModal to use the modern overlay pattern with useWarningModal, completing the modal migration effort started in previous PRs. Changes: - Removed deprecated confirmModal launcher and confirm-modal.tsx file - Migrated topology moveNodeToGroup to use useWarningModal with proper cancel handling via global handler pattern - Updated StopNodeMaintenanceModal to remove deprecated imperative API and keep only the useStopNodeMaintenanceModal hook - Fixed useWarningModal to properly call closeOverlay() and filter it from props spread to prevent React DOM warnings - Added try-catch in topology drag-drop to prevent optimistic update when user cancels the move operation - Updated all metal3-plugin maintenance components to use the hook-based approach This PR builds upon and depends on: - openshift#15932 (ResourceLimitsModal migration) - openshift#15939 (DeletePDBModal migration) - openshift#15940 (ConfigureUnschedulableModal migration) - openshift#15946 (Global error modal utility) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent 6f9a46a commit 274b735

11 files changed

Lines changed: 138 additions & 159 deletions

File tree

frontend/packages/console-shared/src/hooks/useWarningModal.tsx

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,17 +6,22 @@ import { useOverlay } from '@console/dynamic-plugin-sdk/src/app/modal-support/us
66
/**
77
* ControlledWarningModal is a wrapper around WarningModal that manages its open state.
88
*/
9-
const ControlledWarningModal: FC<WarningModalProps> = (props) => {
9+
const ControlledWarningModal: FC<WarningModalProps & { closeOverlay?: () => void }> = ({
10+
closeOverlay,
11+
...props
12+
}) => {
1013
const [isOpen, setIsOpen] = useState(true);
1114

1215
const onClose: WarningModalProps['onClose'] = (e) => {
1316
setIsOpen(false);
1417
props.onClose?.(e);
18+
closeOverlay?.();
1519
};
1620

1721
const onConfirm: WarningModalProps['onConfirm'] = () => {
1822
setIsOpen(false);
1923
props.onConfirm?.();
24+
closeOverlay?.();
2025
};
2126

2227
return <WarningModal {...props} isOpen={isOpen} onClose={onClose} onConfirm={onConfirm} />;

frontend/packages/metal3-plugin/src/components/maintenance/StartingMaintenancePopoverContent.tsx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import {
2020
getNodeMaintenanceLastError,
2121
getNodeMaintenancePendingPods,
2222
} from '../../selectors';
23-
import stopNodeMaintenanceModal from '../modals/StopNodeMaintenanceModal';
23+
import { useStopNodeMaintenanceModal } from '../modals/StopNodeMaintenanceModal';
2424
import MaintenancePopoverPodList from './MaintenancePopoverPodList';
2525

2626
type StartingMaintenancePopoverContentProps = {
@@ -31,6 +31,7 @@ const StartingMaintenancePopoverContent: FC<StartingMaintenancePopoverContentPro
3131
nodeMaintenance,
3232
}) => {
3333
const { t } = useTranslation();
34+
const stopNodeMaintenanceModalLauncher = useStopNodeMaintenanceModal(nodeMaintenance);
3435
const reason = getNodeMaintenanceReason(nodeMaintenance);
3536
const creationTimestamp = getNodeMaintenanceCreationTimestamp(nodeMaintenance);
3637
const lastError = getNodeMaintenanceLastError(nodeMaintenance);
@@ -78,7 +79,7 @@ const StartingMaintenancePopoverContent: FC<StartingMaintenancePopoverContentPro
7879
<MaintenancePopoverPodList pods={pendingPods} />
7980
</ExpandableSection>
8081
<br />
81-
<Button variant="link" onClick={() => stopNodeMaintenanceModal(nodeMaintenance, t)} isInline>
82+
<Button variant="link" onClick={() => stopNodeMaintenanceModalLauncher()} isInline>
8283
{t('metal3-plugin~Stop maintenance')}
8384
</Button>
8485
</>

frontend/packages/metal3-plugin/src/components/maintenance/UnderMaintenancePopoverContent.tsx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { useTranslation } from 'react-i18next';
1010
import { K8sResourceKind } from '@console/internal/module/k8s';
1111
import { Timestamp } from '@console/shared/src/components/datetime/Timestamp';
1212
import { getNodeMaintenanceReason, getNodeMaintenanceCreationTimestamp } from '../../selectors';
13-
import stopNodeMaintenanceModal from '../modals/StopNodeMaintenanceModal';
13+
import { useStopNodeMaintenanceModal } from '../modals/StopNodeMaintenanceModal';
1414

1515
type UnderMaintenancePopoverContentProps = {
1616
nodeMaintenance: K8sResourceKind;
@@ -20,6 +20,7 @@ const UnderMaintenancePopoverContent: FC<UnderMaintenancePopoverContentProps> =
2020
nodeMaintenance,
2121
}) => {
2222
const { t } = useTranslation();
23+
const stopNodeMaintenanceModalLauncher = useStopNodeMaintenanceModal(nodeMaintenance);
2324
const reason = getNodeMaintenanceReason(nodeMaintenance);
2425
const creationTimestamp = getNodeMaintenanceCreationTimestamp(nodeMaintenance);
2526

@@ -43,7 +44,7 @@ const UnderMaintenancePopoverContent: FC<UnderMaintenancePopoverContentProps> =
4344
</DescriptionListGroup>
4445
</DescriptionList>
4546
<br />
46-
<Button variant="link" onClick={() => stopNodeMaintenanceModal(nodeMaintenance, t)} isInline>
47+
<Button variant="link" onClick={() => stopNodeMaintenanceModalLauncher()} isInline>
4748
{t('metal3-plugin~Stop maintenance')}
4849
</Button>
4950
</>

frontend/packages/metal3-plugin/src/components/maintenance/actions.tsx

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { NodeKind } from '@console/internal/module/k8s';
66
import { useMaintenanceCapability } from '../../hooks/useMaintenanceCapability';
77
import { findNodeMaintenance } from '../../selectors';
88
import { useStartNodeMaintenanceModalLauncher } from '../modals/StartNodeMaintenanceModal';
9-
import stopNodeMaintenanceModal from '../modals/StopNodeMaintenanceModal';
9+
import { useStopNodeMaintenanceModal } from '../modals/StopNodeMaintenanceModal';
1010

1111
export const useNodeMaintenanceActions: ExtensionHook<Action[], NodeKind> = (resource) => {
1212
const { t } = useTranslation();
@@ -24,9 +24,10 @@ export const useNodeMaintenanceActions: ExtensionHook<Action[], NodeKind> = (res
2424
namespaced: false,
2525
});
2626

27-
const actions = useMemo(() => {
28-
const nodeMaintenance = findNodeMaintenance(maintenances, resource.metadata.name);
27+
const nodeMaintenance = findNodeMaintenance(maintenances, resource.metadata.name);
28+
const stopNodeMaintenanceModalLauncher = useStopNodeMaintenanceModal(nodeMaintenance);
2929

30+
const actions = useMemo(() => {
3031
let action: Action = {
3132
id: 'start-node-maintenance',
3233
label: t('metal3-plugin~Start Maintenance'),
@@ -38,13 +39,13 @@ export const useNodeMaintenanceActions: ExtensionHook<Action[], NodeKind> = (res
3839
action = {
3940
id: 'stop-node-maintenance',
4041
label: t('metal3-plugin~Stop Maintenance'),
41-
cta: () => stopNodeMaintenanceModal(nodeMaintenance, t),
42+
cta: () => stopNodeMaintenanceModalLauncher(),
4243
insertBefore: 'edit-labels',
4344
};
4445
}
4546
return [action];
4647
// eslint-disable-next-line react-hooks/exhaustive-deps
47-
}, [maintenances, resource.metadata.name, t]);
48+
}, [maintenances, resource.metadata.name, t, stopNodeMaintenanceModalLauncher]);
4849

4950
return [actions, loading, loadError];
5051
};

frontend/packages/metal3-plugin/src/components/modals/StopNodeMaintenanceModal.tsx

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,4 @@
1-
import { TFunction } from 'i18next';
21
import { Trans, useTranslation } from 'react-i18next';
3-
import { confirmModal } from '@console/internal/components/modals/confirm-modal';
42
import { k8sKill, K8sResourceKind } from '@console/internal/module/k8s';
53
import { useWarningModal } from '@console/shared/src/hooks/useWarningModal';
64
import {
@@ -26,23 +24,6 @@ const getMaintenanceModel = (nodeMaintenance: K8sResourceKind) => {
2624
return NodeMaintenanceKubevirtAlphaModel;
2725
};
2826

29-
const stopNodeMaintenanceModal = (nodeMaintenance: K8sResourceKind, t: TFunction) => {
30-
const reason = getNodeMaintenanceReason(nodeMaintenance);
31-
const reasonLabel = reason ? `(${reason})` : '';
32-
const nodeName = getNodeMaintenanceNodeName(nodeMaintenance);
33-
return confirmModal({
34-
title: t('metal3-plugin~Stop maintenance'),
35-
message: (
36-
<Trans t={t} ns="metal3-plugin">
37-
Are you sure you want to stop maintenance <strong>{reasonLabel}</strong> on node{' '}
38-
<strong>{nodeName}</strong>?
39-
</Trans>
40-
),
41-
btnText: t('metal3-plugin~Stop maintenance'),
42-
executeFn: () => k8sKill(getMaintenanceModel(nodeMaintenance), nodeMaintenance),
43-
});
44-
};
45-
4627
export const useStopNodeMaintenanceModal = (nodeMaintenance: K8sResourceKind) => {
4728
const { t } = useTranslation();
4829
const reason = getNodeMaintenanceReason(nodeMaintenance);
@@ -61,5 +42,3 @@ export const useStopNodeMaintenanceModal = (nodeMaintenance: K8sResourceKind) =>
6142
});
6243
return stopNodeMaintenanceModalLauncher;
6344
};
64-
65-
export default stopNodeMaintenanceModal;

frontend/packages/topology/src/components/graph-view/Topology.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ import { TOPOLOGY_LAYOUT_CONFIG_STORAGE_KEY, TOPOLOGY_LAYOUT_LOCAL_STORAGE_KEY }
4141
import { odcElementFactory } from '../../elements';
4242
import { getTopologyGraphModel, setTopologyGraphModel } from '../../redux/action';
4343
import { SHOW_GROUPING_HINT_EVENT, ShowGroupingHintEventListener } from '../../topology-types';
44-
import { useSetupMoveNodeToGroupErrorHandler } from '../../utils';
44+
import { useSetupMoveNodeToGroupHandlers } from '../../utils';
4545
import { componentFactory } from './components';
4646
import { DEFAULT_LAYOUT, SUPPORTED_LAYOUTS, layoutFactory } from './layouts/layoutFactory';
4747
import TopologyControlBar from './TopologyControlBar';
@@ -154,8 +154,8 @@ const Topology: FC<
154154
TopologyComponentFactory
155155
>(isTopologyComponentFactory);
156156

157-
// Setup global error handlers for topology operations
158-
useSetupMoveNodeToGroupErrorHandler();
157+
// Setup global error and confirmation handlers for topology operations
158+
useSetupMoveNodeToGroupHandlers();
159159
useSetupGlobalErrorModalLauncher();
160160

161161
const createVisualization = useCallback(() => {

frontend/packages/topology/src/components/graph-view/components/componentUtils.ts

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -136,18 +136,23 @@ const nodeDragSourceSpec = (
136136
if (monitor.didDrop() && dropResult && props && props.element.getParent() !== dropResult) {
137137
const controller = props.element.getController();
138138

139-
await moveNodeToGroup(
140-
props.element as Node,
141-
isNode(dropResult) ? (dropResult as Node) : null,
142-
);
143-
144-
// perform the optimistic update in an action so as not to render too soon
145-
action(() => {
146-
// FIXME: check shouldn't be necessary if we handled the async and backend data refresh correctly
147-
if (controller.getNodeById(props.element.getId())) {
148-
dropResult.appendChild(props.element);
149-
}
150-
})();
139+
try {
140+
await moveNodeToGroup(
141+
props.element as Node,
142+
isNode(dropResult) ? (dropResult as Node) : null,
143+
);
144+
145+
// perform the optimistic update in an action so as not to render too soon
146+
action(() => {
147+
// FIXME: check shouldn't be necessary if we handled the async and backend data refresh correctly
148+
if (controller.getNodeById(props.element.getId())) {
149+
dropResult.appendChild(props.element);
150+
}
151+
})();
152+
} catch (error) {
153+
// User cancelled or operation failed - don't perform the optimistic update
154+
// The node will remain in its original position
155+
}
151156
} else {
152157
// cancel operation
153158
return Promise.reject();

frontend/packages/topology/src/utils/moveNodeToGroup.tsx

Lines changed: 100 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
import { useCallback, useEffect } from 'react';
22
import { Node } from '@patternfly/react-topology';
3-
import { Trans } from 'react-i18next';
3+
import { Trans, useTranslation } from 'react-i18next';
44
import { useOverlay } from '@console/dynamic-plugin-sdk/src/app/modal-support/useOverlay';
5-
import { confirmModal } from '@console/internal/components/modals';
65
import { ErrorModal, ErrorModalProps } from '@console/internal/components/modals/error-modal';
6+
import { useWarningModal } from '@console/shared/src/hooks/useWarningModal';
77
import { updateTopologyResourceApplication } from './topology-utils';
88

99
// Module-level error handler that can be set by the Topology component
@@ -13,6 +13,31 @@ export const setMoveNodeToGroupErrorHandler = (handler: ((error: string) => void
1313
globalErrorHandler = handler;
1414
};
1515

16+
// Module-level confirm handler that can be set by the Topology component
17+
let globalConfirmHandler:
18+
| ((
19+
title: string,
20+
message: React.ReactNode,
21+
confirmButtonText: string,
22+
onConfirm: () => Promise<void>,
23+
onCancel: () => void,
24+
) => void)
25+
| null = null;
26+
27+
export const setMoveNodeToGroupConfirmHandler = (
28+
handler:
29+
| ((
30+
title: string,
31+
message: React.ReactNode,
32+
confirmButtonText: string,
33+
onConfirm: () => Promise<void>,
34+
onCancel: () => void,
35+
) => void)
36+
| null,
37+
) => {
38+
globalConfirmHandler = handler;
39+
};
40+
1641
export const moveNodeToGroup = (
1742
node: Node,
1843
targetGroup: Node,
@@ -24,14 +49,14 @@ export const moveNodeToGroup = (
2449
}
2550

2651
if (sourceGroup) {
27-
// t('topology~Move component node')
28-
// t('topology~Remove component node from application')
29-
const titleKey = targetGroup
30-
? 'topology~Move component node'
31-
: 'topology~Remove component node from application';
3252
const nodeLabel = node.getLabel();
3353
const sourceLabel = sourceGroup.getLabel();
3454
const targetLabel = targetGroup?.getLabel();
55+
56+
// t('topology~Move component node')
57+
// t('topology~Remove component node from application')
58+
const title = targetGroup ? 'Move component node' : 'Remove component node from application';
59+
3560
const message = targetGroup ? (
3661
<Trans ns="topology">
3762
Are you sure you want to move <strong>{{ nodeLabel }}</strong> from {{ sourceLabel }} to{' '}
@@ -42,37 +67,35 @@ export const moveNodeToGroup = (
4267
Are you sure you want to remove <strong>{{ nodeLabel }}</strong> from {{ sourceLabel }}?
4368
</Trans>
4469
);
70+
4571
// t('topology~Move')
4672
// t('topology~Remove')
47-
const btnTextKey = targetGroup ? 'topology~Move' : 'topology~Remove';
73+
const confirmButtonText = targetGroup ? 'Move' : 'Remove';
4874

4975
return new Promise((resolve, reject) => {
50-
confirmModal({
51-
titleKey,
52-
message,
53-
btnTextKey,
54-
close: () => {
55-
reject();
56-
},
57-
cancel: () => {
58-
reject();
59-
},
60-
executeFn: () => {
61-
return updateTopologyResourceApplication(
62-
node,
63-
targetGroup ? targetGroup.getLabel() : null,
64-
)
65-
.then(resolve)
66-
.catch((err) => {
67-
const error = err.message;
68-
const errorHandler = onError || globalErrorHandler;
69-
if (errorHandler) {
70-
errorHandler(error);
71-
}
72-
reject(err);
73-
});
74-
},
75-
});
76+
if (!globalConfirmHandler) {
77+
reject(new Error('Confirm handler not initialized'));
78+
return;
79+
}
80+
81+
const handleConfirm = () => {
82+
return updateTopologyResourceApplication(node, targetGroup ? targetGroup.getLabel() : null)
83+
.then(resolve)
84+
.catch((err) => {
85+
const error = err.message;
86+
const errorHandler = onError || globalErrorHandler;
87+
if (errorHandler) {
88+
errorHandler(error);
89+
}
90+
reject(err);
91+
});
92+
};
93+
94+
const handleCancel = () => {
95+
reject(new Error('User cancelled'));
96+
};
97+
98+
globalConfirmHandler(title, message, confirmButtonText, handleConfirm, handleCancel);
7699
});
77100
}
78101

@@ -86,6 +109,49 @@ export const moveNodeToGroup = (
86109
};
87110

88111
/**
112+
* Hook that sets up both error and confirm handling for moveNodeToGroup using useOverlay.
113+
* This sets global handlers that will be used by all moveNodeToGroup calls.
114+
*/
115+
export const useSetupMoveNodeToGroupHandlers = () => {
116+
const { t } = useTranslation();
117+
const launcher = useOverlay();
118+
const launchWarningModal = useWarningModal();
119+
120+
useEffect(() => {
121+
const errorHandler = (error: string) => {
122+
launcher<ErrorModalProps>(ErrorModal, { error });
123+
};
124+
125+
const confirmHandler = (
126+
title: string,
127+
message: React.ReactNode,
128+
confirmButtonText: string,
129+
onConfirm: () => Promise<void>,
130+
onCancel: () => void,
131+
) => {
132+
launchWarningModal({
133+
title: t(`topology~${title}`),
134+
children: message,
135+
confirmButtonLabel: t(`topology~${confirmButtonText}`),
136+
onConfirm,
137+
onClose: onCancel,
138+
});
139+
};
140+
141+
setMoveNodeToGroupErrorHandler(errorHandler);
142+
setMoveNodeToGroupConfirmHandler(confirmHandler);
143+
144+
return () => {
145+
setMoveNodeToGroupErrorHandler(null);
146+
setMoveNodeToGroupConfirmHandler(null);
147+
};
148+
// Intentionally only run once on mount
149+
// eslint-disable-next-line react-hooks/exhaustive-deps
150+
}, []);
151+
};
152+
153+
/**
154+
* @deprecated Use useSetupMoveNodeToGroupHandlers in the parent component instead
89155
* Hook that sets up error handling for moveNodeToGroup using useOverlay.
90156
* This sets a global error handler that will be used by all moveNodeToGroup calls.
91157
*/

0 commit comments

Comments
 (0)