[WIP] CONSOLE-5012: Additional modal migrations to overlay pattern #15940
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@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. DetailsIn response to this:
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. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rhamilto The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
component/dev-console
rhamilto
changed the title
|
@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. DetailsIn response to this:
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
bot
added
the
do-not-merge/work-in-progress
|
@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. DetailsIn response to this:
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: 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. DetailsIn response to this:
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. |
📝 WalkthroughWalkthroughThis pull request refactors the modal system across the console application to use a provider-based overlay pattern instead of direct modal launchers. Action hooks are converted to React hooks (e.g., DeleteApplicationAction → useDeleteApplicationAction), and modal invocations migrate from createModalLauncher to overlay-based launchModal calls with lazy-loaded provider components. Modal components are wrapped in ModalWrapper with OverlayComponent type signatures, and Suspense boundaries are added to OverlayProvider for handling lazy-loaded modals. The changes affect action hooks, modal provider files, and modal index exports across the console-app, dev-console, and knative-plugin packages, plus updates to public modal utilities and factory code. 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/packages/console-app/src/components/pdb/modals/DeletePDBModal.tsx (1)
26-41: Spinner persists indefinitely on error.When
k8sKillfails,setIsSubmitting(true)is never reset tofalse. The user sees an endless spinner alongside the error message, with no way to retry or dismiss cleanly.🐛 Proposed fix
k8sKill(PodDisruptionBudgetModel, pdb) .then(() => { close(); }) .catch((error) => { + setIsSubmitting(false); setSubmitError( error?.message || t('console-app~Unknown error removing PodDisruptionBudget {{pdbName}}.', { pdbName, }), ); });frontend/public/components/modals/delete-pvc-modal.tsx (1)
41-42: Minor typo in comment.Small typo: "resourcce" should be "resource".
📝 Proposed fix
- // Redirect to resourcce list page if the resouce is deleted. + // Redirect to resource list page if the resource is deleted.
🤖 Fix all issues with AI agents
In `@frontend/packages/dev-console/src/actions/context-menu.ts`:
- Around line 24-38: The onSubmit handler passed to deleteResourceModal assumes
application.resources is always an array and calls forEach, which will throw if
undefined; update the onSubmit in the deleteResourceModal call to guard against
missing resources by iterating over (application.resources || []) or using
optional chaining (application.resources?.forEach) so cleanUpWorkload is only
called when resources exist and Promise.all is still returned (for an empty
array when there are no resources).
In `@frontend/packages/knative-plugin/src/actions/creators.ts`:
- Around line 4-5: The import of useOverlay in creators.ts uses an internal
path; update the import to use the public re-export from
`@console/dynamic-plugin-sdk` by replacing the existing import that references
'@console/dynamic-plugin-sdk/src/app/modal-support/useOverlay' with an import
from '@console/dynamic-plugin-sdk' so that the useOverlay symbol is consumed
from the public API surface (ensure any other imports in this file remain
unchanged, and keep LazyDeleteModalProvider import as-is).
- Around line 120-153: Add the missing ESLint disable comment before the
dependency array in the useDeleteKnativeServiceResource hook: place the line "//
eslint-disable-next-line react-hooks/exhaustive-deps" immediately above the
dependency array that currently lists [kind, obj, serviceTypeValue,
serviceCreatedFromWebFlag, launchModal] so the hook matches the pattern used by
useSetTrafficDistributionAction, useMoveSinkPubsubAction, and
useDeleteRevisionAction.
🧹 Nitpick comments (8)
frontend/packages/knative-plugin/src/actions/providers.ts (1)
98-150: Simplify delete action selection to avoid creating unused actions.You can compute the “generated-by” flag once and call
useDeleteKnativeServiceResourcea single time. This reduces duplicate action objects and trims the dependency list.♻️ Suggested refactor
- const deleteKnativeServiceFromWebAction = useDeleteKnativeServiceResource( - kindObj, - resource, - serviceTypeValue, - true, - ); - const deleteKnativeServiceAction = useDeleteKnativeServiceResource( - kindObj, - resource, - serviceTypeValue, - false, - ); + const serviceCreatedFromWebConsole = + resource.metadata.annotations?.['openshift.io/generated-by'] === 'OpenShiftWebConsole'; + const deleteKnativeServiceAction = useDeleteKnativeServiceResource( + kindObj, + resource, + serviceTypeValue, + serviceCreatedFromWebConsole, + ); @@ - ...(resource.metadata.annotations?.['openshift.io/generated-by'] === - 'OpenShiftWebConsole' - ? [deleteKnativeServiceFromWebAction] - : [deleteKnativeServiceAction]), + deleteKnativeServiceAction, @@ - deleteKnativeServiceFromWebAction, - deleteKnativeServiceAction, + deleteKnativeServiceAction,frontend/packages/console-app/src/components/modals/clone/clone-pvc-modal.tsx (1)
11-18: Prefer extractingcloseOverlaybefore passing props.Passing
{...props}last can let caller-suppliedcancel/closeoverride the overlay wiring; extractingcloseOverlaykeeps it explicit and avoids leaking it into the inner modal. If you keep the current ordering, please confirm nolaunchModalcallers passcancel/close.♻️ Proposed refactor
-export const ClonePVCModalProvider: OverlayComponent<ClonePVCModalProps> = (props) => { - return ( - <ModalWrapper blocking onClose={props.closeOverlay}> - <ClonePVCModal cancel={props.closeOverlay} close={props.closeOverlay} {...props} /> - </ModalWrapper> - ); -}; +export const ClonePVCModalProvider: OverlayComponent<ClonePVCModalProps> = ({ + closeOverlay, + ...rest +}) => { + return ( + <ModalWrapper blocking onClose={closeOverlay}> + <ClonePVCModal {...rest} cancel={closeOverlay} close={closeOverlay} /> + </ModalWrapper> + ); +};Also applies to: 279-288
frontend/public/components/modals/rollback-modal.jsx (1)
8-8: Prefer extractingcloseOverlaybefore passing props.Passing
{...props}last can let caller-suppliedcancel/closeoverride the overlay wiring; extractingcloseOverlaykeeps it explicit and avoids leaking it into the inner modal. If you keep the current ordering, please confirm nolaunchModalcallers passcancel/close.♻️ Proposed refactor
-export const RollbackModalProvider = (props) => { - return ( - <ModalWrapper blocking onClose={props.closeOverlay}> - <BaseRollbackModal cancel={props.closeOverlay} close={props.closeOverlay} {...props} /> - </ModalWrapper> - ); -}; +export const RollbackModalProvider = ({ closeOverlay, ...rest }) => { + return ( + <ModalWrapper blocking onClose={closeOverlay}> + <BaseRollbackModal {...rest} cancel={closeOverlay} close={closeOverlay} /> + </ModalWrapper> + ); +};Also applies to: 217-226
frontend/packages/console-app/src/components/modals/resource-limits/index.ts (1)
4-10: Lazy loading correctly configured, though transformation is redundant.The
React.lazywrapper withwebpackChunkNameis correctly set up for code splitting. However, the.then((m) => ({ default: m.default }))transformation is unnecessary—React.lazyalready expects a module with adefaultexport, whichResourceLimitsModalLauncher.tsxprovides directly.This pattern appears intentional for consistency across the migration, so no action required, but for future reference:
♻️ Simplified version (optional)
export const LazyResourceLimitsModalProvider = lazy(() => - import('./ResourceLimitsModalLauncher' /* webpackChunkName: "resource-limits-modal" */).then( - (m) => ({ - default: m.default, - }), - ), + import('./ResourceLimitsModalLauncher' /* webpackChunkName: "resource-limits-modal" */), );frontend/packages/console-app/src/actions/hooks/usePDBActions.ts (1)
101-103: SamelaunchModaldependency concern asuseReplicaSetActions.ts.This follows the same pattern of omitting
launchModalfrom dependencies. For consistency across the codebase, consider addressing this uniformly — either document why it's safe to omit here but not inuseCommonActions.ts, or investigate the root cause of the "max depth exceeded" issue.frontend/packages/console-app/src/actions/hooks/useReplicationControllerActions.ts (1)
98-100: Verify hook dependency suppression remains safe withlaunchModal.
IfuseOverlay()oruseWarningModal()return unstable callbacks, excluding them can create stale closures. Please confirm stability or wrap them (e.g.,useCallback/useEvent) to keep deps accurate without the depth-error.frontend/packages/console-app/src/components/nodes/menu-actions.tsx (1)
73-85: Avoid suppressing hook deps forlaunchModal.
IfuseOverlayis stable, include it in the deps and drop the eslint disable. If it isn’t, consider stabilizing it (or wrapping via a ref) so the memo can safely depend on it without churn.frontend/packages/console-app/src/components/nodes/modals/ConfigureUnschedulableModal.tsx (1)
51-60: Prevent caller props from overriding overlay close handlers.In JSX,
{...props}after explicit prop assignments will override them. SinceModalComponentPropsincludes optionalcancelandclosefields, a caller could pass these props and override thecloseOverlayassignment, leaving the overlay open. While current usage ({ resource: obj }) doesn't trigger this, the provider should defensively control its own lifecycle.Destructure
closeOverlayand spread the rest of props after the explicit handlers:♻️ Suggested adjustment
-export const ConfigureUnschedulableModalProvider: OverlayComponent<ConfigureUnschedulableModalProps> = ( - props, -) => ( - <ModalWrapper blocking onClose={props.closeOverlay}> - <ConfigureUnschedulableModal - cancel={props.closeOverlay} - close={props.closeOverlay} - {...props} - /> - </ModalWrapper> -); +export const ConfigureUnschedulableModalProvider: OverlayComponent<ConfigureUnschedulableModalProps> = ( + props, +) => { + const { closeOverlay, ...rest } = props; + return ( + <ModalWrapper blocking onClose={closeOverlay}> + <ConfigureUnschedulableModal cancel={closeOverlay} close={closeOverlay} {...rest} /> + </ModalWrapper> + ); +};
| return { | ||
| id: 'delete-application', | ||
| label: t('devconsole~Delete application'), | ||
| cta: () => { | ||
| const reqs = []; | ||
| deleteResourceModal({ | ||
| blocking: true, | ||
| resourceName: application.name, | ||
| resourceType: ApplicationModel.label, | ||
| onSubmit: () => { | ||
| application.resources.forEach((resource) => { | ||
| reqs.push(cleanUpWorkload(resource.resource)); | ||
| }); | ||
| return Promise.all(reqs); | ||
| }, |
There was a problem hiding this comment.
Guard against missing application.resources before iterating.
application.resources is treated as optional elsewhere, but forEach will throw if it’s undefined. This can break delete for apps without resources.
✅ Suggested fix
- cta: () => {
- const reqs = [];
- deleteResourceModal({
- blocking: true,
- resourceName: application.name,
- resourceType: ApplicationModel.label,
- onSubmit: () => {
- application.resources.forEach((resource) => {
- reqs.push(cleanUpWorkload(resource.resource));
- });
- return Promise.all(reqs);
- },
- });
- },
+ cta: () => {
+ deleteResourceModal({
+ blocking: true,
+ resourceName: application.name,
+ resourceType: ApplicationModel.label,
+ onSubmit: () => {
+ const resources = application.resources ?? [];
+ return Promise.all(
+ resources.map((resource) => cleanUpWorkload(resource.resource)),
+ );
+ },
+ });
+ },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return { | |
| id: 'delete-application', | |
| label: t('devconsole~Delete application'), | |
| cta: () => { | |
| const reqs = []; | |
| deleteResourceModal({ | |
| blocking: true, | |
| resourceName: application.name, | |
| resourceType: ApplicationModel.label, | |
| onSubmit: () => { | |
| application.resources.forEach((resource) => { | |
| reqs.push(cleanUpWorkload(resource.resource)); | |
| }); | |
| return Promise.all(reqs); | |
| }, | |
| return { | |
| id: 'delete-application', | |
| label: t('devconsole~Delete application'), | |
| cta: () => { | |
| deleteResourceModal({ | |
| blocking: true, | |
| resourceName: application.name, | |
| resourceType: ApplicationModel.label, | |
| onSubmit: () => { | |
| const resources = application.resources ?? []; | |
| return Promise.all( | |
| resources.map((resource) => cleanUpWorkload(resource.resource)), | |
| ); | |
| }, | |
| }); | |
| }, |
🤖 Prompt for AI Agents
In `@frontend/packages/dev-console/src/actions/context-menu.ts` around lines 24 -
38, The onSubmit handler passed to deleteResourceModal assumes
application.resources is always an array and calls forEach, which will throw if
undefined; update the onSubmit in the deleteResourceModal call to guard against
missing resources by iterating over (application.resources || []) or using
optional chaining (application.resources?.forEach) so cleanUpWorkload is only
called when resources exist and Promise.all is still returned (for an empty
array when there are no resources).
| import { useOverlay } from '@console/dynamic-plugin-sdk/src/app/modal-support/useOverlay'; | ||
| import { LazyDeleteModalProvider } from '@console/internal/components/modals'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the main entry point of dynamic-plugin-sdk and check exports
echo "=== Checking dynamic-plugin-sdk package structure ==="
fd -t f "package.json" | grep "console-dynamic-plugin-sdk" | head -5
# Find the index or main entry files
echo "=== Looking for index files in dynamic-plugin-sdk ==="
find . -path "*/console-dynamic-plugin-sdk/src/*" -name "index.ts" -o -name "index.js" | head -20Repository: openshift/console
Length of output: 178
🏁 Script executed:
#!/bin/bash
# Search for useOverlay exports in the dynamic-plugin-sdk
echo "=== Searching for useOverlay exports ==="
rg "export.*useOverlay" frontend/packages/console-dynamic-plugin-sdk/src/ -n --type ts
# Also check the specific modal-support directory
echo "=== Checking modal-support directory ==="
find frontend/packages/console-dynamic-plugin-sdk/src/app/modal-support -type f -name "*.ts*" 2>/dev/null | head -20Repository: openshift/console
Length of output: 751
🏁 Script executed:
#!/bin/bash
# Check how other files in knative-plugin import from dynamic-plugin-sdk
echo "=== Checking import patterns in knative-plugin ==="
rg "from '@console/dynamic-plugin-sdk" frontend/packages/knative-plugin/src/ -n --type ts | head -30Repository: openshift/console
Length of output: 5332
Import useOverlay from the public API surface.
useOverlay is re-exported from @console/dynamic-plugin-sdk in the public API (dynamic-core-api.ts). Import it from @console/dynamic-plugin-sdk directly, consistent with other public hooks like useActivePerspective, rather than reaching into the internal /src/ path.
Change:
import { useOverlay } from '@console/dynamic-plugin-sdk/src/app/modal-support/useOverlay';
to:
import { useOverlay } from '@console/dynamic-plugin-sdk';
🤖 Prompt for AI Agents
In `@frontend/packages/knative-plugin/src/actions/creators.ts` around lines 4 - 5,
The import of useOverlay in creators.ts uses an internal path; update the import
to use the public re-export from `@console/dynamic-plugin-sdk` by replacing the
existing import that references
'@console/dynamic-plugin-sdk/src/app/modal-support/useOverlay' with an import
from '@console/dynamic-plugin-sdk' so that the useOverlay symbol is consumed
from the public API surface (ensure any other imports in this file remain
unchanged, and keep LazyDeleteModalProvider import as-is).
| export const useDeleteKnativeServiceResource = ( | ||
| kind: K8sKind, | ||
| obj: K8sResourceKind, | ||
| serviceTypeValue: ServiceTypeValue, | ||
| serviceCreatedFromWebFlag: boolean, | ||
| ): Action => ({ | ||
| id: `delete-resource`, | ||
| label: | ||
| serviceTypeValue === ServiceTypeValue.Function | ||
| ? i18next.t('knative-plugin~Delete Function') | ||
| : i18next.t('knative-plugin~Delete Service'), | ||
| cta: () => | ||
| deleteModal( | ||
| serviceCreatedFromWebFlag | ||
| ? { | ||
| kind, | ||
| resource: obj, | ||
| deleteAllResources: () => cleanUpWorkload(obj), | ||
| } | ||
| : { | ||
| kind, | ||
| resource: obj, | ||
| }, | ||
| ), | ||
| accessReview: asAccessReview(kind, obj, 'delete'), | ||
| }); | ||
| ): Action => { | ||
| const launchModal = useOverlay(); | ||
|
|
||
| return useMemo<Action>( | ||
| () => ({ | ||
| id: `delete-resource`, | ||
| label: | ||
| serviceTypeValue === ServiceTypeValue.Function | ||
| ? i18next.t('knative-plugin~Delete Function') | ||
| : i18next.t('knative-plugin~Delete Service'), | ||
| cta: () => | ||
| launchModal( | ||
| LazyDeleteModalProvider, | ||
| serviceCreatedFromWebFlag | ||
| ? { | ||
| kind, | ||
| resource: obj, | ||
| deleteAllResources: () => cleanUpWorkload(obj), | ||
| } | ||
| : { | ||
| kind, | ||
| resource: obj, | ||
| }, | ||
| ), | ||
| accessReview: asAccessReview(kind, obj, 'delete'), | ||
| }), | ||
| [kind, obj, serviceTypeValue, serviceCreatedFromWebFlag, launchModal], | ||
| ); | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for old function name - should find zero results if rename is complete
echo "=== Searching for old function name 'deleteKnativeServiceResource' ==="
rg -n "deleteKnativeServiceResource[^_]" --type ts frontend/ || echo "No matches found (as expected)"
echo ""
echo "=== Searching for new hook name usage ==="
rg -n "useDeleteKnativeServiceResource" --type ts frontend/ -A 2
echo ""
echo "=== Checking dependency arrays in related hooks ==="
echo "Checking useSetTrafficDistributionAction:"
rg -n "useSetTrafficDistributionAction" --type ts frontend/ -A 30 | head -50
echo ""
echo "Checking useDeleteRevisionAction:"
rg -n "useDeleteRevisionAction" --type ts frontend/ -A 30 | head -50Repository: openshift/console
Length of output: 10992
🏁 Script executed:
#!/bin/bash
# Now let's examine the actual dependency arrays in the creators.ts file
echo "=== Viewing the creators.ts file with focus on dependency arrays ==="
cat -n frontend/packages/knative-plugin/src/actions/creators.ts | sed -n '1,80p'
echo ""
echo "=== Viewing useSetTrafficDistributionAction dependency array ==="
rg -n "useSetTrafficDistributionAction" --type ts frontend/packages/knative-plugin/src/actions/creators.ts -A 35 | head -50
echo ""
echo "=== Viewing useDeleteRevisionAction dependency array ==="
rg -n "useDeleteRevisionAction" --type ts frontend/packages/knative-plugin/src/actions/creators.ts -A 35 | head -50
echo ""
echo "=== Viewing useDeleteKnativeServiceResource dependency array (full function) ==="
rg -n "useDeleteKnativeServiceResource" --type ts frontend/packages/knative-plugin/src/actions/creators.ts -A 35Repository: openshift/console
Length of output: 8489
Rename is complete—add the missing eslint-disable comment for consistency.
The migration to useDeleteKnativeServiceResource is properly implemented with both call sites in useKnativeServiceActionsProvider correctly updated. However, this function is missing the // eslint-disable-next-line react-hooks/exhaustive-deps comment that appears on every other action hook creator in this file (useSetTrafficDistributionAction, useMoveSinkPubsubAction, useDeleteRevisionAction, etc.). Add the disable comment before the dependency array on line 151 to align with the established pattern:
),
// eslint-disable-next-line react-hooks/exhaustive-deps
[kind, obj, serviceTypeValue, serviceCreatedFromWebFlag, launchModal],🤖 Prompt for AI Agents
In `@frontend/packages/knative-plugin/src/actions/creators.ts` around lines 120 -
153, Add the missing ESLint disable comment before the dependency array in the
useDeleteKnativeServiceResource hook: place the line "//
eslint-disable-next-line react-hooks/exhaustive-deps" immediately above the
dependency array that currently lists [kind, obj, serviceTypeValue,
serviceCreatedFromWebFlag, launchModal] so the hook matches the pattern used by
useSetTrafficDistributionAction, useMoveSinkPubsubAction, and
useDeleteRevisionAction.
41c0ea6 to
cc483ff
Compare
Migrates PVC-related modals (expand, clone, delete, restore) from the legacy createModalLauncher pattern to the modern OverlayComponent pattern with lazy loading. Changes include: - Convert modal exports to OverlayComponent providers - Implement lazy loading for modal components - Update action hooks to use useOverlay() and launchModal() - Set react-modal app element globally in App component - Maintain backward compatibility with existing modal exports Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit completes the migration from createModalLauncher to the OverlayComponent pattern for delete modals and related actions. Changes: - Refactored delete-modal.tsx to use OverlayComponent pattern - Added LazyDeleteModalProvider with React.lazy() for code splitting - Converted action creators to hooks to comply with React rules: * DeleteResourceAction → useDeleteResourceAction (context-menu.ts) * deleteKnativeServiceResource → useDeleteKnativeServiceResource (creators.ts) - Updated all consumers to use new hook-based actions: * useCommonActions - uses LazyDeleteModalProvider with launchModal * deployment-provider - uses useDeleteResourceAction hook * deploymentconfig-provider - uses useDeleteResourceAction hook * knative-plugin providers - uses useDeleteKnativeServiceResource hook - Updated public/components/modals/index.ts with lazy export - Eliminated all deleteModal() function calls Note: DeleteApplicationAction remains as a non-hook function creator since it doesn't use modals and doesn't need to comply with hooks rules. This change maintains backward compatibility through default exports and preserves lazy loading functionality. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Remove launchModal from dependency arrays in pre-existing hooks to prevent infinite loops. These files were not modified as part of the modal migration work, but had the same issue. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit refactors configure-update-strategy-modal to use the OverlayComponent pattern, eliminating createModalLauncher. Changes: - Refactored configure-update-strategy-modal.tsx to use OverlayComponent - Added ConfigureUpdateStrategyModalProvider with ModalWrapper - Added LazyConfigureUpdateStrategyModalProvider with React.lazy() - Updated useDeploymentActions to use the new lazy provider - Removed deprecated configureUpdateStrategyModal from index.ts This change maintains backward compatibility through default exports and preserves lazy loading functionality. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit migrates the configure namespace pull secret modal from the legacy createModalLauncher pattern to the new OverlayComponent pattern. Changes: - Exported ConfigureNamespacePullSecretModalProvider as OverlayComponent - Updated index.ts to lazy-load the modal provider - Modified namespace.jsx to use useOverlay hook instead of direct modal call Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…tern This commit migrates the configure cluster upstream modal from the legacy createModalLauncher pattern to the new OverlayComponent pattern and fixes react-modal accessibility warnings. Changes: - Exported ConfigureClusterUpstreamModalProvider as OverlayComponent - Updated index.ts to lazy-load the modal provider - Modified details-page.tsx to use useOverlay hook instead of direct modal call - Removed unnecessary TFunction prop from modal type definition - Fixed react-modal warnings by using useLayoutEffect for global app element setup - Added explicit appElement prop to ModalWrapper for robust timing handling Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit migrates the managed resource save modal from the legacy createModalLauncher pattern to the new OverlayComponent pattern. Changes: - Exported ManagedResourceSaveModalProvider as OverlayComponent - Updated index.ts to lazy-load the modal provider - Modified edit-yaml.tsx to use useOverlay hook instead of direct modal call - Removed unnecessary 'kind' prop from modal invocation - Updated type definition to extend ModalComponentProps Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
The remove-user-modal was already replaced by useWarningModal in group.tsx and is no longer used anywhere in the codebase. Changes: - Deleted remove-user-modal.tsx - Removed LazyRemoveUserModalProvider export from index.ts Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit migrates the ConfigureUnschedulableModal from the legacy createModalLauncher pattern to the new OverlayComponent pattern. Changes: - Exported ConfigureUnschedulableModalProvider as OverlayComponent - Updated modals/index.ts to lazy-load the modal provider - Modified useNodeActions hook to use useOverlay hook - Removed createModalLauncher usage Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
rhamilto
force-pushed
the
CONSOLE-5012-3
branch
from
cc483ff to
b136855
Compare
|
@rhamilto: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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. |
2 participants
Summary
This PR continues the migration of modals from the legacy
createModalLauncherpattern to the newOverlayComponentpattern using theuseOverlayhook.Note: This PR builds on:
This PR includes only the following new modal migrations:
New Modals Migrated (3)
ConfigureUnschedulableModal - Used for marking nodes as unschedulable
packages/console-app/src/components/nodes/modals/ConfigureUnschedulableModal.tsxpackages/console-app/src/components/nodes/menu-actions.tsxDeletePDBModal - Used for removing PodDisruptionBudgets from workloads
packages/console-app/src/components/pdb/modals/DeletePDBModal.tsxpackages/console-app/src/actions/hooks/usePDBActions.tsResourceLimitsModal - Used for editing resource limits on deployments
packages/console-app/src/components/modals/resource-limits/ResourceLimitsModalLauncher.tsxpackages/console-app/src/actions/hooks/useDeploymentActions.tsChanges Made
For each modal:
createModalLauncherusageOverlayComponentandModalWrapperimports[Name]Providerwrapping content inModalWrapperReact.lazy()for code splittinguseOverlay()hook withlaunchModal()launchModaldependencies where needed🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.