CONSOLE-5049: Migrate OLM modals to overlay pattern #15968
Conversation
Remove React Router wrapper from taints modal to unblock Router v7 upgrade. Part of CONSOLE-5012 to eliminate all createModalLauncher usage. Assisted by: Claude Code
Converts the modal to use the new overlay pattern, replacing createModalLauncher with useOverlay hook. Assisted by: Claude Code
openshift-ci-robot
added
the
jira/valid-reference
|
@sg00dwin: This pull request references CONSOLE-5049 which is a valid jira issue. 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 NOT APPROVED This pull-request has been approved by: sg00dwin The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
@sg00dwin: This pull request references CONSOLE-5049 which is a valid jira issue. 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 modal management across OpenShift Console from a createModalLauncher-based pattern to an overlay-based provider pattern. Modal files export OverlayComponent-wrapped providers instead of launcher functions. Consumer code uses the useOverlay hook to launch these providers with appropriate props. Updated files include modals for taints, subscriptions, install plans, resource requirements, update strategy, and related components. Props types are restructured to match provider requirements. Test files are updated to mock useOverlay and useK8sModel hooks. Public API signatures change from exported launcher functions to exported provider components and types. 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 🧪 Unit Test Generation v2 is now available!We have significantly improved our unit test generation capabilities. To enable: Add this to your reviews:
finishing_touches:
unit_tests:
enabled: trueTry it out by using the Have feedback? Share your thoughts on our Discord thread! 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/packages/operator-lifecycle-manager/src/components/modals/uninstall-operator-modal.tsx (2)
657-665:⚠️ Potential issue | 🟡 MinorRemove unused
k8sGetandk8sPatchprops from the modal type and provider spread.The modal imports
k8sGetResource(line 16) andk8sPatch(line 43) directly from their respective modules and uses those in the implementation (lines 144, 176), never touching the passed-in props. These props are defined inUninstallOperatorModalProps(lines 683–684) but the modal component doesn't destructure them (lines 65–71), yet they're still being spread from the provider (line 662). This adds unnecessary coupling and type noise without providing any value.Remove
k8sGetandk8sPatchfromUninstallOperatorModalPropsandUninstallOperatorModalProviderPropstype definitions, then strip them from the spread inUninstallOperatorModalProviderto keep the API surface clean.
434-444:⚠️ Potential issue | 🟡 MinorRemove unused
UninstallOperatorOverlayexport.This component is never imported or referenced anywhere in the codebase—all usage has migrated to
UninstallOperatorModalProviderwithModalWrapper. Additionally, line 440 has an invalid trailing semicolon in the JSX that should be removed if you keep this code during the refactor.Consider deleting
UninstallOperatorOverlayentirely (lines 434–444) to eliminate dead code and avoid confusion about which modal pattern to use.
🤖 Fix all issues with AI agents
In
`@frontend/packages/operator-lifecycle-manager/src/components/descriptors/spec/resource-requirements.tsx`:
- Around line 156-191: The overlay is launched before the K8s model finishes
loading; update ResourceRequirementsModalLink to destructure the loading flag
from useK8sModel (e.g., const [model, inFlight] = useK8sModel(...)), prevent
launchOverlay in onClick when inFlight is true (early return) and pass a
disabled prop to the Button (and optionally a tooltip or aria-busy) while
inFlight so the modal cannot be opened until the model is ready.
In
`@frontend/packages/operator-lifecycle-manager/src/components/modals/uninstall-operator-modal.tsx`:
- Around line 667-677: UninstallOperatorModalProviderProps declares k8sGet and
k8sPatch but UninstallOperatorModal doesn't use them (it imports k8sPatch from
`@console/internal/module/k8s` and uses k8sGetResource elsewhere), so either
remove k8sGet and k8sPatch from UninstallOperatorModalProviderProps (and
corresponding UninstallOperatorModalProps) to avoid dead API surface, or
refactor UninstallOperatorModal to accept and use the injected props
(k8sGet/k8sPatch) instead of importing k8sPatch directly; update all
consumers/tests to pass the props if you choose injection, or remove the unused
prop declarations if you choose cleanup.
In
`@frontend/packages/operator-lifecycle-manager/src/components/subscription.tsx`:
- Around line 431-439: The code calls launcher(UninstallOperatorModalProvider,
...) during render which is a side effect; move this logic into a React
useEffect hook so it runs after render when the URL contains showDelete.
Specifically, create a useEffect that checks new
URLSearchParams(window.location.search).has('showDelete'), calls
launcher(UninstallOperatorModalProvider, { k8sKill, k8sGet, k8sPatch,
subscription: obj }), then calls removeQueryArgument('showDelete'); include
k8sKill, k8sGet, k8sPatch, obj (subscription) in the effect dependencies as
appropriate (or an empty array if you only want it once) to avoid repeated
launches on rerenders.
In `@frontend/public/components/modals/index.ts`:
- Around line 68-71: The exported taintsModal currently calls
m.TaintsModalProvider(props) which invokes the overlay component instead of
returning a launcher; either remove this vestigial export if unused, or change
it to the overlay-launcher pattern used by tolerationsModal: dynamically import
TaintsModalProvider but return a launcher (e.g., via createModalLauncher or a
function that calls launchOverlay with the component and closeOverlay) so the
component is passed to launchOverlay/useOverlay rather than invoked directly;
update references to taintsModal to use the new launcher or delete the export if
no consumers remain.
🧹 Nitpick comments (1)
frontend/packages/operator-lifecycle-manager/src/components/modals/subscription-channel-modal.tsx (1)
116-121: Consider documenting thecloseOverlayinjection more explicitly in the type.The comment is helpful, but for better TypeScript ergonomics, you might consider extending the props type to explicitly include
closeOverlayeven though it's auto-injected. This would improve IDE autocomplete for consumers and make the contract clearer.type SubscriptionChannelModalProviderProps = { subscription: SubscriptionKind; pkg: PackageManifestKind; k8sUpdate: (kind: K8sKind, newObj: K8sResourceKind) => Promise<any>; } & { closeOverlay?: () => void }; // injected by OverlayComponentThat said, if
OverlayComponent<T>already handles this via its generic constraint, the current approach is acceptable.
| export const ResourceRequirementsModalLink: FC<ResourceRequirementsModalLinkProps> = (props) => { | ||
| const { obj, type, path } = props; | ||
| const { t } = useTranslation(); | ||
| const launchOverlay = useOverlay(); | ||
| const [model] = useK8sModel(referenceFor(obj)); | ||
| const none = t('public~None'); | ||
| const { cpu, memory, 'ephemeral-storage': storage } = _.get(obj.spec, `${path}.${type}`, {}); | ||
|
|
||
| const onClick = () => { | ||
| const modal = createModalLauncher(ResourceRequirementsModal); | ||
| const description = t('olm~Define the resource {{type}} for this {{kind}} instance.', { | ||
| type, | ||
| kind: obj.kind, | ||
| }); | ||
| const title = t('olm~{{kind}} Resource {{type}}', { | ||
| kind: obj.kind, | ||
| type: _.capitalize(type), | ||
| }); | ||
| const onClick = () => { | ||
| const description = t('olm~Define the resource {{type}} for this {{kind}} instance.', { | ||
| type, | ||
| kind: obj.kind, | ||
| }); | ||
| const title = t('olm~{{kind}} Resource {{type}}', { | ||
| kind: obj.kind, | ||
| type: _.capitalize(type), | ||
| }); | ||
|
|
||
| return modal({ title, description, obj, model, type, path }); | ||
| }; | ||
| launchOverlay(ResourceRequirementsModalProvider, { | ||
| title, | ||
| description, | ||
| obj, | ||
| model, | ||
| type, | ||
| path, | ||
| }); | ||
| }; | ||
|
|
||
| return ( | ||
| <Button | ||
| icon={<PencilAltIcon />} | ||
| iconPosition="end" | ||
| type="button" | ||
| isInline | ||
| data-test-id="configure-modal-btn" | ||
| onClick={onClick} | ||
| variant="link" | ||
| > | ||
| {t('olm~CPU: {{cpu}}, Memory: {{memory}}, Storage: {{storage}}', { | ||
| cpu: cpu || none, | ||
| memory: memory || none, | ||
| storage: storage || none, | ||
| })} | ||
| </Button> | ||
| ); | ||
| }, | ||
| ); | ||
| return ( | ||
| <Button | ||
| icon={<PencilAltIcon />} | ||
| iconPosition="end" | ||
| type="button" | ||
| isInline | ||
| data-test-id="configure-modal-btn" | ||
| onClick={onClick} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find useK8sModel hook definition
fd -e ts -e tsx -path '*/hooks/*' | xargs grep -l "useK8sModel" | head -5Repository: openshift/console
Length of output: 231
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for useK8sModel export/definition
rg "export.*useK8sModel|function useK8sModel" -A 20 -B 2Repository: openshift/console
Length of output: 2556
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check the specific file to see actual usage
cat -n frontend/packages/operator-lifecycle-manager/src/components/descriptors/spec/resource-requirements.tsx | sed -n '156,191p'Repository: openshift/console
Length of output: 1306
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Get the complete useK8sModel implementation
cat -n frontend/packages/console-dynamic-plugin-sdk/src/utils/k8s/hooks/useK8sModel.tsRepository: openshift/console
Length of output: 1958
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check the type definition for UseK8sModel
rg "type UseK8sModel|interface UseK8sModel" -A 5 -B 2Repository: openshift/console
Length of output: 1436
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check if getK8sModel can return undefined
rg "getK8sModel" -A 10 | head -40Repository: openshift/console
Length of output: 2750
Guard overlay launch until the model is loaded.
useK8sModel returns [K8sModel, inFlight] where the second element indicates loading state. Launching the modal while inFlight is true risks passing an undefined model to k8sUpdate. Gate the overlay launch and disable the button until the model is ready.
🔧 Suggested fix
- const [model] = useK8sModel(referenceFor(obj));
+ const [model, inFlight] = useK8sModel(referenceFor(obj));
@@
const onClick = () => {
+ if (inFlight || !model) {
+ return;
+ }
const description = t('olm~Define the resource {{type}} for this {{kind}} instance.', {
@@
<Button
icon={<PencilAltIcon />}
iconPosition="end"
type="button"
isInline
data-test-id="configure-modal-btn"
+ isDisabled={inFlight || !model}
onClick={onClick}🤖 Prompt for AI Agents
In
`@frontend/packages/operator-lifecycle-manager/src/components/descriptors/spec/resource-requirements.tsx`
around lines 156 - 191, The overlay is launched before the K8s model finishes
loading; update ResourceRequirementsModalLink to destructure the loading flag
from useK8sModel (e.g., const [model, inFlight] = useK8sModel(...)), prevent
launchOverlay in onClick when inFlight is true (early return) and pass a
disabled prop to the Button (and optionally a tooltip or aria-busy) while
inFlight so the modal cannot be opened until the model is ready.
| type UninstallOperatorModalProviderProps = { | ||
| k8sKill: (kind: K8sKind, resource: K8sResourceKind, options: any, json: any) => Promise<any>; | ||
| k8sGet: (kind: K8sKind, name: string, namespace: string) => Promise<K8sResourceKind>; | ||
| k8sPatch: ( | ||
| kind: K8sKind, | ||
| resource: K8sResourceKind, | ||
| data: { op: string; path: string; value: any }[], | ||
| ) => Promise<any>; | ||
| subscription: K8sResourceKind; | ||
| csv?: K8sResourceKind; | ||
| }; |
There was a problem hiding this comment.
Props type includes k8sGet and k8sPatch but these appear unused by the modal.
The UninstallOperatorModalProviderProps defines k8sGet and k8sPatch, but the UninstallOperatorModal component doesn't destructure these from props. The modal imports k8sPatch directly from @console/internal/module/k8s (line 43) and uses k8sGetResource from the dynamic-plugin-sdk (line 16).
If these props are intended for future testability/mocking, consider either:
- Actually using the props in the modal instead of direct imports
- Removing them from the props types if they're not needed
This is a minor inconsistency but could cause confusion for future maintainers.
🔧 Option 1: Remove unused props
type UninstallOperatorModalProviderProps = {
k8sKill: (kind: K8sKind, resource: K8sResourceKind, options: any, json: any) => Promise<any>;
- k8sGet: (kind: K8sKind, name: string, namespace: string) => Promise<K8sResourceKind>;
- k8sPatch: (
- kind: K8sKind,
- resource: K8sResourceKind,
- data: { op: string; path: string; value: any }[],
- ) => Promise<any>;
subscription: K8sResourceKind;
csv?: K8sResourceKind;
};And similarly for UninstallOperatorModalProps.
📝 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.
| type UninstallOperatorModalProviderProps = { | |
| k8sKill: (kind: K8sKind, resource: K8sResourceKind, options: any, json: any) => Promise<any>; | |
| k8sGet: (kind: K8sKind, name: string, namespace: string) => Promise<K8sResourceKind>; | |
| k8sPatch: ( | |
| kind: K8sKind, | |
| resource: K8sResourceKind, | |
| data: { op: string; path: string; value: any }[], | |
| ) => Promise<any>; | |
| subscription: K8sResourceKind; | |
| csv?: K8sResourceKind; | |
| }; | |
| type UninstallOperatorModalProviderProps = { | |
| k8sKill: (kind: K8sKind, resource: K8sResourceKind, options: any, json: any) => Promise<any>; | |
| subscription: K8sResourceKind; | |
| csv?: K8sResourceKind; | |
| }; |
🤖 Prompt for AI Agents
In
`@frontend/packages/operator-lifecycle-manager/src/components/modals/uninstall-operator-modal.tsx`
around lines 667 - 677, UninstallOperatorModalProviderProps declares k8sGet and
k8sPatch but UninstallOperatorModal doesn't use them (it imports k8sPatch from
`@console/internal/module/k8s` and uses k8sGetResource elsewhere), so either
remove k8sGet and k8sPatch from UninstallOperatorModalProviderProps (and
corresponding UninstallOperatorModalProps) to avoid dead API surface, or
refactor UninstallOperatorModal to accept and use the injected props
(k8sGet/k8sPatch) instead of importing k8sPatch directly; update all
consumers/tests to pass the props if you choose injection, or remove the unused
prop declarations if you choose cleanup.
| if (new URLSearchParams(window.location.search).has('showDelete')) { | ||
| uninstallOperatorModal(); | ||
| launcher(UninstallOperatorModalProvider, { | ||
| k8sKill, | ||
| k8sGet, | ||
| k8sPatch, | ||
| subscription: obj, | ||
| }); | ||
| removeQueryArgument('showDelete'); | ||
| } |
There was a problem hiding this comment.
Invoking overlay launcher during render is a side effect—move to useEffect.
Calling launcher() directly in the component body (outside of an event handler or useEffect) executes during render, which violates React's rules against side effects in render. If a re-render occurs, this could launch the modal multiple times or cause unpredictable behavior.
Wrap this logic in a useEffect to handle the query param trigger cleanly:
Proposed fix
+ useEffect(() => {
+ if (new URLSearchParams(window.location.search).has('showDelete')) {
+ launcher(UninstallOperatorModalProvider, {
+ k8sKill,
+ k8sGet,
+ k8sPatch,
+ subscription: obj,
+ });
+ removeQueryArgument('showDelete');
+ }
+ }, [launcher, obj]);
+
- if (new URLSearchParams(window.location.search).has('showDelete')) {
- launcher(UninstallOperatorModalProvider, {
- k8sKill,
- k8sGet,
- k8sPatch,
- subscription: obj,
- });
- removeQueryArgument('showDelete');
- }📝 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.
| if (new URLSearchParams(window.location.search).has('showDelete')) { | |
| uninstallOperatorModal(); | |
| launcher(UninstallOperatorModalProvider, { | |
| k8sKill, | |
| k8sGet, | |
| k8sPatch, | |
| subscription: obj, | |
| }); | |
| removeQueryArgument('showDelete'); | |
| } | |
| useEffect(() => { | |
| if (new URLSearchParams(window.location.search).has('showDelete')) { | |
| launcher(UninstallOperatorModalProvider, { | |
| k8sKill, | |
| k8sGet, | |
| k8sPatch, | |
| subscription: obj, | |
| }); | |
| removeQueryArgument('showDelete'); | |
| } | |
| }, [launcher, obj]); |
🤖 Prompt for AI Agents
In `@frontend/packages/operator-lifecycle-manager/src/components/subscription.tsx`
around lines 431 - 439, The code calls launcher(UninstallOperatorModalProvider,
...) during render which is a side effect; move this logic into a React
useEffect hook so it runs after render when the URL contains showDelete.
Specifically, create a useEffect that checks new
URLSearchParams(window.location.search).has('showDelete'), calls
launcher(UninstallOperatorModalProvider, { k8sKill, k8sGet, k8sPatch,
subscription: obj }), then calls removeQueryArgument('showDelete'); include
k8sKill, k8sGet, k8sPatch, obj (subscription) in the effect dependencies as
appropriate (or an empty array if you only want it once) to avoid repeated
launches on rerenders.
| export const taintsModal = (props) => | ||
| import('./taints-modal' /* webpackChunkName: "taints-modal" */).then((m) => m.taintsModal(props)); | ||
| import('./taints-modal' /* webpackChunkName: "taints-modal" */).then((m) => | ||
| m.TaintsModalProvider(props), | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate the taints-modal implementation
find . -name "taints-modal*" -type f | head -20Repository: openshift/console
Length of output: 113
🏁 Script executed:
# Check the taints-modal implementation
cat -n frontend/public/components/modals/taints-modal.tsx | head -100Repository: openshift/console
Length of output: 3986
🏁 Script executed:
# Search for consumers of taintsModal from the barrel export
rg -n "from ['\"].*components/modals['\"]" --type=ts --type=tsx | head -30Repository: openshift/console
Length of output: 88
🏁 Script executed:
# Search for direct taintsModal usage patterns
rg -n "taintsModal" --type=ts --type=tsx | grep -v "index.ts" | grep -v "taints-modal.tsx"Repository: openshift/console
Length of output: 88
🏁 Script executed:
# Check the modals index.ts for context around the taintsModal export
cat -n frontend/public/components/modals/index.ts | sed -n '60,75p'Repository: openshift/console
Length of output: 808
🏁 Script executed:
# Search for TaintsModalProvider export in taints-modal.tsx
rg -n "TaintsModalProvider" frontend/public/components/modals/taints-modal.tsxRepository: openshift/console
Length of output: 223
🏁 Script executed:
# Search for consumers of taintsModal from the barrel export
rg -n "from.*modals['\"]" --type=ts | grep -v index.tsRepository: openshift/console
Length of output: 3512
🏁 Script executed:
# Check what's exported from taints-modal
rg -n "export" frontend/public/components/modals/taints-modal.tsxRepository: openshift/console
Length of output: 133
🏁 Script executed:
# Compare with other modal patterns - check tolerations-modal for pattern
rg -n "export|Modal" frontend/public/components/modals/tolerations-modal.tsx | head -30Repository: openshift/console
Length of output: 672
🏁 Script executed:
# Check if there are actual direct usages of taintsModal pattern
rg -n "taintsModal\(" --type=tsRepository: openshift/console
Length of output: 43
🏁 Script executed:
# Search specifically for taintsModal imports
rg -n "taintsModal" --type=ts --type=jsRepository: openshift/console
Length of output: 144
🏁 Script executed:
# Check what OverlayComponent actually is
rg -n "type OverlayComponent|interface OverlayComponent" --type=tsRepository: openshift/console
Length of output: 238
🏁 Script executed:
# Get context around the OverlayComponent definition
rg -B5 -A10 "type OverlayComponent" --type=tsRepository: openshift/console
Length of output: 1895
🏁 Script executed:
# Check if there are any recent commits changing this pattern
git log --oneline --all -- frontend/public/components/modals/taints-modal.tsx | head -5Repository: openshift/console
Length of output: 367
Remove or update this barrel export—it's using an incorrect pattern that doesn't work with the overlay system.
The current export calls m.TaintsModalProvider(props) directly, which invokes the OverlayComponent as a function. This returns unreachable JSX instead of mounting the modal via the overlay system. The component expects to be passed to launchOverlay() with the closeOverlay callback, not invoked standalone.
However, searching the codebase shows no active consumers of this taintsModal export, suggesting it's vestigial from the overlay migration. Either remove this export if all consumers have migrated to launchOverlay/useOverlay, or update it to match the proper pattern (like tolerationsModal still using createModalLauncher).
🤖 Prompt for AI Agents
In `@frontend/public/components/modals/index.ts` around lines 68 - 71, The
exported taintsModal currently calls m.TaintsModalProvider(props) which invokes
the overlay component instead of returning a launcher; either remove this
vestigial export if unused, or change it to the overlay-launcher pattern used by
tolerationsModal: dynamically import TaintsModalProvider but return a launcher
(e.g., via createModalLauncher or a function that calls launchOverlay with the
component and closeOverlay) so the component is passed to
launchOverlay/useOverlay rather than invoked directly; update references to
taintsModal to use the new launcher or delete the export if no consumers remain.
|
@sg00dwin: 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. |
|
PR needs rebase. 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. |
openshift-merge-robot
added
the
needs-rebase
4 participants
Description
Migrates OLM and related modals from the deprecated
createModalLauncherpattern to the modernOverlayComponentpattern. This is part of CONSOLE-5012 to modernize modal handling across the console.Changes
Modals Migrated (8 total)
All modals now follow the
OverlayComponentpattern with consistent implementation:TaintsModal (
public/components/modals/taints-modal.tsx)TaintsModalProvider: OverlayComponentcreateModalLauncherusageUpdateStrategyModal (
packages/operator-lifecycle-manager/src/components/modals/update-strategy-modal.tsx)UpdateStrategyModalProvider: OverlayComponentcreateModalLauncherusageEditDefaultSourcesModal (
packages/operator-lifecycle-manager/src/components/modals/edit-default-sources-modal.tsx)EditDefaultSourcesModalProvider: OverlayComponentcreateModalLauncherusageDisableDefaultSourceModal (
packages/operator-lifecycle-manager/src/components/modals/disable-default-source-modal.tsx)DisableDefaultSourceModalProvider: OverlayComponentcreateModalLauncherusageInstallPlanApprovalModal (
packages/operator-lifecycle-manager/src/components/modals/installplan-approval-modal.tsx)InstallPlanApprovalModalProvider: OverlayComponentcreateModalLauncherusageSubscriptionChannelModal (
packages/operator-lifecycle-manager/src/components/modals/subscription-channel-modal.tsx)SubscriptionChannelModalProvider: OverlayComponentcreateModalLauncherusageInstallPlanPreviewModal (
packages/operator-lifecycle-manager/src/components/modals/installplan-preview-modal.tsx)InstallPlanPreviewModalProvider: OverlayComponentcreateModalLauncherusageUninstallOperatorModal (
packages/operator-lifecycle-manager/src/components/modals/uninstall-operator-modal.tsx)UninstallOperatorModalProvider: OverlayComponentcreateModalLauncherusageUpdated to Use New Overlay Modals
Replaced direct modal function calls with
useOverlay():useCommonActions.ts: Changed fromtaintsModaltolaunchOverlay(TaintsModalProvider, ...)useOperatorActions.ts: Changed to uselauncher(UninstallOperatorModalProvider, ...)in action ctauseSubscriptionActions.ts: Changed to uselauncher(UninstallOperatorModalProvider, ...)in action ctasubscription.tsx: Updated to useuseOverlay()for InstallPlanApprovalModal, SubscriptionChannelModal, and UninstallOperatorModalinstall-plan.tsx: Changed to uselaunchOverlay(InstallPlanPreviewModalProvider, ...)configure-update-strategy.tsx: Changed to uselaunchOverlay(UpdateStrategyModalProvider, ...)operator-hub-details.tsx: Changed to uselaunchOverlay(EditDefaultSourcesModalProvider, ...)catalog-source-provider.ts: Changed to uselaunchOverlay(DisableDefaultSourceModalProvider, ...)React Hooks Dependency Array Updates
Added
launcher/launchOverlayto dependency arrays where used:useCommonActions.ts: AddedlaunchOverlayto dependency arrayuseOperatorActions.ts: Addedlauncherto dependency arrayuseSubscriptionActions.ts: Addedlauncherto dependency arraycatalog-source-provider.ts: AddedlaunchOverlayto dependency arrayuninstall-operator-modal.tsx: AddeduseCallbackfor functions used in useEffect dependencies to satisfy exhaustive-deps rulesNote: While
launchOverlayfromuseOverlay()is referentially stable, it's included in dependency arrays for correctness and to satisfy exhaustive-deps linting rules.Naming Convention
All modal components use named exports following team standards per feedback from rhamilto on PR #15935:
TaintsModalProvider,UninstallOperatorModalProvider,UpdateStrategyModalProvider, etc.import { TaintsModalProvider } from ...public/components/modals/index.tsto use named exportTest Plan
Related
Assisted-by: Claude Code
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.