Skip to content

CONSOLE-4447: Migrate operator-lifecycle-manager modals to PatternFly v6 #16122

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
openshift-merge-bot merged 4 commits into from
Mar 13, 2026
Merged

CONSOLE-4447: Migrate operator-lifecycle-manager modals to PatternFly v6 #16122

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
16ae1fc
CONSOLE-4447: Migrate operator-lifecycle-manager modals to PatternFly v6
rhamilto Mar 9, 2026
36ea34c
CONSOLE-4447: Address code review feedback for OLM modals
rhamilto Mar 9, 2026
a8a4b54
CONSOLE-4447: Change OK button to primary variant in installplan-prev…
rhamilto Mar 9, 2026
331dc3c
Fix accessibility and UX issues in OLM modals
rhamilto Mar 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions ...tend/packages/operator-lifecycle-manager/integration-tests-cypress/views/operator.view.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,7 @@ export const operator = {
modal.modalTitleShouldContain('Uninstall Operator?');
cy.get('.loading-skeleton--table', { timeout: 120000 }).should('not.exist');
},
checkDeleteAllOperands: () =>
cy.byTestID('Delete all operand instances for this operator__checkbox').click(),
checkDeleteAllOperands: () => cy.byTestID('delete-all-operands').click(),
},
createOperand: (
operatorName: string,
Expand Down
3 changes: 2 additions & 1 deletion frontend/packages/operator-lifecycle-manager/locales/en/olm.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@
"Manual update approval is required when not installing the latest version for the selected channel.": "Manual update approval is required when not installing the latest version for the selected channel.",
"Show community Operator": "Show community Operator",
"Community Operators are Operators which have not been vetted or verified by Red Hat. Community Operators should be used with caution because their stability is unknown. Red Hat provides no support for community Operators.": "Community Operators are Operators which have not been vetted or verified by Red Hat. Community Operators should be used with caution because their stability is unknown. Red Hat provides no support for community Operators.",
"Learn more about Red Hat’s third party software support policy": "Learn more about Red Hat’s third party software support policy",
"Learn more about Red Hat's third party software support policy": "Learn more about Red Hat's third party software support policy",
"Do not show this warning again": "Do not show this warning again",
"Continue": "Continue",
"OperatorHub details": "OperatorHub details",
Expand All @@ -278,6 +278,7 @@
"This Operator is being installed on the cluster.": "This Operator is being installed on the cluster.",
"Community Operator": "Community Operator",
"This is a community provided Operator. These are Operators which have not been vetted or verified by Red Hat. Community Operators should be used with caution because their stability is unknown. Red Hat provides no support for community Operators.": "This is a community provided Operator. These are Operators which have not been vetted or verified by Red Hat. Community Operators should be used with caution because their stability is unknown. Red Hat provides no support for community Operators.",
"Learn more about Red Hat’s third party software support policy": "Learn more about Red Hat’s third party software support policy",
"Cluster in STS Mode": "Cluster in STS Mode",
"This cluster is using AWS Security Token Service to reach the cloud API. In order for this operator to take the actions it requires directly with the cloud API, you must provide a role ARN (with an attached policy) during installation. Please see the operator description for more details.": "This cluster is using AWS Security Token Service to reach the cloud API. In order for this operator to take the actions it requires directly with the cloud API, you must provide a role ARN (with an attached policy) during installation. Please see the operator description for more details.",
"Cluster in Azure Workload Identity / Federated Identity Mode": "Cluster in Azure Workload Identity / Federated Identity Mode",
Expand Down
84 changes: 53 additions & 31 deletions ...ages/operator-lifecycle-manager/src/components/descriptors/spec/resource-requirements.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,23 @@
import type { FC } from 'react';
import { useState } from 'react';
import { Button, Grid, GridItem } from '@patternfly/react-core';
import { useState, useId } from 'react';
import {
Button,
Form,
Grid,
GridItem,
Modal,
ModalBody,
ModalHeader,
ModalVariant,
} from '@patternfly/react-core';
import { PencilAltIcon } from '@patternfly/react-icons/dist/esm/icons/pencil-alt-icon';
import * as _ from 'lodash';
import { useTranslation } from 'react-i18next';
import type { OverlayComponent } from '@console/dynamic-plugin-sdk/src/app/modal-support/OverlayProvider';
import { useOverlay } from '@console/dynamic-plugin-sdk/src/app/modal-support/useOverlay';
import {
ModalTitle,
ModalBody,
ModalSubmitFooter,
ModalWrapper,
} from '@console/internal/components/factory/modal';
import type { K8sKind, K8sResourceKind } from '@console/internal/module/k8s';
import { k8sUpdate, referenceFor } from '@console/internal/module/k8s';
import { ModalFooterWithAlerts } from '@console/shared/src/components/modals/ModalFooterWithAlerts';
import { usePromiseHandler } from '@console/shared/src/hooks/promise-handler';
import { useK8sModel } from '@console/shared/src/hooks/useK8sModel';

Expand Down Expand Up @@ -93,6 +97,7 @@ export const ResourceRequirementsModal = (props: ResourceRequirementsModalProps)
const [storage, setStorage] = useState<string>(
_.get(obj.spec, `${path}.${type}.ephemeral-storage`, ''),
);
const formId = useId();

const submit = (e) => {
e.preventDefault();
Expand All @@ -109,43 +114,60 @@ export const ResourceRequirementsModal = (props: ResourceRequirementsModalProps)
};

return (
<form onSubmit={(e) => submit(e)} className="modal-content">
<ModalTitle>{props.title}</ModalTitle>
<>
<ModalHeader title={props.title} data-test-id="modal-title" />
<ModalBody>
<Grid hasGutter>
<GridItem>{props.description}</GridItem>
<ResourceRequirements
cpu={cpu}
memory={memory}
storage={storage}
onChangeCPU={setCPU}
onChangeMemory={setMemory}
onChangeStorage={setStorage}
path={path}
/>
</Grid>
<Form id={formId} onSubmit={(e) => submit(e)}>
<Grid hasGutter>
<GridItem>{props.description}</GridItem>
<ResourceRequirements
cpu={cpu}
memory={memory}
storage={storage}
onChangeCPU={setCPU}
onChangeMemory={setMemory}
onChangeStorage={setStorage}
path={path}
/>
</Grid>
</Form>
</ModalBody>
<ModalSubmitFooter
errorMessage={errorMessage}
inProgress={inProgress}
submitText={t('public~Save')}
cancel={cancel}
/>
</form>
<ModalFooterWithAlerts errorMessage={errorMessage}>
<Button
type="submit"
variant="primary"
form={formId}
isLoading={inProgress}
isDisabled={inProgress}
data-test="confirm-action"
id="confirm-action"
>
{t('public~Save')}
</Button>
<Button
variant="link"
onClick={cancel}
isDisabled={inProgress}
data-test-id="modal-cancel-action"
>
{t('public~Cancel')}
</Button>
</ModalFooterWithAlerts>
</>
);
};

const ResourceRequirementsModalOverlay: OverlayComponent<ResourceRequirementsModalOverlayProps> = (
props,
) => {
return (
<ModalWrapper blocking onClose={props.closeOverlay}>
<Modal variant={ModalVariant.small} isOpen onClose={props.closeOverlay}>
<ResourceRequirementsModal
{...props}
close={props.closeOverlay}
cancel={props.closeOverlay}
/>
</ModalWrapper>
</Modal>
);
};

Expand Down
8 changes: 2 additions & 6 deletions ...tor-lifecycle-manager/src/components/modals/__tests__/installplan-approval-modal.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,8 @@ jest.mock('@console/internal/module/k8s', () => ({
modelFor: jest.fn(),
}));

jest.mock('@console/internal/components/factory/modal', () => ({
...jest.requireActual('@console/internal/components/factory/modal'),
ModalTitle: jest.fn(({ children }) => children),
ModalBody: jest.fn(({ children }) => children),
ModalSubmitFooter: jest.fn(() => null),
ModalWrapper: jest.fn(({ children }) => children),
jest.mock('@console/shared/src/components/modals/ModalFooterWithAlerts', () => ({
ModalFooterWithAlerts: jest.fn(({ children }) => <div>{children}</div>),
Comment on lines +16 to +17
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Preserve the footer submit path in this mock.

This mock reduces ModalFooterWithAlerts to a passive wrapper, so the spec now bypasses the PF v6 footer/button wiring and submits the <form> directly. That means a broken form association or disabled submit button in InstallPlanApprovalModal would still pass here. Please render a minimal submit button in the mock and drive the submit assertions through that button instead.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In
`@frontend/packages/operator-lifecycle-manager/src/components/modals/__tests__/installplan-approval-modal.spec.tsx`
around lines 16 - 17, The test mock for ModalFooterWithAlerts currently only
renders children and bypasses PF submit wiring; update the jest.mock for
ModalFooterWithAlerts to render a minimal submit button that forwards form
submission (e.g., render a <button type="submit">) so tests exercise the
component's real submit path; ensure the mock's exported ModalFooterWithAlerts
(used by InstallPlanApprovalModal tests) still renders children and includes the
submit button so assertions trigger via clicking the button rather than directly
submitting the form.

}));

const mockModelFor = modelFor as jest.Mock;
Expand Down
24 changes: 6 additions & 18 deletions ...tor-lifecycle-manager/src/components/modals/__tests__/subscription-channel-modal.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,8 @@ import type { SubscriptionKind, PackageManifestKind } from '../../../types';
import type { SubscriptionChannelModalProps } from '../subscription-channel-modal';
import { SubscriptionChannelModal } from '../subscription-channel-modal';

jest.mock('@console/internal/components/factory/modal', () => ({
...jest.requireActual('@console/internal/components/factory/modal'),
ModalTitle: jest.fn(({ children }) => children),
ModalBody: jest.fn(({ children }) => children),
ModalSubmitFooter: jest.fn(({ submitText, submitDisabled }) => (
<button type="submit" disabled={submitDisabled}>
{submitText}
</button>
)),
ModalWrapper: jest.fn(({ children }) => children),
jest.mock('@console/shared/src/components/modals/ModalFooterWithAlerts', () => ({
ModalFooterWithAlerts: jest.fn(({ children }) => <div>{children}</div>),
}));

describe('SubscriptionChannelModal', () => {
Expand Down Expand Up @@ -96,10 +88,8 @@ describe('SubscriptionChannelModal', () => {
const nightlyRadio = screen.getByRole('radio', { name: /nightly/i });
fireEvent.click(nightlyRadio);

const form = screen.getByRole('button', { name: 'Save' }).closest('form');
if (form) {
fireEvent.submit(form);
}
const saveButton = screen.getByRole('button', { name: 'Save' });
fireEvent.click(saveButton);

await waitFor(() => {
expect(k8sUpdate).toHaveBeenCalledTimes(1);
Expand All @@ -121,10 +111,8 @@ describe('SubscriptionChannelModal', () => {
const nightlyRadio = screen.getByRole('radio', { name: /nightly/i });
fireEvent.click(nightlyRadio);

const form = screen.getByRole('button', { name: 'Save' }).closest('form');
if (form) {
fireEvent.submit(form);
}
const saveButton = screen.getByRole('button', { name: 'Save' });
fireEvent.click(saveButton);

await waitFor(() => {
expect(close).toHaveBeenCalledTimes(1);
Expand Down
16 changes: 12 additions & 4 deletions ...rator-lifecycle-manager/src/components/modals/__tests__/uninstall-operator-modal.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ jest.mock('@console/shared/src/hooks/useOperands', () => ({
useOperands: jest.fn(),
}));

jest.mock('@console/shared/src/components/modals/ModalFooterWithAlerts', () => ({
ModalFooterWithAlerts: jest.fn(({ children }) => <div>{children}</div>),
}));

jest.mock('react-i18next', () => ({
useTranslation: () => ({
t: (key: string) => key.replace(/^[^~]+~/, ''), // Remove namespace prefix (e.g., "olm~")
Expand Down Expand Up @@ -74,7 +78,8 @@ describe(UninstallOperatorModal.name, () => {
it('deletes subscription when form is submitted', async () => {
renderWithProviders(<UninstallOperatorModal {...uninstallOperatorModalProps} />);

fireEvent.submit(screen.getByRole('form'));
const uninstallButton = screen.getByRole('button', { name: 'Uninstall' });
fireEvent.click(uninstallButton);

await waitFor(() => {
expect(mockK8sKill).toHaveBeenCalledTimes(2);
Expand All @@ -96,7 +101,8 @@ describe(UninstallOperatorModal.name, () => {
it('deletes ClusterServiceVersion when form is submitted', async () => {
renderWithProviders(<UninstallOperatorModal {...uninstallOperatorModalProps} />);

fireEvent.submit(screen.getByRole('form'));
const uninstallButton = screen.getByRole('button', { name: 'Uninstall' });
fireEvent.click(uninstallButton);

await waitFor(() => {
expect(mockK8sKill).toHaveBeenCalledTimes(2);
Expand Down Expand Up @@ -125,7 +131,8 @@ describe(UninstallOperatorModal.name, () => {
<UninstallOperatorModal {...uninstallOperatorModalProps} subscription={testSubscription} />,
);

fireEvent.submit(screen.getByRole('form'));
const uninstallButton = screen.getByRole('button', { name: 'Uninstall' });
fireEvent.click(uninstallButton);

await waitFor(() => {
expect(mockK8sKill).toHaveBeenCalledTimes(1);
Expand All @@ -135,7 +142,8 @@ describe(UninstallOperatorModal.name, () => {
it('calls close callback after successful form submission', async () => {
renderWithProviders(<UninstallOperatorModal {...uninstallOperatorModalProps} />);

fireEvent.submit(screen.getByRole('form'));
const uninstallButton = screen.getByRole('button', { name: 'Uninstall' });
fireEvent.click(uninstallButton);

await waitFor(() => {
expect(uninstallOperatorModalProps.close).toHaveBeenCalledTimes(1);
Expand Down
63 changes: 38 additions & 25 deletions ...ackages/operator-lifecycle-manager/src/components/modals/disable-default-source-modal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,13 @@
import type { FC, FormEvent } from 'react';
import { useCallback } from 'react';
import { Button, Form, Modal, ModalBody, ModalHeader, ModalVariant } from '@patternfly/react-core';
import * as _ from 'lodash';
import { useTranslation } from 'react-i18next';
import type { OverlayComponent } from '@console/dynamic-plugin-sdk/src/app/modal-support/OverlayProvider';
import type { ModalComponentProps } from '@console/internal/components/factory/modal';
import {
ModalTitle,
ModalBody,
ModalSubmitFooter,
ModalWrapper,
} from '@console/internal/components/factory/modal';
import type { K8sKind } from '@console/internal/module/k8s';
import { k8sPatch } from '@console/internal/module/k8s';
import { YellowExclamationTriangleIcon } from '@console/shared';
import { ModalFooterWithAlerts } from '@console/shared/src/components/modals/ModalFooterWithAlerts';
import { usePromiseHandler } from '@console/shared/src/hooks/promise-handler';
import type { OperatorHubKind } from '../operator-hub';

Expand Down Expand Up @@ -52,38 +47,56 @@ const DisableDefaultSourceModal: FC<DisableDefaultSourceModalProps> = ({
);

return (
<form onSubmit={submit} name="form" className="modal-content ">
<ModalTitle>
<YellowExclamationTriangleIcon className="co-icon-space-r" />{' '}
{t('olm~Disable CatalogSource?')}
</ModalTitle>
<>
<ModalHeader
title={t('olm~Disable CatalogSource?')}
titleIconVariant="warning"
data-test-id="modal-title"
labelId="disable-default-source-modal-title"
/>
<ModalBody>
{t(
'olm~By disabling a default source, the operators it provides will no longer appear in Software Catalog and any operator that has been installed from this source will no longer receive updates until the source is re-enabled. Disabling the source will also remove the corresponding OperatorSource and CatalogSource resources from the cluster.',
)}
<Form id="disable-default-source-form" onSubmit={submit}>
{t(
'olm~By disabling a default source, the operators it provides will no longer appear in Software Catalog and any operator that has been installed from this source will no longer receive updates until the source is re-enabled. Disabling the source will also remove the corresponding OperatorSource and CatalogSource resources from the cluster.',
)}
</Form>
</ModalBody>
<ModalSubmitFooter
submitText={t('public~Disable')}
cancel={cancel}
errorMessage={errorMessage}
inProgress={inProgress}
submitDanger
/>
</form>
<ModalFooterWithAlerts errorMessage={errorMessage}>
<Button
type="submit"
variant="danger"
form="disable-default-source-form"
isLoading={inProgress}
isDisabled={inProgress}
data-test="confirm-action"
id="confirm-action"
>
{t('public~Disable')}
</Button>
<Button variant="link" onClick={cancel} data-test-id="modal-cancel-action">
{t('public~Cancel')}
</Button>
</ModalFooterWithAlerts>
</>
);
};

export const DisableDefaultSourceModalOverlay: OverlayComponent<DisableDefaultSourceModalProps> = (
props,
) => {
return (
<ModalWrapper blocking onClose={props.closeOverlay}>
<Modal
variant={ModalVariant.small}
isOpen
onClose={props.closeOverlay}
aria-labelledby="disable-default-source-modal-title"
>
<DisableDefaultSourceModal
{...props}
close={props.closeOverlay}
cancel={props.closeOverlay}
/>
</ModalWrapper>
</Modal>
);
};

Expand Down
Loading