CONSOLE-4978: Add Installed Software page with OLMv0 and OLMv1 tabs#16064
CONSOLE-4978: Add Installed Software page with OLMv0 and OLMv1 tabs#16064TheRealJon wants to merge 2 commits intoopenshift:mainfrom
Conversation
- Create InstalledSoftwarePage with tabbed interface showing: - Cluster extensions (OLMv1) tab with tech preview badge (default) - Operators (OLMv0) tab for legacy ClusterServiceVersions - Add ClusterExtensionListPage using ConsoleDataView pattern - Display ClusterExtensions with columns: Name, Status, Version, Channels, Namespace, Package - Filter cluster-scoped resources by spec.namespace in namespace contexts - Render channels as gray outline labels - Add delete action via default actions provider - Create shared OLMv1TechPreviewBadge component with info popover - Use outline variant for tech preview labels - Include popover explaining OLMv1 features - Share between catalog toolbar and installed software tab - Add showTitle prop to ClusterServiceVersionsPage to hide duplicate title in tab context - Update HorizontalNav to render badges in tab titles - Move InstalledSoftwarePage and navigation to operator-lifecycle-manager-v1 package - Add "Installed Software" navigation with OLMV1_ENABLED flag requirement Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
@TheRealJon: This pull request references CONSOLE-4978 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 story 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-robot
added
the
jira/valid-reference
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: TheRealJon 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
component/olm
|
@TheRealJon: This pull request references CONSOLE-4978 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 story 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. |
|
@TheRealJon: This pull request references CONSOLE-4978 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 story 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. |
📝 WalkthroughWalkthroughThe pull request refactors the OLMv1 operator management interface by introducing a new "Installed Software" page that consolidates cluster extensions and operators under a tabbed view. The changes include removing the "Installed Operators" resource-cluster navigation entry, creating new ClusterExtensionListPage and InstalledSoftwarePage components, and introducing a tech preview badge component to replace previous popover implementations. A new ClusterExtensionKind type is added, new console extension entries are registered, locale entries are updated, and module exports are configured. The ClusterServiceVersionsPage gains an optional showTitle prop, while horizontal navigation gains badge rendering capability. 🚥 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.
Actionable comments posted: 6
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/utils/horizontal-nav.tsx (1)
371-388:⚠️ Potential issue | 🟡 Minor
HorizontalNavFacadesilently drops badge — document or extend the facade.
NavPage(the SDK type) does not include abadgeproperty, but the internalPagetype addsbadge?: ReactNodeand renders it. Plugin consumers usingHorizontalNavFacadecannot supply badges becauseHorizontalNavFacadeProps.pagesacceptsNavPage[]without badge support. If this constraint is intentional (badges are console-internal only), add a brief comment on the facade to make that explicit. If plugins should be able to supply badges in the future, this facade API gap is worth tracking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/public/components/utils/horizontal-nav.tsx` around lines 371 - 388, HorizontalNavFacade currently accepts pages: HorizontalNavFacadeProps.pages typed as NavPage[] which omits the internal Page.badge, so any badge passed by plugins will be dropped—either explicitly document this limitation on HorizontalNavFacade (add a comment above HorizontalNavFacade stating badges are internal-only) or update the facade to accept and forward badge support by widening the pages prop to include badge (e.g. change HorizontalNavFacadeProps.pages to accept NavPage & { badge?: React.ReactNode } or the internal Page type) and pass those through to HorizontalNav so badges render; reference HorizontalNavFacade, HorizontalNavFacadeProps, NavPage, Page and badge when making the change.
🧹 Nitpick comments (2)
frontend/public/components/utils/horizontal-nav.tsx (1)
212-212: Prefer CSS spacing over whitespace-in-fragment for badge separation.
<> {badge}</>inserts a raw space text node. This is fragile under zoom, RTL locales, or ifTabTitleTextswitches its internal layout. A styled wrapper is more maintainable:♻️ Suggested refactor
- {badge && <> {badge}</>} + {badge && <span style={{ marginInlineStart: 'var(--pf-t--global--spacer--xs)' }}>{badge}</span>}Or use a PF token class to stay consistent with the design system's spacing scale.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/public/components/utils/horizontal-nav.tsx` at line 212, Replace the raw space fragment used to separate the badge (currently rendered as {badge && <> {badge}</>}) with a styled wrapper element so spacing comes from CSS not a text node; update the render in horizontal-nav.tsx to render the badge inside a span or small wrapper component (e.g., <span className="tab-badge">...</span> or a dedicated TabBadge component) and add a CSS rule or use a PatternFly spacing token class to provide margin-left (or appropriate RTL-aware spacing) so spacing is consistent regardless of zoom, locale, or internal layout changes in TabTitleText.frontend/packages/operator-lifecycle-manager-v1/src/components/cluster-extension/ClusterExtensionListPage.tsx (1)
25-33: Use a named ID for the actions column instead of an empty string.
tableColumnInfo[6].id = ''is an implicit sentinel that's fragile — a second empty-id entry would silently collide in therowCellsmap and the columns array.♻️ Proposed refactor
export const tableColumnInfo = [ { id: 'name' }, { id: 'status' }, { id: 'version' }, { id: 'channel' }, { id: 'namespace' }, { id: 'package' }, - { id: '' }, + { id: 'actions' }, ];And update the column definition at line 152:
- title: '', - id: tableColumnInfo[6].id, + title: '', + id: tableColumnInfo[6].id, // 'actions'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/operator-lifecycle-manager-v1/src/components/cluster-extension/ClusterExtensionListPage.tsx` around lines 25 - 33, Replace the empty-string sentinel used for the actions column in tableColumnInfo with a named id (e.g., 'actions' or 'actionsColumn') and update any corresponding references that build the columns or rowCells map (search for usage of tableColumnInfo[6] or the actions column definition in the component) so the code uses the new named id instead of ''. Specifically, change the entry in tableColumnInfo from { id: '' } to { id: 'actions' } and update the column definition (the actions column renderer / rowCells lookup) to reference 'actions' so the columns array and rowCells map no longer rely on a fragile empty-string sentinel.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/packages/operator-lifecycle-manager-v1/console-extensions.json`:
- Around line 104-114: The extension entry for type
"console.action/resource-provider" that targets the ClusterExtension model and
uses provider "$codeRef": "defaultActionsProvider.useDefaultActionsProvider"
needs a feature-flag guard; add a "flags" array containing
"CLUSTER_EXTENSION_API" to the extension's properties so the provider is only
registered when the ClusterExtension API is present. Locate the extension object
with model {group: "olm.operatorframework.io", version: "v1", kind:
"ClusterExtension"} and add the flags key to its properties (mirroring other
ClusterExtension entries like the console.resource/create entry).
In
`@frontend/packages/operator-lifecycle-manager-v1/src/components/cluster-extension/ClusterExtensionListPage.tsx`:
- Around line 42-44: The status computation currently only returns the
'Installed' condition reason and falls back to empty; update the logic used
where `status` is derived from `obj.status?.conditions` so it prefers the
Installed reason but if missing falls back to the first available informative
condition (for example: first condition with a non-empty reason, or if none, the
first condition's type or a Ready-equivalent like the first non-True condition's
reason/type) so the status column never renders as a bare dash; locate the
`status` assignment (the line using `.find((condition) => condition.type ===
'Installed')?.reason`) and replace it with fallback selection of
condition.reason or condition.type in the described order.
In
`@frontend/packages/operator-lifecycle-manager-v1/src/components/installed-software/InstalledSoftwarePage.tsx`:
- Around line 16-44: The pages array currently builds inline component factories
inside useMemo (the arrow functions that return <ClusterExtensionListPage .../>
and the <AsyncComponent .../>), which creates new component references on every
[ns, t] change and forces unmount/remount; to fix, replace those inline
factories with stable module-scope components (e.g., define a top-level
OLMv0OperatorsTab component that accepts namespace prop and returns the
AsyncComponent, and ensure the ClusterExtensionListPage usage is provided as a
stable component or wrapper), then update pages to reference these stable
components and only keep the minimal dependencies for memoizing the name string
(use t just to compute name/nameKey) so that ns is passed as a prop and React
preserves component identity and local state.
In
`@frontend/packages/operator-lifecycle-manager-v1/src/components/OLMv1TechPreviewBadge.tsx`:
- Around line 29-34: The icon inside the Button is exposed to assistive tech and
may be redundantly announced; update the JSX where OutlinedQuestionCircleIcon is
used (the Button with aria-label in OLMv1TechPreviewBadge) to mark the icon as
decorative by adding aria-hidden="true" to the OutlinedQuestionCircleIcon
element so the button label remains the only accessible name.
In
`@frontend/packages/operator-lifecycle-manager/src/components/clusterserviceversion.tsx`:
- Around line 770-774: The InstalledSoftwarePage should own the DocumentTitle
when the child clusterserviceversion component is mounted with
showTitle={false}; update the InstalledSoftwarePage component to render
<DocumentTitle>{t('olm-v1~Installed Software')}</DocumentTitle> (i.e., add a
DocumentTitle wrapper with that translated string) so the browser tab title is
set even when clusterserviceversion.tsx receives showTitle={false}; ensure you
do this in the InstalledSoftwarePage render path (not in
clusterserviceversion.tsx) and keep the clusterserviceversion prop handling
(const showTitle = props.showTitle !== false) unchanged.
In `@frontend/public/components/utils/horizontal-nav.tsx`:
- Around line 209-214: The Tab title currently renders the interactive badge
inside TabTitleText (see TabTitleText, nameKey/t(nameKey), name,
badge/OLMv1TechPreviewBadge), which breaks PF6 patterns; move the badge out of
the title and render it via the Tab's actions prop (or use TabAction) so the
title contains only text/icons and the OLMv1TechPreviewBadge (the interactive
Button/Popover) is rendered as an adjacent action; update the Tab element that
currently sets title={<TabTitleText>...{badge}...</TabTitleText>} to instead set
title={<TabTitleText>{nameKey ? t(nameKey) : name}</TabTitleText>} and pass the
badge component into the Tab's actions prop.
---
Outside diff comments:
In `@frontend/public/components/utils/horizontal-nav.tsx`:
- Around line 371-388: HorizontalNavFacade currently accepts pages:
HorizontalNavFacadeProps.pages typed as NavPage[] which omits the internal
Page.badge, so any badge passed by plugins will be dropped—either explicitly
document this limitation on HorizontalNavFacade (add a comment above
HorizontalNavFacade stating badges are internal-only) or update the facade to
accept and forward badge support by widening the pages prop to include badge
(e.g. change HorizontalNavFacadeProps.pages to accept NavPage & { badge?:
React.ReactNode } or the internal Page type) and pass those through to
HorizontalNav so badges render; reference HorizontalNavFacade,
HorizontalNavFacadeProps, NavPage, Page and badge when making the change.
---
Nitpick comments:
In
`@frontend/packages/operator-lifecycle-manager-v1/src/components/cluster-extension/ClusterExtensionListPage.tsx`:
- Around line 25-33: Replace the empty-string sentinel used for the actions
column in tableColumnInfo with a named id (e.g., 'actions' or 'actionsColumn')
and update any corresponding references that build the columns or rowCells map
(search for usage of tableColumnInfo[6] or the actions column definition in the
component) so the code uses the new named id instead of ''. Specifically, change
the entry in tableColumnInfo from { id: '' } to { id: 'actions' } and update the
column definition (the actions column renderer / rowCells lookup) to reference
'actions' so the columns array and rowCells map no longer rely on a fragile
empty-string sentinel.
In `@frontend/public/components/utils/horizontal-nav.tsx`:
- Line 212: Replace the raw space fragment used to separate the badge (currently
rendered as {badge && <> {badge}</>}) with a styled wrapper element so spacing
comes from CSS not a text node; update the render in horizontal-nav.tsx to
render the badge inside a span or small wrapper component (e.g., <span
className="tab-badge">...</span> or a dedicated TabBadge component) and add a
CSS rule or use a PatternFly spacing token class to provide margin-left (or
appropriate RTL-aware spacing) so spacing is consistent regardless of zoom,
locale, or internal layout changes in TabTitleText.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (12)
frontend/packages/operator-lifecycle-manager-v1/console-extensions.jsonfrontend/packages/operator-lifecycle-manager-v1/locales/en/olm-v1.jsonfrontend/packages/operator-lifecycle-manager-v1/package.jsonfrontend/packages/operator-lifecycle-manager-v1/src/actions/providers/default-actions-provider.tsfrontend/packages/operator-lifecycle-manager-v1/src/components/OLMv1Switch.tsxfrontend/packages/operator-lifecycle-manager-v1/src/components/OLMv1TechPreviewBadge.tsxfrontend/packages/operator-lifecycle-manager-v1/src/components/cluster-extension/ClusterExtensionListPage.tsxfrontend/packages/operator-lifecycle-manager-v1/src/components/cluster-extension/index.tsfrontend/packages/operator-lifecycle-manager-v1/src/components/installed-software/InstalledSoftwarePage.tsxfrontend/packages/operator-lifecycle-manager-v1/src/types.tsfrontend/packages/operator-lifecycle-manager/src/components/clusterserviceversion.tsxfrontend/public/components/utils/horizontal-nav.tsx
| { | ||
| "type": "console.action/resource-provider", | ||
| "properties": { | ||
| "model": { | ||
| "group": "olm.operatorframework.io", | ||
| "version": "v1", | ||
| "kind": "ClusterExtension" | ||
| }, | ||
| "provider": { "$codeRef": "defaultActionsProvider.useDefaultActionsProvider" } | ||
| } | ||
| }, |
There was a problem hiding this comment.
console.action/resource-provider for ClusterExtension is missing a flags guard.
Every other ClusterExtension-specific extension entry in this file is gated by CLUSTER_EXTENSION_API (e.g., console.resource/create at lines 89–103). Without the flag, the delete action provider is registered unconditionally — even on clusters where the ClusterExtension CRD is absent — which can surface errors or stale menu items in the kebab menu.
🔒 Proposed fix
{
"type": "console.action/resource-provider",
"properties": {
"model": {
"group": "olm.operatorframework.io",
"version": "v1",
"kind": "ClusterExtension"
},
"provider": { "$codeRef": "defaultActionsProvider.useDefaultActionsProvider" }
- }
+ },
+ "flags": {
+ "required": ["CLUSTER_EXTENSION_API"]
+ }
},📝 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": "console.action/resource-provider", | |
| "properties": { | |
| "model": { | |
| "group": "olm.operatorframework.io", | |
| "version": "v1", | |
| "kind": "ClusterExtension" | |
| }, | |
| "provider": { "$codeRef": "defaultActionsProvider.useDefaultActionsProvider" } | |
| } | |
| }, | |
| { | |
| "type": "console.action/resource-provider", | |
| "properties": { | |
| "model": { | |
| "group": "olm.operatorframework.io", | |
| "version": "v1", | |
| "kind": "ClusterExtension" | |
| }, | |
| "provider": { "$codeRef": "defaultActionsProvider.useDefaultActionsProvider" } | |
| }, | |
| "flags": { | |
| "required": ["CLUSTER_EXTENSION_API"] | |
| } | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/packages/operator-lifecycle-manager-v1/console-extensions.json`
around lines 104 - 114, The extension entry for type
"console.action/resource-provider" that targets the ClusterExtension model and
uses provider "$codeRef": "defaultActionsProvider.useDefaultActionsProvider"
needs a feature-flag guard; add a "flags" array containing
"CLUSTER_EXTENSION_API" to the extension's properties so the provider is only
registered when the ClusterExtension API is present. Locate the extension object
with model {group: "olm.operatorframework.io", version: "v1", kind:
"ClusterExtension"} and add the flags key to its properties (mirroring other
ClusterExtension entries like the console.resource/create entry).
| const status = | ||
| obj.status?.conditions?.find((condition) => condition.type === 'Installed')?.reason || ''; | ||
|
|
There was a problem hiding this comment.
Status column may show — for resources not yet in the Installed condition.
Only the 'Installed' condition's reason is used. A freshly created or progressing ClusterExtension that hasn't reached that condition yet will silently render DASH, giving users no feedback about its current state (e.g., Progressing, Failed, etc.).
Consider falling back to other notable condition types (e.g., the first non-True condition or a general Ready-equivalent), or at minimum the first available condition reason, so the status column is never vacuously empty for a live resource.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@frontend/packages/operator-lifecycle-manager-v1/src/components/cluster-extension/ClusterExtensionListPage.tsx`
around lines 42 - 44, The status computation currently only returns the
'Installed' condition reason and falls back to empty; update the logic used
where `status` is derived from `obj.status?.conditions` so it prefers the
Installed reason but if missing falls back to the first available informative
condition (for example: first condition with a non-empty reason, or if none, the
first condition's type or a Ready-equivalent like the first non-True condition's
reason/type) so the status column never renders as a bare dash; locate the
`status` assignment (the line using `.find((condition) => condition.type ===
'Installed')?.reason`) and replace it with fallback selection of
condition.reason or condition.type in the described order.
| const pages: Page[] = useMemo( | ||
| () => [ | ||
| { | ||
| href: '', | ||
| name: t('olm-v1~Cluster extensions (OLMv1)'), | ||
| badge: <OLMv1TechPreviewBadge />, | ||
| component: () => <ClusterExtensionListPage namespace={ns} />, | ||
| }, | ||
| { | ||
| href: 'olmv0-operators', | ||
| // t('olm-v1~Operators (OLMv0)') | ||
| nameKey: 'olm-v1~Operators (OLMv0)', | ||
| component: () => ( | ||
| <AsyncComponent | ||
| loader={() => | ||
| import( | ||
| '@console/operator-lifecycle-manager/src/components/clusterserviceversion' | ||
| ).then((m) => m.ClusterServiceVersionsPage) | ||
| } | ||
| namespace={ns} | ||
| kind="" | ||
| resourceDescriptions={[]} | ||
| showTitle={false} | ||
| /> | ||
| ), | ||
| }, | ||
| ], | ||
| [ns, t], | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Inline component factories in useMemo cause full unmount/remount on every ns change.
Every time useMemo([ns, t]) re-evaluates, both () => <ClusterExtensionListPage .../> and () => <AsyncComponent .../> produce new function references. React uses reference equality to identify component types, so a new reference = a new type = the currently rendered tab is fully unmounted and a fresh one is mounted. This discards all local state (filters, sort order, pagination) inside the tabs and doubles DOM churn on every namespace switch.
The t dependency is also unnecessarily broad here — only the name string depends on t, not the component constructors.
Preferred fix — use stable references with pageData:
-const pages: Page[] = useMemo(
- () => [
- {
- href: '',
- name: t('olm-v1~Cluster extensions (OLMv1)'),
- badge: <OLMv1TechPreviewBadge />,
- component: () => <ClusterExtensionListPage namespace={ns} />,
- },
- {
- href: 'olmv0-operators',
- // t('olm-v1~Operators (OLMv0)')
- nameKey: 'olm-v1~Operators (OLMv0)',
- component: () => (
- <AsyncComponent
- loader={() =>
- import(
- '@console/operator-lifecycle-manager/src/components/clusterserviceversion'
- ).then((m) => m.ClusterServiceVersionsPage)
- }
- namespace={ns}
- kind=""
- resourceDescriptions={[]}
- showTitle={false}
- />
- ),
- },
- ],
- [ns, t],
-);
+const clusterExtensionTab: Page<{ namespace?: string }> = useMemo(
+ () => ({
+ href: '',
+ name: t('olm-v1~Cluster extensions (OLMv1)'),
+ badge: <OLMv1TechPreviewBadge />,
+ component: ClusterExtensionListPage,
+ pageData: { namespace: ns },
+ }),
+ [ns, t],
+);
+
+const olmv0Tab: Page<{ namespace?: string }> = useMemo(
+ () => ({
+ href: 'olmv0-operators',
+ nameKey: 'olm-v1~Operators (OLMv0)',
+ component: OLMv0OperatorsTab,
+ pageData: { namespace: ns },
+ }),
+ [ns],
+);
+
+const pages = useMemo(() => [clusterExtensionTab, olmv0Tab], [clusterExtensionTab, olmv0Tab]);Where OLMv0OperatorsTab is a stable component defined at module scope (or via lazy):
// at module level — stable reference, never recreated
const OLMv0OperatorsTab: FC<{ namespace?: string }> = ({ namespace }) => (
<AsyncComponent
loader={() =>
import('@console/operator-lifecycle-manager/src/components/clusterserviceversion').then(
(m) => m.ClusterServiceVersionsPage,
)
}
namespace={namespace}
kind=""
resourceDescriptions={[]}
showTitle={false}
/>
);This keeps component identity stable across namespace changes; only props change, so React re-renders without unmounting.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@frontend/packages/operator-lifecycle-manager-v1/src/components/installed-software/InstalledSoftwarePage.tsx`
around lines 16 - 44, The pages array currently builds inline component
factories inside useMemo (the arrow functions that return
<ClusterExtensionListPage .../> and the <AsyncComponent .../>), which creates
new component references on every [ns, t] change and forces unmount/remount; to
fix, replace those inline factories with stable module-scope components (e.g.,
define a top-level OLMv0OperatorsTab component that accepts namespace prop and
returns the AsyncComponent, and ensure the ClusterExtensionListPage usage is
provided as a stable component or wrapper), then update pages to reference these
stable components and only keep the minimal dependencies for memoizing the name
string (use t just to compute name/nameKey) so that ns is passed as a prop and
React preserves component identity and local state.
| <Button | ||
| icon={<OutlinedQuestionCircleIcon />} | ||
| aria-label={t('olm-v1~OLMv1 information')} | ||
| variant="link" | ||
| isInline | ||
| /> |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
PatternFly v6 React Button icon prop aria-hidden decorative
💡 Result:
In PatternFly v6, treat the icon you pass to a React Button as decorative by default:
- Icon-only (or icon + text) button: put the accessible name on the
Button(via visible text oraria-label), and keep the icon out of the accessibility tree witharia-hidden="true". PatternFly explicitly recommendsaria-labelwhen a button has no visible text, andaria-hidden="true"for decorative icons. [1], [2] - Using
@patternfly/react-icons: if you render an icon without atitleprop, the icon component defaults toaria-hidden="true"; only addtitlewhen you intentionally want the SVG to be exposed to assistive tech. [3]
Example:
import { Button } from '@patternfly/react-core';
import TimesIcon from '@patternfly/react-icons/dist/esm/icons/times-icon';
<Button variant="plain" aria-label="Close">
<TimesIcon aria-hidden="true" /> {/* or omit title to get aria-hidden by default */}
</Button>Sources: PatternFly Button accessibility guidance [1], PatternFly “Develop for accessibility” icon guidance [2], @patternfly/react-icons accessibility behavior (title vs aria-hidden) [3].
🌐 Web query:
PatternFly 6 Button component icon accessibility aria-label
💡 Result:
In PatternFly v6, if your Button’s visible content is only an icon (no readable text), you must give the Button an accessible name via aria-label (React prop aria-label, or HTML attribute aria-label). (patternfly.org)
React (PF6)
<Button variant="plain" aria-label="Close">
<TimesIcon aria-hidden="true" />
</Button>- Put the label on the
<Button>, not on the icon. (patternfly.org) - Mark the icon as decorative (e.g.,
aria-hidden="true") so screen readers don’t announce it redundantly. (patternfly.org)
HTML (PF6)
<button class="pf-v6-c-button pf-m-plain" aria-label="Close">
<i class="..." aria-hidden="true"></i>
</button>Citations:
- 1: https://www.patternfly.org/components/button/accessibility?utm_source=openai
- 2: https://www.patternfly.org/components/button/accessibility?utm_source=openai
- 3: https://www.patternfly.org/accessibility/develop-for-accessibility?utm_source=openai
- 4: https://www.patternfly.org/components/button/accessibility?utm_source=openai
Add aria-hidden="true" to the icon to prevent redundant screen reader announcement.
This button displays only an icon, with the accessible name provided via aria-label on the Button element itself. PatternFly v6 guidance explicitly recommends marking decorative icons with aria-hidden="true" to avoid screen readers announcing the icon name in addition to the button label. Without it, the icon is exposed to the accessibility tree and may be redundantly announced.
Proposed fix
- icon={<OutlinedQuestionCircleIcon />}
+ icon={<OutlinedQuestionCircleIcon aria-hidden />}📝 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.
| <Button | |
| icon={<OutlinedQuestionCircleIcon />} | |
| aria-label={t('olm-v1~OLMv1 information')} | |
| variant="link" | |
| isInline | |
| /> | |
| <Button | |
| icon={<OutlinedQuestionCircleIcon aria-hidden />} | |
| aria-label={t('olm-v1~OLMv1 information')} | |
| variant="link" | |
| isInline | |
| /> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@frontend/packages/operator-lifecycle-manager-v1/src/components/OLMv1TechPreviewBadge.tsx`
around lines 29 - 34, The icon inside the Button is exposed to assistive tech
and may be redundantly announced; update the JSX where
OutlinedQuestionCircleIcon is used (the Button with aria-label in
OLMv1TechPreviewBadge) to mark the icon as decorative by adding
aria-hidden="true" to the OutlinedQuestionCircleIcon element so the button label
remains the only accessible name.
| const showTitle = props.showTitle !== false; | ||
|
|
||
| return ( | ||
| <> | ||
| <DocumentTitle>{title}</DocumentTitle> | ||
| {showTitle && <DocumentTitle>{title}</DocumentTitle>} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -type f -name "*InstalledSoftware*" | head -20Repository: openshift/console
Length of output: 231
🏁 Script executed:
rg -l "InstalledSoftwarePage" --type=tsx --type=ts -g "!*.test.*" | head -20Repository: openshift/console
Length of output: 88
🏁 Script executed:
rg -n "DocumentTitle" --type=tsx --type=ts -g "!*.test.*" -B2 -A2 | head -100Repository: openshift/console
Length of output: 88
🏁 Script executed:
fd -e tsx -e ts InstalledSoftwarePageRepository: openshift/console
Length of output: 168
🏁 Script executed:
fd -e tsx -e ts ".*InstalledSoftware.*"Repository: openshift/console
Length of output: 168
🏁 Script executed:
rg -g "*.tsx" -g "*.ts" "InstalledSoftwarePage" | head -20Repository: openshift/console
Length of output: 356
🏁 Script executed:
cat -n frontend/packages/operator-lifecycle-manager-v1/src/components/installed-software/InstalledSoftwarePage.tsxRepository: openshift/console
Length of output: 2132
🏁 Script executed:
fd -e tsx -e ts "MultiTabListPage"Repository: openshift/console
Length of output: 144
🏁 Script executed:
rg -g "*.tsx" -g "*.ts" "class MultiTabListPage|function MultiTabListPage|const MultiTabListPage" -A 30Repository: openshift/console
Length of output: 3663
🏁 Script executed:
wc -l frontend/packages/console-shared/src/components/multi-tab-list/MultiTabListPage.tsxRepository: openshift/console
Length of output: 148
🏁 Script executed:
cat -n frontend/packages/console-shared/src/components/multi-tab-list/MultiTabListPage.tsx | head -100Repository: openshift/console
Length of output: 4286
🏁 Script executed:
cat -n frontend/packages/console-shared/src/components/multi-tab-list/MultiTabListPage.tsx | tail -20Repository: openshift/console
Length of output: 766
🏁 Script executed:
fd -e tsx -e ts "PageHeading"Repository: openshift/console
Length of output: 224
🏁 Script executed:
rg -g "*.tsx" -g "*.ts" "DocumentTitle" frontend/packages/console-shared/src/components/heading/ -B 2 -A 2Repository: openshift/console
Length of output: 43
🏁 Script executed:
cat -n frontend/packages/console-shared/src/components/heading/PageHeading.tsxRepository: openshift/console
Length of output: 4031
🏁 Script executed:
rg -g "*.tsx" -g "*.ts" "HorizontalNav" frontend/packages/console-shared/src/ frontend/packages/operator-lifecycle-manager-v1/ -lRepository: openshift/console
Length of output: 144
🏁 Script executed:
rg -g "*.tsx" -g "*.ts" "PageTitleContext" -A 3 -B 3 | head -50Repository: openshift/console
Length of output: 4780
🏁 Script executed:
rg -g "*.tsx" -g "*.ts" "DocumentTitle" -l | head -20Repository: openshift/console
Length of output: 1678
🏁 Script executed:
rg -g "*.tsx" -g "*.ts" "DocumentTitle" -B 2 -A 2 | head -80Repository: openshift/console
Length of output: 9508
Verify InstalledSoftwarePage owns the DocumentTitle when showTitle={false}
When this component is mounted as a tab with showTitle={false}, no DocumentTitle is emitted here. The parent InstalledSoftwarePage does not render a DocumentTitle either, so the browser tab title will be stale (reflecting whatever was last set) when the OLMv0 tab is active. InstalledSoftwarePage should render <DocumentTitle>{t('olm-v1~Installed Software')}</DocumentTitle> to own this responsibility.
🤖 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/clusterserviceversion.tsx`
around lines 770 - 774, The InstalledSoftwarePage should own the DocumentTitle
when the child clusterserviceversion component is mounted with
showTitle={false}; update the InstalledSoftwarePage component to render
<DocumentTitle>{t('olm-v1~Installed Software')}</DocumentTitle> (i.e., add a
DocumentTitle wrapper with that translated string) so the browser tab title is
set even when clusterserviceversion.tsx receives showTitle={false}; ensure you
do this in the InstalledSoftwarePage render path (not in
clusterserviceversion.tsx) and keep the clusterserviceversion prop handling
(const showTitle = props.showTitle !== false) unchanged.
|
/assign @Leo6Leo |
|
QE Approver Docs Approver: PX Approver: |
|
@TheRealJon: GitHub didn't allow me to assign the following users: jseseCCS, rh-joshbeverly. Note that only openshift members with read permissions, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time. 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 kubernetes-sigs/prow repository. |
|
@TheRealJon: 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. |
|
/label px-approved |
openshift-ci
bot
added
the
px-approved
6 participants
Summary
Implements the Installed Software page with a tabbed interface for viewing both OLMv0 Operators and OLMv1 Cluster Extensions in a unified location.
Changes
spec.namespacein namespace contextsshowTitleprop to hide duplicate titles in tab contextArchitecture
operator-lifecycle-manager-v1package🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Improvements