CONSOLE-5068: Remove createModalLauncher from modal factory#16066
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@sg00dwin: This pull request references CONSOLE-5068 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 sub-task 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
sg00dwin
force-pushed
the
CONSOLE-5068-remove-createmodallauncher
branch
from
f7ec55c to
84fd938
Compare
|
@sg00dwin: This pull request references CONSOLE-5068 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 sub-task 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. |
|
/retest-required |
1 similar comment
|
/retest-required |
|
/retest |
|
/test e2e-gcp-console |
|
/retest-required |
2 similar comments
|
/retest-required |
|
/retest-required |
sg00dwin
changed the title
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
|
/hold for #15997 to merge first |
openshift-ci
bot
added
the
do-not-merge/hold
|
/hold cancel |
openshift-ci
bot
removed
the
do-not-merge/hold
Assisted by claude code
rhamilto
force-pushed
the
CONSOLE-5068-remove-createmodallauncher
branch
from
84fd938 to
4776197
Compare
|
@sg00dwin: This pull request references CONSOLE-5068 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 sub-task 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. |
|
/approve Tech debt /verified by @rhamilto |
openshift-ci-robot
added
the
verified
|
@rhamilto: This PR has been marked as verified by 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
px-approved
📝 WalkthroughWalkthroughThe PR removes legacy modal launcher infrastructure from the modal component module, eliminating deprecated utilities (createModal, createModalLauncher, GetModalContainer, CreateModal, CreateModalLauncher types) and their supporting wiring including Redux stores, router contexts, and container rendering logic. PatternFly-based modal components (ModalWrapper, ModalTitle, ModalBody, ModalFooter, ModalSubmitFooter) and their props remain intact and functional. React imports are simplified to remove ReactElement and ComponentType, retaining only SyntheticEvent, FC, and ReactNode. The change results in a net reduction of 80 lines while maintaining the current modal component interface. 🚥 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)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/public/components/factory/modal.tsx (1)
170-215:⚠️ Potential issue | 🟠 MajorRestore the temporary
createModalLaunchercompatibility surface.This file no longer exports
createModalLauncher,CreateModalLauncher, orCreateModalLauncherProps, so the change is already a breaking public API removal. That contradicts the staged rollout described in the PR objective, where consumers still in flight should keep compiling against a deprecated no-op stub until the final cleanup lands. Please re-add the deprecated stub and its types here, then remove them in the follow-up once the dependent migrations are merged.Suggested compatibility stub
-import type { SyntheticEvent, FC, ReactNode } from 'react'; +import type { SyntheticEvent, FC, ReactNode, ComponentType } from 'react'; ... export type ModalSubmitFooterProps = { message?: string; errorMessage?: string; inProgress: boolean; cancel: (e: SyntheticEvent<any, Event>) => void; cancelText?: ReactNode; className?: string; resetText?: ReactNode; reset?: (e: SyntheticEvent<any, Event>) => void; submitText: ReactNode; submitDisabled?: boolean; submitDanger?: boolean; buttonAlignment?: 'left' | 'right'; ariaLabel?: string; }; + +/** `@deprecated` Use PF modals instead */ +export type CreateModalLauncherProps = { + blocking?: boolean; + modalClassName?: string; +}; + +/** `@deprecated` Use PF modals instead */ +export type CreateModalLauncher = <P extends ModalComponentProps>( + C: ComponentType<P>, + modalWrapper?: boolean, +) => (props: P & CreateModalLauncherProps) => { result: Promise<unknown> }; + +/** `@deprecated` Use PF modals instead */ +export const createModalLauncher: CreateModalLauncher = + () => + () => ({ + result: Promise.resolve(undefined), + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/public/components/factory/modal.tsx` around lines 170 - 215, Re-add the deprecated compatibility surface by exporting a no-op stub function createModalLauncher and its associated types CreateModalLauncher and CreateModalLauncherProps so existing consumers still compile; define CreateModalLauncherProps matching previous modal launcher props shape, export type alias CreateModalLauncher as the function type, and implement createModalLauncher as a deprecated wrapper that returns a noop launcher (e.g., a function that returns a component or does nothing) while forwarding to the new API where appropriate; mark them as deprecated in comments and export them alongside ModalWrapperProps/ModalComponentProps to preserve the public API.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@frontend/public/components/factory/modal.tsx`:
- Around line 170-215: Re-add the deprecated compatibility surface by exporting
a no-op stub function createModalLauncher and its associated types
CreateModalLauncher and CreateModalLauncherProps so existing consumers still
compile; define CreateModalLauncherProps matching previous modal launcher props
shape, export type alias CreateModalLauncher as the function type, and implement
createModalLauncher as a deprecated wrapper that returns a noop launcher (e.g.,
a function that returns a component or does nothing) while forwarding to the new
API where appropriate; mark them as deprecated in comments and export them
alongside ModalWrapperProps/ModalComponentProps to preserve the public API.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 99d8abbb-245c-479c-9ba6-f23d84c0ee5b
📒 Files selected for processing (1)
frontend/public/components/factory/modal.tsx
|
@sg00dwin: This pull request references CONSOLE-5068 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 sub-task 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. |
|
@sg00dwin: This pull request references CONSOLE-5068 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 sub-task 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, sg00dwin 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
approved
|
@sg00dwin: This pull request references CONSOLE-5068 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 sub-task 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. |
|
@sg00dwin: This pull request references CONSOLE-5068 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 sub-task 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. |
|
@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. |
3 participants
This PR must wait to merge after all dependency modal migration PRs have merged.
Summary
Removes the deprecated
createModalLauncherfunction and all associated types frompublic/components/factory/modal.tsx. This is the final cleanup step in the CONSOLE-5012 modal migration effort.Changes
Removed from
modal.tsx:createModalfunctioncreateModalLauncherimplementation (replaced with no-op stub)GetModalContainertypeCreateModaltypeReactDOM,Provider,Router,CompatRouter,store,history,PluginStoreProvider,pluginStoreRetained (still in use by migrated modals):
ModalWrapper,ModalTitle,ModalBody,ModalFooter,ModalSubmitFooterModalComponentPropsTest plan
yarn buildcompiles without errorsyarn lintpassesyarn testpassesAssisted by Claude Code
Summary by CodeRabbit