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 @@ -78,7 +78,7 @@ export const deleteApplication = {
};

export const deleteRevision = {
verifyMessage: (message: string) => cy.get('form p').should('contain.text', message),
verifyMessage: (message: string) => cy.get('[role="dialog"] p').should('contain.text', message),
clickOK: () => cy.byLegacyTestID('modal-cancel-action').click(),
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ When('user adds new label {string}', (labelName: string) => {

When('user clicks on the Save button on the modal', () => {
cy.get(topologyPO.graph.saveModal).click();
cy.get(topologyPO.graph.modalContent).should('not.exist');
cy.byLegacyTestID('modal-title').should('not.exist');
cy.reload();
app.waitForLoad();
perspective.switchTo(switchPerspective.Developer);
Expand All @@ -149,7 +149,7 @@ When('user selects {string} from Subscriber drop down', (subscriberName: string)

When('user clicks on Add button', () => {
cy.get(eventingPO.broker.confirm).click();
cy.get('[class*="modal-dialog"]').should('not.exist');
cy.byLegacyTestID('modal-title').should('not.exist');
});

Then('user will see {string} created', (triggerName) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ When('user adds the label {string}', (label: string) => {

When('user clicks on the Save button on the modal to save labels and close the modal', () => {
cy.get(eventingPO.channel.save).click('center', { force: true });
cy.get('[role="dialog"]').should('not.exist');
cy.byLegacyTestID('modal-title').should('not.exist');
navigateTo(devNavigationMenu.Add);
});

Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import type { FC } from 'react';
import { useCallback, useEffect } from 'react';
import { Modal } from '@patternfly/react-core';
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/modal';
import { ModalWrapper } from '@console/internal/components/factory/modal';
import type { K8sResourceKind } from '@console/internal/module/k8s';
import PubSub from './PubSub';
import { setPubSubModalLauncher } from './PubSubModalLauncher';
Expand Down Expand Up @@ -31,13 +31,14 @@ export const PubSubModalOverlay: OverlayComponent<Props> = (props) => {
}, [props]);

return (
<ModalWrapper blocking onClose={handleOverlayDismiss}>
<PubSubController
cancel={props.cancel || props.closeOverlay}
close={props.close || props.closeOverlay}
{...props}
/>
</ModalWrapper>
<Modal
isOpen
onClose={handleOverlayDismiss}
variant="small"
aria-labelledby="pub-sub-modal-title"
>
<PubSubController cancel={props.closeOverlay} close={props.closeOverlay} {...props} />
</Modal>
);
};

Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,11 @@
import type { FC } from 'react';
import { TextInputTypes } from '@patternfly/react-core';
import { Button, Form, ModalBody, ModalHeader, TextInputTypes } from '@patternfly/react-core';
import type { FormikProps, FormikValues } from 'formik';
import * as _ from 'lodash';
import { useTranslation } from 'react-i18next';
import FormSection from '@console/dev-console/src/components/import/section/FormSection';
import {
ModalTitle,
ModalBody,
ModalSubmitFooter,
} from '@console/internal/components/factory/modal';
import InputField from '@console/shared/src/components/formik-fields/InputField';
import { ModalFooterWithAlerts } from '@console/shared/src/components/modals/ModalFooterWithAlerts';
import PubSubFilter from './form-fields/PubSubFilter';
import PubSubSubscriber from './form-fields/PubSubSubscriber';

Expand All @@ -34,29 +30,39 @@ const PubSubModal: FC<Props> = ({
const { t } = useTranslation();
const dirty = values?.formData?.metadata?.name && values?.formData?.spec?.subscriber?.ref?.name;
return (
<form className="modal-content" onSubmit={handleSubmit}>
<ModalTitle>{labelTitle}</ModalTitle>
<>
<ModalHeader title={labelTitle} labelId="pub-sub-modal-title" data-test-id="modal-title" />
<ModalBody>
<FormSection fullWidth>
<InputField
type={TextInputTypes.text}
name="formData.metadata.name"
label={t('knative-plugin~Name')}
required
/>
<PubSubSubscriber cancel={cancel} />
{filterEnabled && <PubSubFilter />}
</FormSection>
<Form id="pub-sub-form" onSubmit={handleSubmit} className="pf-v6-u-mr-md">
<FormSection fullWidth>
<InputField
type={TextInputTypes.text}
name="formData.metadata.name"
label={t('knative-plugin~Name')}
required
/>
<PubSubSubscriber cancel={cancel} />
{filterEnabled && <PubSubFilter />}
</FormSection>
</Form>
</ModalBody>
<ModalSubmitFooter
inProgress={isSubmitting}
submitText={t('knative-plugin~Add')}
cancelText={t('knative-plugin~Cancel')}
submitDisabled={!dirty || !_.isEmpty(errors) || isSubmitting}
cancel={cancel}
errorMessage={status.error}
/>
</form>
<ModalFooterWithAlerts errorMessage={status.error}>
<Button
type="submit"
variant="primary"
isLoading={isSubmitting}
isDisabled={!dirty || !_.isEmpty(errors) || isSubmitting}
form="pub-sub-form"
data-test="confirm-action"
id="confirm-action"
>
{t('knative-plugin~Add')}
</Button>
<Button variant="link" onClick={cancel} type="button" data-test-id="modal-cancel-action">
{t('knative-plugin~Cancel')}
</Button>
</ModalFooterWithAlerts>
</>
);
};

Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,9 @@
import type { FC } from 'react';
import { Alert } from '@patternfly/react-core';
import { Alert, Button, Form, ModalBody, ModalHeader } from '@patternfly/react-core';
import type { FormikProps, FormikValues } from 'formik';
import { useTranslation } from 'react-i18next';
import {
ModalTitle,
ModalBody,
ModalSubmitFooter,
} from '@console/internal/components/factory/modal';
import type { K8sResourceKind } from '@console/internal/module/k8s';
import { YellowExclamationTriangleIcon } from '@console/shared';
import { ModalFooterWithAlerts } from '@console/shared/src/components/modals/ModalFooterWithAlerts';
import { KNATIVE_SERVING_LABEL } from '../../const';
import { RevisionModel } from '../../models';
import type { RevisionItems } from '../../utils/traffic-splitting-utils';
Expand All @@ -29,42 +24,52 @@ const DeleteRevisionModal: FC<Props> = (props) => {
const serviceName = deleteRevision.metadata.labels[KNATIVE_SERVING_LABEL];

return (
<form className="modal-content" onSubmit={handleSubmit}>
<ModalTitle>
<YellowExclamationTriangleIcon className="co-icon-space-r" />{' '}
{t('knative-plugin~Delete {{revlabel}}?', { revlabel: RevisionModel.label })}
</ModalTitle>
<>
<ModalHeader
title={t('knative-plugin~Delete {{revlabel}}?', { revlabel: RevisionModel.label })}
titleIconVariant="warning"
labelId="delete-revision-modal-title"
data-test-id="modal-title"
/>
<ModalBody>
<p>
{t('knative-plugin~Are you sure you want to delete ')}
<strong className="co-break-word">{deleteRevision.metadata.name}</strong>{' '}
{t('knative-plugin~from ')} <strong className="co-break-word">{serviceName}</strong>{' '}
{t('knative-plugin~in namespace ')} <strong>{deleteRevision.metadata.namespace}</strong>?
</p>
{showTraffic && (
<>
<Alert
isInline
className="co-alert"
variant="custom"
title={t(
'knative-plugin~Update the traffic distribution among the remaining Revisions',
)}
/>
<TrafficSplittingFields {...props} />
</>
)}
<Form id="delete-revision-form" onSubmit={handleSubmit} className="pf-v6-u-mr-md">
<p>
{t('knative-plugin~Are you sure you want to delete ')}
<strong className="co-break-word">{deleteRevision.metadata.name}</strong>{' '}
{t('knative-plugin~from ')} <strong className="co-break-word">{serviceName}</strong>{' '}
{t('knative-plugin~in namespace ')} <strong>{deleteRevision.metadata.namespace}</strong>
?
Comment on lines +36 to +41
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 | 🟡 Minor

Keep the confirmation sentence in one translation key.

The three separate t() calls around the <strong> markers hard-code English word order and punctuation. That won't localize cleanly once translators need to reorder revision, service, and namespace. Please switch this paragraph to a single Trans or interpolated message.

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

In
`@frontend/packages/knative-plugin/src/components/revisions/DeleteRevisionModal.tsx`
around lines 36 - 41, The confirmation text in DeleteRevisionModal currently
uses three separate t() calls around strong elements which hard-codes English
word order; replace them with a single translation key (using either i18n's t
with interpolation or the Trans component) that accepts placeholders for
revision, service and namespace (pass deleteRevision.metadata.name, serviceName,
and deleteRevision.metadata.namespace) and preserve the <strong> styling by
using Trans's components or t(..., { interpolation }) so translators can reorder
words and punctuation correctly.

</p>
{showTraffic && (
<>
<Alert
isInline
className="co-alert"
variant="custom"
title={t(
'knative-plugin~Update the traffic distribution among the remaining Revisions',
)}
/>
<TrafficSplittingFields {...props} />
</>
)}
</Form>
</ModalBody>
<ModalSubmitFooter
inProgress={isSubmitting}
submitText={t('knative-plugin~Delete')}
cancelText={t('knative-plugin~Cancel')}
cancel={cancel}
errorMessage={status.error}
submitDisabled={isSubmitting}
submitDanger
/>
</form>
<ModalFooterWithAlerts errorMessage={status.error}>
<Button
type="submit"
variant="danger"
isLoading={isSubmitting}
isDisabled={isSubmitting}
form="delete-revision-form"
>
{t('knative-plugin~Delete')}
</Button>
<Button variant="link" onClick={cancel} type="button" data-test-id="modal-cancel-action">
{t('knative-plugin~Cancel')}
</Button>
</ModalFooterWithAlerts>
</>
);
};

Expand Down
Original file line number Diff line number Diff line change
@@ -1,24 +1,17 @@
import type { FC } from 'react';
import { useCallback, useMemo } from 'react';
import { ActionGroup, Button } from '@patternfly/react-core';
import { Button, Modal, ModalBody, ModalFooter, ModalHeader } from '@patternfly/react-core';
import type { FormikHelpers, FormikValues } from 'formik';
import { Formik } from 'formik';
import { useTranslation } from 'react-i18next';
import { useNavigate } from 'react-router-dom-v5-compat';
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 {
ModalBody,
ModalFooter,
ModalTitle,
ModalWrapper,
} from '@console/internal/components/factory';
import { resourceListPathFromModel } from '@console/internal/components/utils';
import { useK8sWatchResources } from '@console/internal/components/utils/k8s-watch-hook';
import type { K8sResourceKind } from '@console/internal/module/k8s';
import { k8sKill, k8sPatch, referenceForModel } from '@console/internal/module/k8s';
import { RedExclamationCircleIcon } from '@console/shared';
import { KNATIVE_SERVING_LABEL } from '../../const';
import { ConfigurationModel, RevisionModel, ServiceModel } from '../../models';
import { getKnativeRevisionsData } from '../../topology/knative-topology-utils';
Expand Down Expand Up @@ -90,11 +83,15 @@ const DeleteRevisionModalController: FC<DeleteRevisionModalControllerProps> = ({

if (revisions.length === 0) {
return (
<form className="modal-content" onSubmit={close}>
<ModalTitle>
<RedExclamationCircleIcon className="co-icon-space-r" />
{t('knative-plugin~Unable to delete {{revlabel}}', { revlabel: RevisionModel.label })}
</ModalTitle>
<>
<ModalHeader
title={t('knative-plugin~Unable to delete {{revlabel}}', {
revlabel: RevisionModel.label,
})}
titleIconVariant="danger"
labelId="delete-revision-modal-title"
data-test-id="modal-title"
/>
Comment on lines +87 to +94
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 | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cd frontend/packages/knative-plugin && find . -name "DeleteRevisionModalController.tsx" -type f

Repository: openshift/console

Length of output: 121


🏁 Script executed:

cat -n frontend/packages/knative-plugin/src/components/revisions/DeleteRevisionModalController.tsx | head -220

Repository: openshift/console

Length of output: 8718


🌐 Web query:

PatternFly Modal aria-labelledby accessibility requirement

💡 Result:

In PatternFly, a Modal must have an accessible name. The recommended/default way is to set aria-labelledby on the modal container to the id of the element that visually titles the modal (typically the modal title element, e.g. .pf-v6-c-modal-box__title). This is an accessibility requirement because it gives the dialog its programmatic label. [1], [2]

For PatternFly React specifically:

  • Use aria-labelledby when the modal’s label comes from an element in the modal header—PatternFly notes it’s required when you use the header prop rather than the title prop. [1]
  • If there is no visible title text to reference, provide an accessible name with aria-label instead (PatternFly notes this is required in that case). [1]

Sources
[1] PatternFly Modal – Accessibility (React) (patternfly.org)
[2] PatternFly Modal – HTML docs (patternfly.org)

Citations:


🏁 Script executed:

cat -n frontend/packages/knative-plugin/src/components/revisions/DeleteRevisionModal.tsx | head -150

Repository: openshift/console

Length of output: 3479


Add aria-labelledby to the outer Modal and normalize the title id across both branches.

PatternFly's Modal component requires an accessible name via aria-labelledby on the outer dialog container. Your code paths render two different title ids—unable-delete-revision-modal-title (early return) and delete-revision-modal-title (normal flow)—making it impossible for the Modal at line 204 to reference a stable id. Normalize to a single title id and add the aria-labelledby attribute to the Modal container.

♿ Fix outline
-          labelId="unable-delete-revision-modal-title"
+          labelId="delete-revision-modal-title"
@@
-    <Modal isOpen onClose={props.closeOverlay} variant="small">
+    <Modal
+      isOpen
+      onClose={props.closeOverlay}
+      variant="small"
+      aria-labelledby="delete-revision-modal-title"
+    >
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@frontend/packages/knative-plugin/src/components/revisions/DeleteRevisionModalController.tsx`
around lines 87 - 94, The Modal's title id is inconsistent between the
early-return ModalHeader ("unable-delete-revision-modal-title") and the
normal-flow title ("delete-revision-modal-title"), preventing the outer Modal
from referencing a stable label; update both ModalHeader usages in
DeleteRevisionModalController.tsx to use a single normalized id (e.g.,
"delete-revision-modal-title") and add aria-labelledby="{that-id}" to the outer
Modal component (the Modal instance rendered in this controller) so the dialog
correctly references the title for accessibility.

<ModalBody>
<p>
{t('knative-plugin~You cannot delete the last {{revlabel}} for the {{serviceLabel}}.', {
Expand All @@ -103,19 +100,17 @@ const DeleteRevisionModalController: FC<DeleteRevisionModalControllerProps> = ({
})}
</p>
</ModalBody>
<ModalFooter inProgress={false}>
<ActionGroup className="pf-v6-c-form pf-v6-c-form__actions--right pf-v6-c-form__group--no-top-margin">
<Button
type="button"
variant="secondary"
data-test-id="modal-cancel-action"
onClick={close}
>
{t('knative-plugin~OK')}
</Button>
</ActionGroup>
<ModalFooter>
<Button
type="button"
variant="secondary"
data-test-id="modal-cancel-action"
onClick={close}
>
{t('knative-plugin~OK')}
</Button>
</ModalFooter>
</form>
</>
);
}

Expand Down Expand Up @@ -206,13 +201,18 @@ type Props = DeleteRevisionModalControllerProps & ModalComponentProps;

const DeleteRevisionModalProvider: OverlayComponent<Props> = (props) => {
return (
<ModalWrapper blocking onClose={props.closeOverlay}>
<Modal
isOpen
onClose={props.closeOverlay}
variant="small"
aria-labelledby="delete-revision-modal-title"
>
<DeleteRevisionModalController
cancel={props.closeOverlay}
close={props.closeOverlay}
{...props}
/>
</ModalWrapper>
</Modal>
);
};

Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import type { FC } from 'react';
import { useCallback } from 'react';
import { Modal } from '@patternfly/react-core';
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 { ModalWrapper } from '@console/internal/components/factory';
import type { K8sResourceKind } from '@console/internal/module/k8s';
import SinkPubsub from './SinkPubsub';

Expand All @@ -18,13 +18,16 @@ const SinkPubsubController: FC<SinkPubsubControllerProps> = ({ source, ...props

type Props = SinkPubsubControllerProps & ModalComponentProps;

const SinkPubsubModalProvider: OverlayComponent<Props> = (props) => {
return (
<ModalWrapper blocking onClose={props.closeOverlay}>
<SinkPubsubController cancel={props.closeOverlay} close={props.closeOverlay} {...props} />
</ModalWrapper>
);
};
const SinkPubsubModalProvider: OverlayComponent<Props> = (props) => (
<Modal
isOpen
onClose={props.closeOverlay}
variant="small"
aria-labelledby="sink-pubsub-modal-title"
>
<SinkPubsubController cancel={props.closeOverlay} close={props.closeOverlay} {...props} />
</Modal>
);

export const useSinkPubsubModalLauncher = (props: Props) => {
const launcher = useOverlay();
Expand Down
Loading