Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export const helmDetailsPage = {
cy.get('dl dt').contains(fieldName).next('dd').should('contain.text', fieldValue);
},
uninstallHelmRelease: () => {
cy.get('form.modal-content').within(() => {
cy.get('form').within(() => {
cy.byLegacyTestID('modal-title').should('contain.text', 'Delete Helm Release?');
cy.get('button[type=submit]').click({ force: true });
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import type { FC } from 'react';
import { useCallback } from 'react';
import { useCallback, useState } from 'react';
import { Modal, ModalBody, ModalHeader, ModalVariant } from '@patternfly/react-core';
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 type { ModalComponentProps } from '@console/internal/components/factory';
import { ModalTitle, ModalBody, ModalWrapper } from '@console/internal/components/factory';
import { SyncMarkdownView } from '@console/internal/components/markdown-view';

type HelmReadmeModalProps = {
Expand All @@ -13,24 +13,35 @@ type HelmReadmeModalProps = {
};
type Props = HelmReadmeModalProps & ModalComponentProps;

const HelmReadmeModal: FC<Props> = ({ readme, theme, close }) => {
const HelmReadmeModal: FC<Props> = ({ readme, theme }) => {
const { t } = useTranslation();
return (
<div className="modal-content">
<ModalTitle close={close}>{t('helm-plugin~README')}</ModalTitle>
<>
<ModalHeader title={t('helm-plugin~README')} labelId="helm-readme-modal-title" />
<ModalBody>
<SyncMarkdownView content={readme} theme={theme} />
</ModalBody>
</div>
</>
);
};

const HelmReadmeModalProvider: OverlayComponent<Props> = (props) => {
return (
<ModalWrapper blocking onClose={props.closeOverlay} className="modal-lg">
<HelmReadmeModal close={props.closeOverlay} cancel={props.closeOverlay} {...props} />
</ModalWrapper>
);
const [isOpen, setIsOpen] = useState(true);
const handleClose = () => {
setIsOpen(false);
props.closeOverlay();
};

return isOpen ? (
<Modal
variant={ModalVariant.large}
isOpen
onClose={handleClose}
aria-labelledby="helm-readme-modal-title"
>
<HelmReadmeModal {...props} />
</Modal>
) : null;
Comment on lines +35 to +44
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🌐 Web query:

PatternFly v6 Modal aria-labelledby ModalHeader accessibility

💡 Result:

In PatternFly v6, the modal’s accessible name comes from either:

  • title on <Modal> (PatternFly will auto-set aria-labelledby for you), or
  • aria-labelledby on <Modal> that points at the ID of the element that visually labels the modal—this is required when you use a custom header / the header approach instead of title. [1]

With the v6 React subcomponents, that typically means:

  • Set an ID on the header title via <ModalHeader labelId="my-modal-title" ... />
  • Point the modal at it via <Modal aria-labelledby="my-modal-title" ... /> (you can include multiple IDs if needed). [1][2]

Example:

<Modal
  isOpen
  onClose={onClose}
  aria-labelledby="my-modal-title"
>
  <ModalHeader labelId="my-modal-title">My modal title</ModalHeader>
  <ModalBody>...</ModalBody>
</Modal>

Notes:

  • If there is no visible title, use aria-label on <Modal> instead. [1]
  • If you provide a description with <ModalHeader descriptorId="..." />, the modal’s aria-describedby should match that descriptorId (or the relevant header ID), per the docs. [2]

Sources: PatternFly Modal accessibility + props documentation. [1][2]

References:
[1] PatternFly Modal – Accessibility (aria-labelledby requirements; title auto-wiring) https://www.patternfly.org/components/modal/accessibility/
[2] PatternFly Modal – React props (Modal aria-labelledby; ModalHeader labelId / descriptorId) https://www.patternfly.org/components/modal/


🌐 Web query:

PatternFly v6 Modal accessibility best practices ARIA attributes

💡 Result:

PatternFly v6 Modal: accessibility + ARIA best practices

Required / expected ARIA on the modal container

  • Use role="dialog" on the modal container (.pf-v6-c-modal-box). [1]
  • Use aria-modal="true" on the modal container. (PatternFly marks this as required for its HTML modal.) [1]

Give the modal an accessible name (required)

Use one of:

  • aria-labelledby="ID_OF_VISIBLE_TITLE" (recommended when you have a visible title). [1]
  • aria-label="Text title" if there is no visible text title. [1]

PatternFly React props that map to this

  • title (automatically sets aria-labelledby) or aria-labelledby (if you provide a custom header), or aria-label (if no visible title). [1]

Provide an accessible description (recommended when there’s static “primary message” text)

  • Add aria-describedby="ID_OF_PRIMARY_CONTENT" pointing to the element that describes the modal’s purpose/message. [1], [2]

PatternFly React prop

  • aria-describedby. [1]

Close control (required)

  • Ensure there is at least one clickable way to close the modal (close icon, Cancel button, etc.). [1]
  • If the close control is an icon-only button, give it an accessible name (e.g., aria-label="Close"). [1]

Make the background inert + trap focus (required behavior for a modal)

  • While open, users must not be able to tab to or interact with page content outside the modal (focus trap). [1], [3]
  • PatternFly’s guidance for screen readers is to set aria-hidden="true" on the parent wrapper of the page content (with the modal rendered outside that wrapper as a sibling), so screen readers can’t navigate the background. [1], [3]
  • If you set aria-modal="true", ensure the dialog actually behaves modally (no background interaction), per ARIA practices guidance. [2]

PatternFly React prop

  • Avoid disableFocusTrap unless you implement your own focus management correctly. [1]

If the modal body scrolls (overflow)

  • Give the scrollable body an accessible name and role, and make it keyboard-focusable:
    • Body: aria-label="…", role="region" (typical), and tabindex="0" when the body is scrollable. [1]

PatternFly React props

  • bodyAriaLabel (and optionally bodyAriaRole). [1]

🏁 Script executed:

# Locate and examine HelmReadmeModal component
find . -name "HelmReadmeModal.tsx" -type f

Repository: openshift/console

Length of output: 149


🏁 Script executed:

# Find the actual HelmReadmeModal component and examine its implementation
fd -t f "HelmReadmeModal" --exec cat -n {} \;

Repository: openshift/console

Length of output: 1935


Add aria-labelledby to Modal; PatternFly v6 does NOT auto-associate custom ModalHeader with the modal's accessible name.

When using a custom <ModalHeader> composition (rather than Modal's title prop), you must explicitly set aria-labelledby on <Modal> pointing to the labelId on <ModalHeader>. Currently, the modal has no accessible name, so screen readers won't announce the README title on open.

<Modal 
  variant={ModalVariant.large} 
  isOpen 
  aria-labelledby="helm-readme-modal-title"
  onClose={handleClose}
>
  <HelmReadmeModal {...props} titleId="helm-readme-modal-title" />
</Modal>

Update HelmReadmeModal to accept and pass the title ID:

const HelmReadmeModal: FunctionComponent<Props & { titleId?: string }> = ({ readme, theme, titleId }) => {
  const { t } = useTranslation();
  return (
    <>
      <ModalHeader title={t('helm-plugin~README')} labelId={titleId} />
      <ModalBody>
        <SyncMarkdownView content={readme} theme={theme} />
      </ModalBody>
    </>
  );
};

Alternatively, simplify by moving the title to Modal's title prop (PatternFly auto-wires aria-labelledby for you).

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

In
`@frontend/packages/helm-plugin/src/components/forms/install-upgrade/HelmReadmeModal.tsx`
around lines 35 - 39, The Modal instance rendering HelmReadmeModal lacks
aria-labelledby so the custom ModalHeader title isn't exposed to assistive tech;
update the Modal invocation (the Modal with variant={ModalVariant.large} isOpen
onClose={handleClose}) to include aria-labelledby="helm-readme-modal-title" and
change the HelmReadmeModal component signature to accept a titleId prop (e.g.,
titleId?: string) and pass that into ModalHeader via its labelId prop
(ModalHeader labelId={titleId}); alternatively, move the title into the Modal's
title prop so PatternFly wires aria-labelledby automatically.

};

export const useHelmReadmeModalLauncher = (props: Props) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,18 @@ import {
ModalVariant,
Modal,
ModalHeader,
ModalBody as PfModalBody,
ModalFooter as PfModalFooter,
ModalBody,
} from '@patternfly/react-core';
import type { TFunction } from 'i18next';
import { Trans, useTranslation } from 'react-i18next';
import type { OverlayComponent } from '@console/dynamic-plugin-sdk/src/lib-core';
import { useOverlay } from '@console/dynamic-plugin-sdk/src/lib-core';
import type { ModalComponentProps } from '@console/internal/components/factory';
import { ErrorMessage, LoadingBox } from '@console/internal/components/utils';
import { LoadingBox } from '@console/internal/components/utils';
import { useK8sWatchResource } from '@console/internal/components/utils/k8s-watch-hook';
import { PodModel } from '@console/internal/models';
import type { PodKind } from '@console/internal/module/k8s';
import { ModalFooterWithAlerts } from '@console/shared/src/components/modals/ModalFooterWithAlerts';
import { usePromiseHandler } from '@console/shared/src/hooks/promise-handler';
import {
NODE_STATUS_UNDER_MAINTENANCE,
Expand Down Expand Up @@ -203,9 +203,18 @@ const PowerOffHostModal: OverlayComponent<PowerOffHostModalProps> = (props) => {
const isUnderMaintenance = status.status === NODE_STATUS_UNDER_MAINTENANCE;

return (
<Modal isOpen onClose={props.closeOverlay} variant={ModalVariant.small}>
<ModalHeader title={t('metal3-plugin~Power Off Host')} />
<PfModalBody>
<Modal
isOpen
onClose={props.closeOverlay}
variant={ModalVariant.small}
aria-labelledby="power-off-host-modal-title"
>
<ModalHeader
title={t('metal3-plugin~Power Off Host')}
data-test-id="modal-title"
labelId="power-off-host-modal-title"
/>
<ModalBody>
{!loaded ? (
<LoadingBox />
) : canPowerOffSafely ? (
Expand All @@ -229,21 +238,22 @@ const PowerOffHostModal: OverlayComponent<PowerOffHostModalProps> = (props) => {
cancel={closeOverlay}
/>
)}
</PfModalBody>
<PfModalFooter>
{errorMessage && <ErrorMessage message={errorMessage} />}
</ModalBody>
<ModalFooterWithAlerts errorMessage={errorMessage}>
<Button
variant="primary"
onClick={submit}
isLoading={inProgress}
isDisabled={!canPowerOffSafely && !forceOff}
data-test="confirm-action"
id="confirm-action"
>
{t('metal3-plugin~Power Off')}
</Button>
<Button variant="link" onClick={closeOverlay}>
<Button variant="link" onClick={closeOverlay} data-test-id="modal-cancel-action">
{t('metal3-plugin~Cancel')}
</Button>
</PfModalFooter>
</ModalFooterWithAlerts>
</Modal>
);
};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,10 @@
import { useCallback } from 'react';
import {
Modal,
ModalHeader,
ModalVariant,
ModalBody as PfModalBody,
ModalFooter as PfModalFooter,
Button,
} from '@patternfly/react-core';
import { Modal, ModalHeader, ModalVariant, ModalBody, Button } from '@patternfly/react-core';
import { useTranslation } from 'react-i18next';
import type { OverlayComponent } from '@console/dynamic-plugin-sdk/src/lib-core';
import { useOverlay } from '@console/dynamic-plugin-sdk/src/lib-core';
import type { ModalComponentProps } from '@console/internal/components/factory';
import { ErrorMessage } from '@console/internal/components/utils';
import { ModalFooterWithAlerts } from '@console/shared/src/components/modals/ModalFooterWithAlerts';
import { usePromiseHandler } from '@console/shared/src/hooks/promise-handler';
import { restartHost } from '../../k8s/requests/bare-metal-host';
import type { BareMetalHostKind } from '../../types';
Expand Down Expand Up @@ -45,21 +38,35 @@ const RestartHostModal: OverlayComponent<RestartHostModalProps> = (props) => {
);

return (
<Modal isOpen onClose={props.closeOverlay} variant={ModalVariant.small}>
<ModalHeader title={t('metal3-plugin~Restart Bare Metal Host')} />
<PfModalBody>
<Modal
isOpen
onClose={props.closeOverlay}
variant={ModalVariant.small}
aria-labelledby="restart-host-modal-title"
>
<ModalHeader
title={t('metal3-plugin~Restart Bare Metal Host')}
data-test-id="modal-title"
labelId="restart-host-modal-title"
/>
<ModalBody>
<p>{t('metal3-plugin~The host will be powered off and on again.')}</p>
<PowerOffWarning restart />
</PfModalBody>
<PfModalFooter>
{errorMessage && <ErrorMessage message={errorMessage} />}
<Button variant="primary" onClick={onSubmit} isLoading={inProgress}>
</ModalBody>
<ModalFooterWithAlerts errorMessage={errorMessage}>
<Button
variant="primary"
onClick={onSubmit}
isLoading={inProgress}
data-test="confirm-action"
id="confirm-action"
>
{t('metal3-plugin~Restart')}
</Button>
<Button variant="link" onClick={closeOverlay}>
<Button variant="link" onClick={closeOverlay} data-test-id="modal-cancel-action">
{t('metal3-plugin~Cancel')}
</Button>
</PfModalFooter>
</ModalFooterWithAlerts>
</Modal>
);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,17 @@ import {
Stack,
StackItem,
TextInput,
ModalBody as PfModalBody,
ModalFooter as PfModalFooter,
ModalBody,
Button,
} from '@patternfly/react-core';
import { Trans, useTranslation } from 'react-i18next';
import type { OverlayComponent } from '@console/dynamic-plugin-sdk/src/lib-core';
import { useOverlay } from '@console/dynamic-plugin-sdk/src/lib-core';
import type { ModalComponentProps } from '@console/internal/components/factory';
import { ErrorMessage } from '@console/internal/components/utils/button-bar';
import { useK8sWatchResource } from '@console/internal/components/utils/k8s-watch-hook';
import type { K8sResourceKind } from '@console/internal/module/k8s';
import { referenceForModel } from '@console/internal/module/k8s';
import { ModalFooterWithAlerts } from '@console/shared/src/components/modals/ModalFooterWithAlerts';
import { usePromiseHandler } from '@console/shared/src/hooks/promise-handler';
import { useMaintenanceCapability } from '../../hooks/useMaintenanceCapability';
import { startNodeMaintenance } from '../../k8s/requests/node-maintenance';
Expand Down Expand Up @@ -60,9 +59,18 @@ export const StartNodeMaintenanceModal: OverlayComponent<StartNodeMaintenanceMod
const cephClusterHealthy = cephCluster?.status?.ceph?.health === 'HEALTH_OK';

return (
<Modal isOpen onClose={closeOverlay} variant={ModalVariant.small}>
<ModalHeader title={t('metal3-plugin~Start Maintenance')} />
<PfModalBody>
<Modal
isOpen
onClose={closeOverlay}
variant={ModalVariant.small}
aria-labelledby="start-node-maintenance-modal-title"
>
<ModalHeader
title={t('metal3-plugin~Start Maintenance')}
data-test-id="modal-title"
labelId="start-node-maintenance-modal-title"
/>
<ModalBody>
<Stack hasGutter>
<StackItem>
{t(
Expand Down Expand Up @@ -99,16 +107,22 @@ export const StartNodeMaintenanceModal: OverlayComponent<StartNodeMaintenanceMod
</StackItem>
)}
</Stack>
</PfModalBody>
<PfModalFooter>
{errorMessage && <ErrorMessage message={errorMessage} />}
<Button variant="primary" onClick={submit} isLoading={inProgress} isDisabled={!loaded}>
</ModalBody>
<ModalFooterWithAlerts errorMessage={errorMessage}>
<Button
variant="primary"
onClick={submit}
isLoading={inProgress}
isDisabled={!loaded}
data-test="confirm-action"
id="confirm-action"
>
{t('metal3-plugin~Start Maintenance')}
</Button>
<Button variant="link" onClick={closeOverlay}>
<Button variant="link" onClick={closeOverlay} data-test-id="modal-cancel-action">
{t('metal3-plugin~Cancel')}
</Button>
</PfModalFooter>
</ModalFooterWithAlerts>
</Modal>
);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ export const topologyPO = {
zoomIn: '#zoom-in',
zoomOut: '#zoom-out',
saveModal: '[data-test="confirm-action"]',
modalContent: '[class="modal-content"]',
fitToScreen: '#fit-to-screen',
emptyGraph: '[data-test-id="topology"]',
nodeLabel: 'g[class$="topology__node__label"]',
Expand Down Expand Up @@ -77,14 +76,14 @@ export const topologyPO = {
actionsDropDown: '[data-test-id="actions-menu-button"]',
showPodCount: '[id$=show-pod-count]',
dialog: '[data-test="topology-sidepane"]',
title: '[role="dialog"] h1',
title: '[data-test="topology-sidepane"] h1',
knativeServiceIcon: '[title="Service"]',
tabs: '[role="dialog"] li button',
tabs: '[data-test="topology-sidepane"] button[role="tab"]',
sectionTitle: 'h2',
close: '[data-test="sidebar-close-button]',
labelsList: '[data-test="label-list"]',
editAnnotations: '[data-test="edit-annotations"]',
tabName: '[role="dialog"] li button',
tabName: '[data-test="topology-sidepane"] button[role="tab"]',
healthCheckAlert: 'div.odc-topology-sidebar-alert',
resourceQuotaAlert: '[class*=warning]',
podScaleUP: '[aria-label="Increase the Pod count"]',
Expand All @@ -103,7 +102,7 @@ export const topologyPO = {
labelsEdit: '[data-test="Labels-details-item__edit-button"]',
},
resourcesTab: {
startLastRun: '[role="dialog"] li.pipeline-overview div button',
startLastRun: '[data-test="topology-sidepane"] li.pipeline-overview div button',
pipelineRuns: 'li.odc-pipeline-run-item',
routeLink: '[data-test-id="route-link"]',
waitingPods: 'button[data-test="waiting-pods"]',
Expand Down Expand Up @@ -156,7 +155,6 @@ export const topologyPO = {
},
highlightNode: '.is-filtered',
createSecret: {
secretForm: '.co-create-secret-form.modal-content',
createSecretButton: '[data-test="images-section"] button',
secretDropDown: '[id="form-ns-dropdown-formData-imagePullSecret-field"]',
secretDropDownItem: '[data-test="console-select-item"]',
Expand Down
Original file line number Diff line number Diff line change
@@ -1,21 +1,4 @@
import { exportApplication, exportModalButton } from '../../page-objects/export-applications-po';

export function clickVisibleButton(flag: boolean = true) {
/* eslint-disable promise/catch-or-return */

cy.get('body').then(($body) => {
if ($body.find('.modal-body').text().includes(' is in progress')) {
cy.log('Close progress modal');
cy.get(exportModalButton('Ok')).should('be.visible').click();
cy.get(exportModalButton('Ok')).should('not.exist');
cy.get(exportApplication.exportApplicationButton).should('be.visible').click();
if (flag === true) {
cy.get(exportModalButton('Restart Export')).should('be.visible').click();
cy.get(exportModalButton('Restart Export')).should('not.exist');
}
}
});
}
import { exportApplication } from '../../page-objects/export-applications-po';

export const exportOfApplication = {
exportApplicationFresh: () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ When('user clicks on Export Application button', () => {
});

When('user clicks on Ok button on Export Application modal to start the export', () => {
cy.get('.modal-body').contains('Do you want to export your application?');
cy.get('.pf-v6-c-modal-box__body').contains('Do you want to export your application?');
cy.byTestID('close-btn').should('be.visible').click();
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ Then('user will see {string} modal', (modalName: string) => {

Then('user will see alert {string}', (alertName: string) => {
app.waitForDocumentLoad();
cy.get('.modal-body-content').contains(alertName).should('be.visible');
cy.get('.pf-v6-c-modal-box__body').contains(alertName).should('be.visible');
modal.cancel();
});

Expand Down
Loading