CONSOLE-5012: Migrate delete modals and PVC modals to overlay pattern

[rhamilto]

Summary

Migrates delete modals and PVC-related modals from the deprecated createModalLauncher pattern to the modern OverlayComponent pattern. This is part of CONSOLE-5012 to modernize modal handling across the console.

Changes

Modals Migrated (5 total)

All modals now follow the OverlayComponent pattern with consistent implementation:

1. DeleteModal (public/components/modals/delete-modal.tsx)

   • Wrapped with DeleteModalOverlay: OverlayComponent
   • Removed createModalLauncher usage

2. DeletePVCModal (public/components/modals/delete-pvc-modal.tsx)

   • Wrapped with DeletePVCModalOverlay: OverlayComponent
   • Replaced history.push with useNavigate from react-router-dom-v5-compat
   • Fixed typo: "resourcce" → "resource"

3. ExpandPVCModal (public/components/modals/expand-pvc-modal.tsx)

   • Wrapped with ExpandPVCModalOverlay: OverlayComponent
   • Replaced history.push with useNavigate from react-router-dom-v5-compat
   • Exported as LazyExpandPVCModalOverlay

4. ClonePVCModal (packages/console-app/src/components/modals/clone/clone-pvc-modal.tsx)

   • Wrapped with ClonePVCModalOverlay: OverlayComponent
   • Replaced history.push with useNavigate from react-router-dom-v5-compat
   • Exported as LazyClonePVCModalOverlay

5. RestorePVCModal (packages/console-app/src/components/modals/restore-pvc/restore-pvc-modal.tsx)

   • Wrapped with RestorePVCModalOverlay: OverlayComponent
   • Replaced history.push with useNavigate from react-router-dom-v5-compat
   • Exported as LazyRestorePVCModalOverlay

Action Creators Converted to Hooks (2 total)

To integrate with the React hooks system and useOverlay() pattern:

1. useDeleteResourceAction (packages/dev-console/src/actions/context-menu.ts)

   • Converted from DeleteResourceAction function to hook
   • Uses useOverlay() and LazyDeleteModalOverlay
   • Updated consumers: deployment-provider.ts, deploymentconfig-provider.ts

2. useDeleteKnativeServiceResource (packages/knative-plugin/src/actions/creators.ts)

   • Converted from deleteKnativeServiceResource function to hook
   • Conditional cleanUpWorkload based on serviceCreatedFromWebFlag
   • Updated consumer: knative-plugin/src/actions/providers.ts

Updated to Use New Overlay Modals

Replaced direct modal function calls with useOverlay() + lazy modal overlays:

• useCommonActions.ts: Changed from deleteModal to launchModal(LazyDeleteModalOverlay, ...)
• usePVCActions.ts:
  • Changed from expandPVCModal to launchModal(LazyExpandPVCModalOverlay, ...)
  • Changed from clonePVCModal to launchModal(LazyClonePVCModalOverlay, ...)
  • Changed from deletePVCModal to launchModal(LazyDeletePVCModalOverlay, ...)
• useVolumeSnapshotActions.ts: Changed from restorePVCModal to launchModal(LazyRestorePVCModalOverlay, ...)

React Hooks Dependency Array Updates

Added launchModal to dependency arrays where it's used:

• useCommonActions.ts: Added launchModal to dependency array with explanatory comment about excluded stable modal launcher functions
• usePVCActions.ts: Uses launchModal from useOverlay() hook
• useVolumeSnapshotActions.ts: Added launchModal to dependency array
• useDeleteResourceAction: Added launchModal to dependency array
• useDeleteKnativeServiceResource: Added launchModal to dependency array

Note: While launchModal from useOverlay() is referentially stable, it's included in dependency arrays for correctness and to satisfy exhaustive-deps linting rules.

Navigation Updates

Replaced deprecated history.push with modern useNavigate hook:

• All PVC modals now use useNavigate from react-router-dom-v5-compat
• Removed setTimeout wrappers that were previously needed with history.push
• Direct navigate() calls are safe with the modern hook (no setState during render warnings)

Naming Convention

All modal overlay components and exports follow the *ModalOverlay naming pattern:

• Component exports: DeleteModalOverlay, DeletePVCModalOverlay, ExpandPVCModalOverlay, ClonePVCModalOverlay, RestorePVCModalOverlay
• Lazy exports: LazyDeleteModalOverlay, LazyDeletePVCModalOverlay, LazyExpandPVCModalOverlay, LazyClonePVCModalOverlay, LazyRestorePVCModalOverlay

This naming convention clearly distinguishes these from the deprecated ModalProvider pattern and aligns with the OverlayComponent type.

Infrastructure Updates

• OverlayProvider.tsx: Added <Suspense> wrapper for lazy-loaded overlay components
• app.tsx: Added global Modal.setAppElement initialization in useLayoutEffect for accessibility
• modal.tsx:
  • Removed individual Modal.setAppElement calls (now set globally)
  • Simplified appElement selector in ModalWrapper (now uses document.getElementById instead of fallback chain)
  • Added appElement prop to ModalWrapper for proper accessibility
• modals/index.ts: Updated exports to use consistent React.lazy() pattern with all new modal overlays

Pattern Example

// Modal file
export const DeleteModalOverlay: OverlayComponent<DeleteModalProps> = (props) => {
  return (
    <ModalWrapper blocking onClose={props.closeOverlay}>
      <DeleteModal {...props} cancel={props.closeOverlay} close={props.closeOverlay} />
    </ModalWrapper>
  );
};

// index.ts
export const LazyDeleteModalOverlay = lazy(() =>
  import('./delete-modal').then((m) => ({ default: m.DeleteModalOverlay })),
);

// Action hook
export const useDeleteResourceAction = (kind: K8sModel, obj: K8sResourceKind): Action => {
  const { t } = useTranslation();
  const launchModal = useOverlay();

  return useMemo(
    () => ({
      id: 'delete-resource',
      label: t('devconsole~Delete {{kind}}', { kind: kind.kind }),
      cta: () => launchModal(LazyDeleteModalOverlay, { kind, resource: obj, deleteAllResources: () => cleanUpWorkload(obj) }),
      accessReview: asAccessReview(kind, obj, 'delete'),
    }),
    [t, kind, obj, launchModal],
  );
};

// Modal with navigation
const MyModal = ({ close, resource }) => {
  const navigate = useNavigate();
  const [handlePromise] = usePromiseHandler();

  const submit = () => {
    handlePromise(k8sCreate(SomeModel, resource)).then((newResource) => {
      close();
      navigate(resourceObjPath(newResource, referenceFor(newResource)));
    });
  };
  // ...
};

Benefits

• ✅ Consistent pattern across all modals
• ✅ Better code splitting with lazy loading
• ✅ Proper React hooks integration with useOverlay()
• ✅ Improved accessibility with global Modal.setAppElement setup
• ✅ Modern navigation with useNavigate hook (no setTimeout wrappers needed)
• ✅ Maintains backward compatibility
• ✅ Improved maintainability
• ✅ Cleaner, more idiomatic React code
• ✅ Clear naming convention with *ModalOverlay pattern

Testing

• All 5 modals open and close correctly
• Delete actions work for Deployments, DeploymentConfigs, and Knative Services
• PVC actions work (expand, clone, restore, delete)
• Navigation after modal actions works correctly without warnings
• No console errors or infinite loops
• Lazy loading works correctly (check Network tab for separate chunks)
• Modal accessibility properly configured
• useNavigate navigation works without setState warnings

Related

• Follows pattern from CONSOLE-4610: Drop use of ceateModalLauncher from Topology package #15446 (EditApplicationModal migration)
• Part of CONSOLE-5012 modal modernization effort

Summary by CodeRabbit

• Refactor

  • Modal system migrated to overlay-based pattern with *ModalOverlay naming convention for delete/expand/clone/restore operations.
  • Modals now lazy-load, improving performance and code-splitting.
  • Navigation updated to use modern navigation hooks.

• Bug Fixes / Accessibility

  • Global modal initialization moved to app mount for more stable behavior and improved accessibility.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[openshift-ci-robot]

@rhamilto: This pull request references CONSOLE-5012 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set.

Details

In response to this:

Summary

Migrates PVC-related modals (expand, clone, delete, restore) from the legacy createModalLauncher pattern to the modern OverlayComponent pattern with lazy loading.

Changes

• Convert modal exports to OverlayComponent providers with ModalWrapper
• Implement lazy loading for modal components using React's lazy()
• Update action hooks (usePVCActions, useVolumeSnapshotActions) to use useOverlay() and launchModal()
• Set react-modal app element globally in App component for accessibility
• Maintain backward compatibility with existing modal exports

Test plan

• Verify expand PVC modal opens and functions correctly
• Verify clone PVC modal opens and functions correctly (when PVC is Bound)
• Verify delete PVC modal opens and functions correctly
• Verify restore PVC modal opens and functions correctly (from volume snapshot)
• Verify modals close properly
• Check that accessibility features work (aria attributes, focus management)

🤖 Generated with Claude Code

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[openshift-ci-robot]

@rhamilto: This pull request references CONSOLE-5012 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set.

Details

In response to this:

Summary

Migrates PVC-related modals (expand, clone, delete, restore) from the legacy createModalLauncher pattern to the modern OverlayComponent pattern with lazy loading.

Changes

• Convert modal exports to OverlayComponent providers with ModalWrapper
• Implement lazy loading for modal components using React's lazy()
• Update action hooks (usePVCActions, useVolumeSnapshotActions) to use useOverlay() and launchModal()
• Set react-modal app element globally in App component for accessibility
• Maintain backward compatibility with existing modal exports

Test plan

• Verify expand PVC modal opens and functions correctly
• Verify clone PVC modal opens and functions correctly (when PVC is Bound)
• Verify delete PVC modal opens and functions correctly
• Verify restore PVC modal opens and functions correctly (from volume snapshot)
• Verify modals close properly
• Check that accessibility features work (aria attributes, focus management)

🤖 Generated with Claude Code

Summary by CodeRabbit

• Refactor
• Updated modal infrastructure for PVC operations (expand, clone, delete) and volume snapshot restoration to use an overlay-based provider architecture.
• Optimized modal loading performance through lazy-loading of modal providers.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[coderabbitai]

📝 Walkthrough

Walkthrough

Modals were migrated from createModalLauncher-based launchers to OverlayComponent providers wrapped by ModalWrapper and wired to closeOverlay. Lazy (React.lazy) provider variants were added and callers now use useOverlay/launchModal to open modals. Several action factories were refactored into hook-based actions (useMemo returning Action) that invoke lazy providers. Modal.setAppElement is initialized in App on mount, and OverlayProvider rendering now uses Suspense for dynamic overlays.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title accurately describes the main architectural change: migrating modals from createModalLauncher to the OverlayComponent pattern, which is the primary refactoring across all modified files.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In `@frontend/public/components/modals/delete-pvc-modal.tsx`:
- Around line 9-16: Restore a default export so existing callers importing the
default don't break: export the modal component previously exposed as the
default (the DeletePVCModal component used with createModalLauncher) as the
file's default export in addition to any named exports; locate the
DeletePVCModal symbol (and the modal wrapper/props usage such as
ModalComponentProps) and add a default export for it so index
re-exports/back-compat continue to work.

In `@frontend/public/components/modals/index.ts`:
- Line 3: OverlayProvider is rendering lazy components from componentsMap
without a Suspense boundary causing runtime throws; import Suspense from 'react'
and wrap each mapped provider render (the map that returns <c.Component
{...c.props} closeOverlay={() => closeOverlay(id)} />) in <Suspense
fallback={null}> so each lazy modal provider is suspended safely; apply the same
change to the LazyClonePVCModalProvider and LazyRestorePVCModalProvider render
blocks so their lazy components are also wrapped with Suspense.

🧹 Nitpick comments (2)

frontend/packages/console-app/src/components/modals/clone/clone-pvc-modal.tsx (1)

279-285: Provider implementation looks solid; consider explicit prop destructuring for clarity.

The pattern is correct: blocking prevents accidental dismissal during form entry, and mapping both cancel and close to closeOverlay aligns with how ClonePVCModal uses them (success path at line 132, cancel button at line 269).

One minor refinement: spreading ...props passes closeOverlay to the inner modal which doesn't consume it. While harmless, destructuring makes the prop threading more explicit:

✨ Optional: explicit prop destructuring

-export const ClonePVCModalProvider: OverlayComponent<ClonePVCModalProps> = (props) => {
+export const ClonePVCModalProvider: OverlayComponent<ClonePVCModalProps> = ({
+  closeOverlay,
+  ...modalProps
+}) => {
   return (
-    <ModalWrapper blocking onClose={props.closeOverlay}>
-      <ClonePVCModal cancel={props.closeOverlay} close={props.closeOverlay} {...props} />
+    <ModalWrapper blocking onClose={closeOverlay}>
+      <ClonePVCModal cancel={closeOverlay} close={closeOverlay} {...modalProps} />
     </ModalWrapper>
   );
 };

frontend/public/components/modals/expand-pvc-modal.tsx (1)

4-11: Keep close non‑optional to prevent runtime undefined calls.

Line 97 now inherits close?: () => void from ModalComponentProps, but Line 47 calls close() unguarded. Consider keeping close required in ExpandPVCModalProps (even if you still reuse the shared type) to avoid accidental misuse in tests or future direct renders.

♻️ Suggested type tightening

 export type ExpandPVCModalProps = {
   kind: K8sKind;
   resource: K8sResourceKind;
-} & ModalComponentProps;
+} & Required<Pick<ModalComponentProps, 'close'>> &
+  Pick<ModalComponentProps, 'cancel'>;

Also applies to: 97-108

[openshift-ci-robot]

@rhamilto: This pull request references CONSOLE-5012 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set.

Details

In response to this:

Summary

This PR migrates PVC-related modals and the general delete modal from the deprecated createModalLauncher pattern to the modern OverlayComponent pattern, following the approach established in #15446.

Changes

Modals Refactored (5 files)

• delete-pvc-modal.tsx - Added DeletePVCModalProvider using OverlayComponent pattern
• clone-pvc-modal.tsx - Added ClonePVCModalProvider
• expand-pvc-modal.tsx - Added ExpandPVCModalProvider
• restore-pvc-modal.tsx - Added RestorePVCModalProvider
• delete-modal.tsx - Added DeleteModalProvider (general resource deletion)

Action Creators Converted to Hooks (3 functions)

To work with the overlay system and React's rules of hooks, several action creator functions were converted to hooks:

• DeleteResourceAction → useDeleteResourceAction in packages/dev-console/src/actions/context-menu.ts
• DeleteApplicationAction → useDeleteApplicationAction in packages/dev-console/src/actions/context-menu.ts
• deleteKnativeServiceResource → useDeleteKnativeServiceResource in packages/knative-plugin/src/actions/creators.ts

Files Updated to Use New Patterns (7 files)

• useCommonActions.ts - Updated to use LazyDeleteModalProvider with launchModal
• usePVCActions.ts - Updated to use lazy PVC modal providers
• useVolumeSnapshotActions.ts - Updated to use LazyRestorePVCModalProvider
• deployment-provider.ts - Updated to use useDeleteResourceAction hook
• deploymentconfig-provider.ts - Updated to use useDeleteResourceAction hook
• knative-plugin/actions/providers.ts - Updated to use useDeleteKnativeServiceResource hook
• dev-console/actions/providers.tsx - Updated to use useDeleteApplicationAction hook

Infrastructure Updates (2 files)

• public/components/modals/index.ts - Added lazy-loaded exports for all refactored modal providers using React.lazy() with webpack chunk names
• packages/console-dynamic-plugin-sdk/src/app/modal-support/OverlayProvider.tsx - Added Suspense wrapper to handle lazy-loaded components

Benefits

• ✅ Eliminates deprecated createModalLauncher usage for all affected modals
• ✅ Preserves lazy loading through React.lazy() pattern
• ✅ Maintains backward compatibility via default exports
• ✅ Improves code splitting with named webpack chunks
• ✅ Follows modern React patterns with hooks
• ✅ All builds, TypeScript compilation, and linting checks pass

Testing

• Production build succeeds
• TypeScript compilation passes
• ESLint passes with no errors
• No instances of deleteModal() function calls remain
• All refactored modals use OverlayComponent pattern

Related

• Follows pattern from CONSOLE-4610: Drop use of ceateModalLauncher from Topology package #15446 (EditApplicationModal)
• Part of CONSOLE-5012 epic to modernize modal handling

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[openshift-ci-robot]

@rhamilto: This pull request references CONSOLE-5012 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set.

Details

In response to this:

Summary

This PR migrates PVC-related modals and the general delete modal from the deprecated createModalLauncher pattern to the modern OverlayComponent pattern, following the approach established in #15446.

Changes

Modals Refactored (5 files)

• delete-pvc-modal.tsx - Added DeletePVCModalProvider using OverlayComponent pattern
• clone-pvc-modal.tsx - Added ClonePVCModalProvider
• expand-pvc-modal.tsx - Added ExpandPVCModalProvider
• restore-pvc-modal.tsx - Added RestorePVCModalProvider
• delete-modal.tsx - Added DeleteModalProvider (general resource deletion)

Action Creators Converted to Hooks (3 functions)

To work with the overlay system and React's rules of hooks, several action creator functions were converted to hooks:

• DeleteResourceAction → useDeleteResourceAction in packages/dev-console/src/actions/context-menu.ts
• DeleteApplicationAction → useDeleteApplicationAction in packages/dev-console/src/actions/context-menu.ts
• deleteKnativeServiceResource → useDeleteKnativeServiceResource in packages/knative-plugin/src/actions/creators.ts

Files Updated to Use New Patterns (7 files)

• useCommonActions.ts - Updated to use LazyDeleteModalProvider with launchModal
• usePVCActions.ts - Updated to use lazy PVC modal providers
• useVolumeSnapshotActions.ts - Updated to use LazyRestorePVCModalProvider
• deployment-provider.ts - Updated to use useDeleteResourceAction hook
• deploymentconfig-provider.ts - Updated to use useDeleteResourceAction hook
• knative-plugin/actions/providers.ts - Updated to use useDeleteKnativeServiceResource hook
• dev-console/actions/providers.tsx - Updated to use useDeleteApplicationAction hook

Infrastructure Updates (2 files)

• public/components/modals/index.ts - Added lazy-loaded exports for all refactored modal providers using React.lazy() with webpack chunk names
• packages/console-dynamic-plugin-sdk/src/app/modal-support/OverlayProvider.tsx - Added Suspense wrapper to handle lazy-loaded components

Benefits

• ✅ Eliminates deprecated createModalLauncher usage for all affected modals
• ✅ Preserves lazy loading through React.lazy() pattern
• ✅ Maintains backward compatibility via default exports
• ✅ Improves code splitting with named webpack chunks
• ✅ Follows modern React patterns with hooks
• ✅ All builds, TypeScript compilation, and linting checks pass

Testing

• Production build succeeds
• TypeScript compilation passes
• ESLint passes with no errors
• No instances of deleteModal() function calls remain
• All refactored modals use OverlayComponent pattern

Related

• Follows pattern from CONSOLE-4610: Drop use of ceateModalLauncher from Topology package #15446 (EditApplicationModal)
• Part of CONSOLE-5012 epic to modernize modal handling

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[coderabbitai]

Actionable comments posted: 3

🤖 Fix all issues with AI agents

In `@frontend/packages/dev-console/src/actions/context-menu.ts`:
- Around line 42-45: The code calls asAccessReview using kind casted unsafely in
useDeleteResourceAction; add a guard for kind === undefined at the start of
useDeleteResourceAction to avoid the unsafe cast — return an Action that is
disabled (with a clear disabledReason) or early-return null/undefined per the
surrounding API, and only call asAccessReview and perform RBAC checks when kind
is defined; update references to useDeleteResourceAction and any downstream
consumers to handle the disabled/early-return case accordingly.

In `@frontend/packages/knative-plugin/src/actions/creators.ts`:
- Around line 120-152: The hook useDeleteKnativeServiceResource currently
assumes kind is defined and dereferences it; update the function to guard for
undefined kind by returning an Action object that is disabled/no-op (e.g., id
`delete-resource`, label same as before, cta as a noop and accessReview omitted
or set to a safe falsy value) when kind is undefined, and only build the full
action (including using LazyDeleteModalProvider, cleanUpWorkload in
deleteAllResources, and asAccessReview) once kind is present; ensure the
returned disabled action still matches the Action shape so render won't throw
and keep dependencies ([kind, obj, serviceTypeValue, serviceCreatedFromWebFlag,
launchModal]) intact.

In `@frontend/public/components/factory/modal.tsx`:
- Around line 37-40: Guard the Modal's appElement prop against a missing DOM
root: when obtaining appElement via document.getElementById('app-content')
ensure you convert a null result to undefined (matching the App component
pattern) before passing it into the Modal component (i.e., normalize the result
of document.getElementById('app-content') to undefined and pass that variable
named appElement into <Modal .../>). This keeps behavior explicit and consistent
with Modal.setAppElement usage.

[rhamilto]

/retest

[rhamilto]

/assign @yapei

Tech debt, so assigning labels
/label px-approved
/label docs-approved

[openshift-ci-robot]

@rhamilto: This pull request references CONSOLE-5012 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set.

Details

In response to this:

Summary

Migrates delete modals and PVC-related modals from the deprecated createModalLauncher pattern to the modern OverlayComponent pattern. This is part of CONSOLE-5012 to modernize modal handling across the console.

Changes

Modals Migrated (5 total)

All modals now follow the OverlayComponent pattern with consistent implementation:

1. DeleteModal (public/components/modals/delete-modal.tsx)

• Wrapped with DeleteModalProvider: OverlayComponent
• Removed createModalLauncher usage

2. DeletePVCModal (public/components/modals/delete-pvc-modal.tsx)

• Wrapped with DeletePVCModalProvider: OverlayComponent
• Replaced history.push with useNavigate from react-router-dom-v5-compat
• Fixed typo: "resourcce" → "resource"

3. ExpandPVCModal (public/components/modals/expand-pvc-modal.tsx)

• Wrapped with ExpandPVCModalProvider: OverlayComponent
• Replaced history.push with useNavigate from react-router-dom-v5-compat
• Exported as LazyExpandPVCModalProvider

4. ClonePVCModal (packages/console-app/src/components/modals/clone/clone-pvc-modal.tsx)

• Wrapped with ClonePVCModalProvider: OverlayComponent
• Replaced history.push with useNavigate from react-router-dom-v5-compat
• Exported as LazyClonePVCModalProvider

5. RestorePVCModal (packages/console-app/src/components/modals/restore-pvc/restore-pvc-modal.tsx)

• Wrapped with RestorePVCModalProvider: OverlayComponent
• Replaced history.push with useNavigate from react-router-dom-v5-compat
• Exported as LazyRestorePVCModalProvider

Action Creators Converted to Hooks (2 total)

To integrate with the React hooks system and useOverlay() pattern:

1. useDeleteResourceAction (packages/dev-console/src/actions/context-menu.ts)

• Converted from DeleteResourceAction function to hook
• Uses useOverlay() and LazyDeleteModalProvider
• Updated consumers: deployment-provider.ts, deploymentconfig-provider.ts

2. useDeleteKnativeServiceResource (packages/knative-plugin/src/actions/creators.ts)

• Converted from deleteKnativeServiceResource function to hook
• Conditional cleanUpWorkload based on serviceCreatedFromWebFlag
• Updated consumer: knative-plugin/src/actions/providers.ts

Updated to Use New Overlay Modals

Replaced direct modal function calls with useOverlay() + lazy modal providers:

• useCommonActions.ts: Changed from deleteModal to launchModal(LazyDeleteModalProvider, ...)
• usePVCActions.ts:
• Changed from expandPVCModal to launchModal(LazyExpandPVCModalProvider, ...)
• Changed from clonePVCModal to launchModal(LazyClonePVCModalProvider, ...)
• Changed from deletePVCModal to launchModal(LazyDeletePVCModalProvider, ...)
• useVolumeSnapshotActions.ts: Changed from restorePVCModal to launchModal(LazyRestorePVCModalProvider, ...)

React Hooks Dependency Array Updates

Added launchModal to dependency arrays where it's used:

• useCommonActions.ts: Added launchModal to dependency array
• usePVCActions.ts: Uses launchModal from useOverlay() hook
• useVolumeSnapshotActions.ts: Added launchModal to dependency array
• useDeleteResourceAction: Added launchModal to dependency array
• useDeleteKnativeServiceResource: Added launchModal to dependency array

Note: While launchModal from useOverlay() is referentially stable, it's included in dependency arrays for correctness and to satisfy exhaustive-deps linting rules.

Navigation Updates

Replaced deprecated history.push with modern useNavigate hook:

• All PVC modals now use useNavigate from react-router-dom-v5-compat
• Removed setTimeout wrappers that were previously needed with history.push
• Direct navigate() calls are safe with the modern hook (no setState during render warnings)

Infrastructure Updates

• OverlayProvider.tsx: Added <Suspense> wrapper for lazy-loaded overlay components
• app.tsx: Added global Modal.setAppElement initialization in useLayoutEffect for accessibility
• modal.tsx:
• Removed individual Modal.setAppElement calls (now set globally)
• Simplified appElement selector in ModalWrapper (now uses document.getElementById instead of fallback chain)
• Added appElement prop to ModalWrapper for proper accessibility
• modals/index.ts: Updated exports to use consistent React.lazy() pattern with all new modal providers

Pattern Example

// Modal file
export const DeleteModalProvider: OverlayComponent<DeleteModalProps> = (props) => {
 return (
   <ModalWrapper blocking onClose={props.closeOverlay}>
     <DeleteModal {...props} cancel={props.closeOverlay} close={props.closeOverlay} />
   </ModalWrapper>
 );
};

export default DeleteModalProvider;

// index.ts
export const LazyDeleteModalProvider = lazy(() =>
 import('./delete-modal').then((m) => ({ default: m.default })),
);

// Action hook
export const useDeleteResourceAction = (kind: K8sModel, obj: K8sResourceKind): Action => {
 const { t } = useTranslation();
 const launchModal = useOverlay();

 return useMemo(
   () => ({
     id: 'delete-resource',
     label: t('devconsole~Delete {{kind}}', { kind: kind.kind }),
     cta: () => launchModal(LazyDeleteModalProvider, { kind, resource: obj, deleteAllResources: () => cleanUpWorkload(obj) }),
     accessReview: asAccessReview(kind, obj, 'delete'),
   }),
   [t, kind, obj, launchModal],
 );
};

// Modal with navigation
const MyModal = ({ close, resource }) => {
 const navigate = useNavigate();
 const [handlePromise] = usePromiseHandler();

 const submit = () => {
   handlePromise(k8sCreate(SomeModel, resource)).then((newResource) => {
     close();
     navigate(resourceObjPath(newResource, referenceFor(newResource)));
   });
 };
 // ...
};

Benefits

• ✅ Consistent pattern across all modals
• ✅ Better code splitting with lazy loading
• ✅ Proper React hooks integration with useOverlay()
• ✅ Improved accessibility with global Modal.setAppElement setup
• ✅ Modern navigation with useNavigate hook (no setTimeout wrappers needed)
• ✅ Maintains backward compatibility
• ✅ Improved maintainability
• ✅ Cleaner, more idiomatic React code

Testing

• All 5 modals open and close correctly
• Delete actions work for Deployments, DeploymentConfigs, and Knative Services
• PVC actions work (expand, clone, restore, delete)
• Navigation after modal actions works correctly without warnings
• No console errors or infinite loops
• Lazy loading works correctly (check Network tab for separate chunks)
• Modal accessibility properly configured
• useNavigate navigation works without setState warnings

Related

• Follows pattern from CONSOLE-4610: Drop use of ceateModalLauncher from Topology package #15446 (EditApplicationModal migration)
• Part of CONSOLE-5012 modal modernization effort

Summary by CodeRabbit

• Refactor

• Modal system migrated to provider-based, overlay-powered modals for delete/expand/clone/restore operations.

• Modals now lazy-load, improving performance and code-splitting.

• Navigation updated to use modern navigation hooks.

• Bug Fixes / Accessibility

• Global modal initialization moved to app mount for more stable behavior and improved accessibility.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[yapei]

performed some regression tests about PVC actions, Expand, Clone, Restore...some delete modal
no regression issues found
/verified by @yapei

[openshift-ci-robot]

@yapei: This PR has been marked as verified by @yapei.

Details

In response to this:

performed some regression tests about PVC actions, Expand, Clone, Restore...some delete modal
no regression issues found
/verified by @yapei

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[openshift-ci-robot]

@rhamilto: This pull request references CONSOLE-5012 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set.

Details

In response to this:

Summary

Migrates delete modals and PVC-related modals from the deprecated createModalLauncher pattern to the modern OverlayComponent pattern. This is part of CONSOLE-5012 to modernize modal handling across the console.

Changes

Modals Migrated (5 total)

All modals now follow the OverlayComponent pattern with consistent implementation:

1. DeleteModal (public/components/modals/delete-modal.tsx)

• Wrapped with DeleteModalOverlay: OverlayComponent
• Removed createModalLauncher usage

2. DeletePVCModal (public/components/modals/delete-pvc-modal.tsx)

• Wrapped with DeletePVCModalOverlay: OverlayComponent
• Replaced history.push with useNavigate from react-router-dom-v5-compat
• Fixed typo: "resourcce" → "resource"

3. ExpandPVCModal (public/components/modals/expand-pvc-modal.tsx)

• Wrapped with ExpandPVCModalOverlay: OverlayComponent
• Replaced history.push with useNavigate from react-router-dom-v5-compat
• Exported as LazyExpandPVCModalOverlay

4. ClonePVCModal (packages/console-app/src/components/modals/clone/clone-pvc-modal.tsx)

• Wrapped with ClonePVCModalOverlay: OverlayComponent
• Replaced history.push with useNavigate from react-router-dom-v5-compat
• Exported as LazyClonePVCModalOverlay

5. RestorePVCModal (packages/console-app/src/components/modals/restore-pvc/restore-pvc-modal.tsx)

• Wrapped with RestorePVCModalOverlay: OverlayComponent
• Replaced history.push with useNavigate from react-router-dom-v5-compat
• Exported as LazyRestorePVCModalOverlay

Action Creators Converted to Hooks (2 total)

To integrate with the React hooks system and useOverlay() pattern:

1. useDeleteResourceAction (packages/dev-console/src/actions/context-menu.ts)

• Converted from DeleteResourceAction function to hook
• Uses useOverlay() and LazyDeleteModalOverlay
• Updated consumers: deployment-provider.ts, deploymentconfig-provider.ts

2. useDeleteKnativeServiceResource (packages/knative-plugin/src/actions/creators.ts)

• Converted from deleteKnativeServiceResource function to hook
• Conditional cleanUpWorkload based on serviceCreatedFromWebFlag
• Updated consumer: knative-plugin/src/actions/providers.ts

Updated to Use New Overlay Modals

Replaced direct modal function calls with useOverlay() + lazy modal overlays:

• useCommonActions.ts: Changed from deleteModal to launchModal(LazyDeleteModalOverlay, ...)
• usePVCActions.ts:
• Changed from expandPVCModal to launchModal(LazyExpandPVCModalOverlay, ...)
• Changed from clonePVCModal to launchModal(LazyClonePVCModalOverlay, ...)
• Changed from deletePVCModal to launchModal(LazyDeletePVCModalOverlay, ...)
• useVolumeSnapshotActions.ts: Changed from restorePVCModal to launchModal(LazyRestorePVCModalOverlay, ...)

React Hooks Dependency Array Updates

Added launchModal to dependency arrays where it's used:

• useCommonActions.ts: Added launchModal to dependency array with explanatory comment about excluded stable modal launcher functions
• usePVCActions.ts: Uses launchModal from useOverlay() hook
• useVolumeSnapshotActions.ts: Added launchModal to dependency array
• useDeleteResourceAction: Added launchModal to dependency array
• useDeleteKnativeServiceResource: Added launchModal to dependency array

Note: While launchModal from useOverlay() is referentially stable, it's included in dependency arrays for correctness and to satisfy exhaustive-deps linting rules.

Navigation Updates

Replaced deprecated history.push with modern useNavigate hook:

• All PVC modals now use useNavigate from react-router-dom-v5-compat
• Removed setTimeout wrappers that were previously needed with history.push
• Direct navigate() calls are safe with the modern hook (no setState during render warnings)

Naming Convention

All modal overlay components and exports follow the *ModalOverlay naming pattern:

• Component exports: DeleteModalOverlay, DeletePVCModalOverlay, ExpandPVCModalOverlay, ClonePVCModalOverlay, RestorePVCModalOverlay
• Lazy exports: LazyDeleteModalOverlay, LazyDeletePVCModalOverlay, LazyExpandPVCModalOverlay, LazyClonePVCModalOverlay, LazyRestorePVCModalOverlay

This naming convention clearly distinguishes these from the deprecated ModalProvider pattern and aligns with the OverlayComponent type.

Infrastructure Updates

• OverlayProvider.tsx: Added <Suspense> wrapper for lazy-loaded overlay components
• app.tsx: Added global Modal.setAppElement initialization in useLayoutEffect for accessibility
• modal.tsx:
• Removed individual Modal.setAppElement calls (now set globally)
• Simplified appElement selector in ModalWrapper (now uses document.getElementById instead of fallback chain)
• Added appElement prop to ModalWrapper for proper accessibility
• modals/index.ts: Updated exports to use consistent React.lazy() pattern with all new modal overlays

Pattern Example

// Modal file
export const DeleteModalOverlay: OverlayComponent<DeleteModalProps> = (props) => {
 return (
   <ModalWrapper blocking onClose={props.closeOverlay}>
     <DeleteModal {...props} cancel={props.closeOverlay} close={props.closeOverlay} />
   </ModalWrapper>
 );
};

// index.ts
export const LazyDeleteModalOverlay = lazy(() =>
 import('./delete-modal').then((m) => ({ default: m.DeleteModalOverlay })),
);

// Action hook
export const useDeleteResourceAction = (kind: K8sModel, obj: K8sResourceKind): Action => {
 const { t } = useTranslation();
 const launchModal = useOverlay();

 return useMemo(
   () => ({
     id: 'delete-resource',
     label: t('devconsole~Delete {{kind}}', { kind: kind.kind }),
     cta: () => launchModal(LazyDeleteModalOverlay, { kind, resource: obj, deleteAllResources: () => cleanUpWorkload(obj) }),
     accessReview: asAccessReview(kind, obj, 'delete'),
   }),
   [t, kind, obj, launchModal],
 );
};

// Modal with navigation
const MyModal = ({ close, resource }) => {
 const navigate = useNavigate();
 const [handlePromise] = usePromiseHandler();

 const submit = () => {
   handlePromise(k8sCreate(SomeModel, resource)).then((newResource) => {
     close();
     navigate(resourceObjPath(newResource, referenceFor(newResource)));
   });
 };
 // ...
};

Benefits

• ✅ Consistent pattern across all modals
• ✅ Better code splitting with lazy loading
• ✅ Proper React hooks integration with useOverlay()
• ✅ Improved accessibility with global Modal.setAppElement setup
• ✅ Modern navigation with useNavigate hook (no setTimeout wrappers needed)
• ✅ Maintains backward compatibility
• ✅ Improved maintainability
• ✅ Cleaner, more idiomatic React code
• ✅ Clear naming convention with *ModalOverlay pattern

Testing

• All 5 modals open and close correctly
• Delete actions work for Deployments, DeploymentConfigs, and Knative Services
• PVC actions work (expand, clone, restore, delete)
• Navigation after modal actions works correctly without warnings
• No console errors or infinite loops
• Lazy loading works correctly (check Network tab for separate chunks)
• Modal accessibility properly configured
• useNavigate navigation works without setState warnings

Related

• Follows pattern from CONSOLE-4610: Drop use of ceateModalLauncher from Topology package #15446 (EditApplicationModal migration)
• Part of CONSOLE-5012 modal modernization effort

Summary by CodeRabbit

• Refactor

• Modal system migrated to overlay-based pattern with *ModalOverlay naming convention for delete/expand/clone/restore operations.

• Modals now lazy-load, improving performance and code-splitting.

• Navigation updated to use modern navigation hooks.

• Bug Fixes / Accessibility

• Global modal initialization moved to app mount for more stable behavior and improved accessibility.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[rhamilto]

/retest

[rhamilto]

/retest

[openshift-ci]

@rhamilto: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-gcp-console	23cb295	link	true	/test e2e-gcp-console

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

[rhamilto]

/retest

[logonoff]

/lgtm

I think in theory some of these new action hooks should be applied and used via a console extension, but we can do that in a follow up.

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: logonoff, rhamilto

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• frontend/OWNERS [logonoff,rhamilto]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[rhamilto]

I think in theory some of these new action hooks should be applied and used via a console extension, but we can do that in a follow up.

IMO, that just adds extra code. I get the dog fooding argument, but...

[logonoff]

I think in theory some of these new action hooks should be applied and used via a console extension, but we can do that in a follow up.

IMO, that just adds extra code. I get the dog fooding argument, but...

It's just the pattern we were doing before.. I guess as long as people can still add their own kebab actions via an extension it's okay