Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughRemoved PatternFly topology dependency and replaced topology-driven update graph with a channel/version presentational UI. Introduced VersionDropdown, readme modal, deprecation indicators, and Formik-driven version/config application; adjusted labels/translations and consolidated update UI components. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/ui-components/src/components/Catalog/EditWizard/steps/UpdateStep.tsx (1)
38-39:⚠️ Potential issue | 🟠 MajorVersion/channel changes now bypass edit-aware config merge and can reset user-edited state.
These paths reinitialize config with
getInitialAppConfig(catalogItem, ...)directly. In edit mode, that skips the wizard-level callback that hasappSpec+currentLabels, so changing channel/version can drop merged existing values/asset selections.💡 Proposed fix
--- a/libs/ui-components/src/components/Catalog/EditWizard/steps/UpdateStep.tsx +++ b/libs/ui-components/src/components/Catalog/EditWizard/steps/UpdateStep.tsx @@ - <FormSelect + <FormSelect name="channel" @@ onChange={(val) => { const latestVersion = getUpdates(catalogItem, val, currentVersion.version).sort((a, b) => semver.rcompare(a.version, b.version), )[0]?.version; const newVersion = latestVersion || currentVersion.version; setFieldValue('version', newVersion); - const appConfig = getInitialAppConfig(catalogItem, newVersion); - applyInitialConfig(setFieldValue, appConfig); + if (onVersionChange) { + onVersionChange(newVersion); + } else { + const appConfig = getInitialAppConfig(catalogItem, newVersion); + applyInitialConfig(setFieldValue, appConfig); + } }} /> @@ - <VersionDropdown catalogItem={catalogItem} versions={updates} /> + <VersionDropdown + catalogItem={catalogItem} + versions={updates} + onVersionChange={(version) => { + if (onVersionChange) { + onVersionChange(version); + return; + } + const appConfig = getInitialAppConfig(catalogItem, version); + applyInitialConfig(setFieldValue, appConfig); + }} + />--- a/libs/ui-components/src/components/Catalog/InstallWizard/steps/SpecificationsStep.tsx +++ b/libs/ui-components/src/components/Catalog/InstallWizard/steps/SpecificationsStep.tsx @@ type VersionDropdownProps = { catalogItem: CatalogItem; versions: CatalogItemVersion[]; + onVersionChange?: (version: string) => void; }; -export const VersionDropdown = ({ catalogItem, versions }: VersionDropdownProps) => { +export const VersionDropdown = ({ catalogItem, versions, onVersionChange }: VersionDropdownProps) => { @@ onChange={(val) => { + if (onVersionChange) { + onVersionChange(val); + return; + } const appConfig = getInitialAppConfig(catalogItem, val); applyInitialConfig(setFieldValue, appConfig); }}Also applies to: 86-94, 117-119
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/ui-components/src/components/Catalog/EditWizard/steps/UpdateStep.tsx` around lines 38 - 39, In UpdateStep.tsx the version/channel change handlers call getInitialAppConfig(...) directly which in edit mode discards wizard-level merged state; update the handlers (e.g., the functions that currently call getInitialAppConfig on channel/version change) to detect isEdit and instead invoke the wizard-level callback (onVersionChange or the equivalent prop) passing the merged appSpec + currentLabels (preserve existing asset selections and edited values) so the parent can perform the edit-aware merge; only fall back to getInitialAppConfig when not isEdit.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@libs/ui-components/src/components/Catalog/EditWizard/steps/UpdateGraph.tsx`:
- Around line 81-86: The icon-only Button lacks an accessible name; update the
Button invocation (the element using props icon, onClick, className) to include
a descriptive accessible label (e.g., aria-label or aria-labelledby) that
incorporates the version and state (use version and isSelected) so screen
readers can identify the control (for example "Select version X" or "Current
version X"); ensure the label is present for both clickable and non-clickable
states and does not rely solely on visual cues.
- Around line 112-141: The code assumes updateIdx is valid but if
sortedUpdates.findIndex returns -1 the later access
sortedUpdates[updateIdx].version will throw; update the logic around updateIdx
(in the block that computes updateIdx, before using sortedUpdates[updateIdx]) to
guard the -1 case—e.g. if updateIdx === -1 set content to a safe fallback (like
MoreVersions with sortedUpdates or undefined) or render an explicit "version not
found" line, and skip the current branch that slices before/after; ensure you
modify the logic that builds beforeSelectedUpd, afterSelectedUpd, getContent and
the JSX assignment to content so they only run when updateIdx >= 0.
In `@libs/ui-components/src/components/Catalog/InstalledSoftware.tsx`:
- Line 221: Remove the defensive "|| ''" fallbacks when passing resource
metadata names into the edit handler: update all calls like
onEdit(osItem.metadata.catalog, osItem.metadata.name || '') to pass
osItem.metadata.name directly (e.g., onEdit(osItem.metadata.catalog,
osItem.metadata.name)); do the same for the other occurrences in this file that
call onEdit (references: onEdit, osItem, and metadata.name) so the edit flow
receives the actual resource ID and conforms to the API contract that
metadata.name is always defined for returned/created resources.
---
Outside diff comments:
In `@libs/ui-components/src/components/Catalog/EditWizard/steps/UpdateStep.tsx`:
- Around line 38-39: In UpdateStep.tsx the version/channel change handlers call
getInitialAppConfig(...) directly which in edit mode discards wizard-level
merged state; update the handlers (e.g., the functions that currently call
getInitialAppConfig on channel/version change) to detect isEdit and instead
invoke the wizard-level callback (onVersionChange or the equivalent prop)
passing the merged appSpec + currentLabels (preserve existing asset selections
and edited values) so the parent can perform the edit-aware merge; only fall
back to getInitialAppConfig when not isEdit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 475362b0-b408-40b9-a863-25862e376cc6
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (9)
apps/ocp-plugin/package.jsonapps/standalone/package.jsonlibs/ui-components/package.jsonlibs/ui-components/src/components/Catalog/EditWizard/EditAppWizard.tsxlibs/ui-components/src/components/Catalog/EditWizard/steps/UpdateGraph.csslibs/ui-components/src/components/Catalog/EditWizard/steps/UpdateGraph.tsxlibs/ui-components/src/components/Catalog/EditWizard/steps/UpdateStep.tsxlibs/ui-components/src/components/Catalog/InstallWizard/steps/SpecificationsStep.tsxlibs/ui-components/src/components/Catalog/InstalledSoftware.tsx
💤 Files with no reviewable changes (3)
- libs/ui-components/package.json
- apps/ocp-plugin/package.json
- apps/standalone/package.json
libs/ui-components/src/components/Catalog/EditWizard/steps/UpdateGraph.tsx
Show resolved
Hide resolved
libs/ui-components/src/components/Catalog/EditWizard/steps/UpdateGraph.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
libs/ui-components/src/components/Catalog/InstalledSoftware.tsx (1)
221-221:⚠️ Potential issue | 🟡 MinorRemove
|| ''fallbacks when passingmetadata.nametoonEdit.Line 221, Line 244, Line 272, and Line 301 currently coerce IDs to
''. That can silently pass invalid identifiers into edit flows.Proposed fix
- onClick={() => onEdit(osItem.metadata.catalog, osItem.metadata.name || '')} + onClick={() => onEdit(osItem.metadata.catalog, osItem.metadata.name)} - onClick: () => onEdit(osItem.metadata.catalog, osItem.metadata.name || ''), + onClick: () => onEdit(osItem.metadata.catalog, osItem.metadata.name), - onClick: () => onEdit(app.item.metadata.catalog, app.item.metadata.name || '', app.name), + onClick: () => onEdit(app.item.metadata.catalog, app.item.metadata.name, app.name), - onClick={() => onEdit(app.item.metadata.catalog, app.item.metadata.name || '', app.name)} + onClick={() => onEdit(app.item.metadata.catalog, app.item.metadata.name, app.name)}As per coding guidelines: "Treat
resource.metadata.nameas always defined for resources returned from the API or created resources, and avoid optional chaining in these contexts; only handle undefined for partial or input payloads."Also applies to: 244-244, 272-272, 301-301
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/ui-components/src/components/Catalog/InstalledSoftware.tsx` at line 221, The code is coercing resource IDs to an empty string when calling onEdit, which can hide invalid identifiers; for each call site that uses onEdit(osItem.metadata.catalog, osItem.metadata.name || '' ) (and the other occurrences in InstalledSoftware.tsx), remove the "|| ''" fallback and pass osItem.metadata.name directly to onEdit so the real (possibly undefined) value surfaces; ensure the onEdit signature/type accepts a string (or narrow the resource type) so that metadata.name is treated as defined for API-returned resources.libs/ui-components/src/components/Catalog/EditWizard/steps/UpdateGraph.tsx (2)
112-141:⚠️ Potential issue | 🟡 MinorGuard
updateIdxbefore indexingsortedUpdates.This also remains unresolved: when
findIndexreturns-1, Line 139/140 dereferencessortedUpdates[-1]and can crash rendering.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/ui-components/src/components/Catalog/EditWizard/steps/UpdateGraph.tsx` around lines 112 - 141, The code uses updateIdx from sortedUpdates.findIndex without guarding the -1 case, so when updateIdx === -1 the code dereferences sortedUpdates[updateIdx] (e.g., sortedUpdates[-1]) and can crash; update the logic around updateIdx (the block that builds content, getContent, and the JSX that references sortedUpdates[updateIdx]) to early-handle the not-found case—e.g., if updateIdx === -1 set content to a safe fallback (such as MoreVersions with all updates, undefined, or an explicit error UI) and skip the slices/JSX that assume a valid index; ensure the guards apply before computing beforeSelectedUpd/afterSelectedUpd and before rendering ChannelLine/ChannelVersion for sortedUpdates[updateIdx].
81-86:⚠️ Potential issue | 🟠 MajorVersion-dot button is still missing an accessible name.
This remains unresolved: assistive tech gets an unnamed control for the icon-only/empty button. Also, the current-version dot should be disabled rather than focusable with a no-op click.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/ui-components/src/components/Catalog/EditWizard/steps/UpdateGraph.tsx` around lines 81 - 86, The Version-dot Button currently renders an icon-only control without an accessible name and leaves the "current" dot focusable with a no-op onClick; update the Button (the instance using props variant, className, icon, onClick, isSelected, current, setValue, version) to provide an accessible name (e.g. aria-label or aria-labelledby like `Select version ${version}`) when rendering icon-only, and make the current-version button truly disabled (use the Button's disabled prop instead of setting onClick to undefined) so it is not focusable or clickable; ensure the enabled button still calls setValue(version) on click.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@libs/i18n/locales/en/translation.json`:
- Around line 242-244: The changes added translation keys ("Other available
paths", "+ More", "{{currentChannel}} channel") directly into
libs/i18n/locales/en/translation.json; instead, undo manual edits and add or fix
these keys in the source code where they originate (e.g., the component or
JSX/TSX files that render "+ More" and "{{currentChannel}} channel" or the
module that produces "Other available paths"), then regenerate the locale
artifacts by running npm run i18n so the keys are properly extracted and merged
into translation.json; ensure any source-level i18n key names match the intended
strings before regenerating.
In `@libs/ui-components/src/components/Catalog/EditWizard/steps/UpdateGraph.css`:
- Line 97: Replace the deprecated word-break value "break-word" in
UpdateGraph.css: set the word-break property to normal and add an overflow-wrap
rule with the value anywhere so the browser uses the modern equivalent; update
the CSS rule that currently uses word-break: break-word to use word-break:
normal and overflow-wrap: anywhere.
In `@libs/ui-components/src/components/Catalog/EditWizard/steps/UpdateStep.tsx`:
- Around line 85-93: The onChange handler for channel/version changes (the
inline onChange that calls getUpdates, getInitialAppConfig, and
applyInitialConfig) is rebuilding and immediately applying a fresh app config
which wipes any edit-time customizations; change it to preserve the existing app
context and merge rather than replace: read the current form's app config values
(via setFieldValue/getFieldValue or form state), call
getInitialAppConfig(catalogItem, newVersion) to get defaults, shallow/deep-merge
those defaults with the existing app config so user-set keys are preserved, and
then call applyInitialConfig only with the merged config (or only update fields
that are missing) instead of overwriting everything; ensure version is still set
with setFieldValue('version', newVersion) and that
getUpdates/currentVersion/catalogItem names are used to locate the handler.
---
Duplicate comments:
In `@libs/ui-components/src/components/Catalog/EditWizard/steps/UpdateGraph.tsx`:
- Around line 112-141: The code uses updateIdx from sortedUpdates.findIndex
without guarding the -1 case, so when updateIdx === -1 the code dereferences
sortedUpdates[updateIdx] (e.g., sortedUpdates[-1]) and can crash; update the
logic around updateIdx (the block that builds content, getContent, and the JSX
that references sortedUpdates[updateIdx]) to early-handle the not-found
case—e.g., if updateIdx === -1 set content to a safe fallback (such as
MoreVersions with all updates, undefined, or an explicit error UI) and skip the
slices/JSX that assume a valid index; ensure the guards apply before computing
beforeSelectedUpd/afterSelectedUpd and before rendering
ChannelLine/ChannelVersion for sortedUpdates[updateIdx].
- Around line 81-86: The Version-dot Button currently renders an icon-only
control without an accessible name and leaves the "current" dot focusable with a
no-op onClick; update the Button (the instance using props variant, className,
icon, onClick, isSelected, current, setValue, version) to provide an accessible
name (e.g. aria-label or aria-labelledby like `Select version ${version}`) when
rendering icon-only, and make the current-version button truly disabled (use the
Button's disabled prop instead of setting onClick to undefined) so it is not
focusable or clickable; ensure the enabled button still calls setValue(version)
on click.
In `@libs/ui-components/src/components/Catalog/InstalledSoftware.tsx`:
- Line 221: The code is coercing resource IDs to an empty string when calling
onEdit, which can hide invalid identifiers; for each call site that uses
onEdit(osItem.metadata.catalog, osItem.metadata.name || '' ) (and the other
occurrences in InstalledSoftware.tsx), remove the "|| ''" fallback and pass
osItem.metadata.name directly to onEdit so the real (possibly undefined) value
surfaces; ensure the onEdit signature/type accepts a string (or narrow the
resource type) so that metadata.name is treated as defined for API-returned
resources.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cdb6826c-41f4-4e12-9b63-251b1dc63a31
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
apps/ocp-plugin/package.jsonapps/standalone/package.jsonlibs/i18n/locales/en/translation.jsonlibs/ui-components/package.jsonlibs/ui-components/src/components/Catalog/EditWizard/EditAppWizard.tsxlibs/ui-components/src/components/Catalog/EditWizard/steps/UpdateGraph.csslibs/ui-components/src/components/Catalog/EditWizard/steps/UpdateGraph.tsxlibs/ui-components/src/components/Catalog/EditWizard/steps/UpdateStep.tsxlibs/ui-components/src/components/Catalog/InstallWizard/steps/SpecificationsStep.tsxlibs/ui-components/src/components/Catalog/InstalledSoftware.tsx
💤 Files with no reviewable changes (3)
- libs/ui-components/package.json
- apps/standalone/package.json
- apps/ocp-plugin/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/ui-components/src/components/Catalog/InstallWizard/steps/SpecificationsStep.tsx
libs/ui-components/src/components/Catalog/EditWizard/steps/UpdateGraph.css
Outdated
Show resolved
Hide resolved
libs/ui-components/src/components/Catalog/EditWizard/steps/UpdateStep.tsx
Show resolved
Hide resolved
rawagner
force-pushed
the
update_chart
branch
2 times, most recently
from
60dda0b to
f68359f
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
libs/ui-components/src/components/Catalog/InstalledSoftware.tsx (1)
221-221:⚠️ Potential issue | 🟡 MinorRemove
|| ''fallbacks when passingmetadata.nameintoonEdit.Line 221, Line 244, Line 272, and Line 301 still coerce
metadata.nameto''. This can pass an invalid ID into edit flows.Proposed fix
- onClick={() => onEdit(osItem.metadata.catalog, osItem.metadata.name || '')} + onClick={() => onEdit(osItem.metadata.catalog, osItem.metadata.name)} - onClick: () => onEdit(osItem.metadata.catalog, osItem.metadata.name || ''), + onClick: () => onEdit(osItem.metadata.catalog, osItem.metadata.name), - onClick: () => onEdit(app.item.metadata.catalog, app.item.metadata.name || '', app.name), + onClick: () => onEdit(app.item.metadata.catalog, app.item.metadata.name, app.name), - onClick={() => onEdit(app.item.metadata.catalog, app.item.metadata.name || '', app.name)} + onClick={() => onEdit(app.item.metadata.catalog, app.item.metadata.name, app.name)}As per coding guidelines: "Treat
resource.metadata.nameas always defined for resources returned from the API or created resources, and avoid optional chaining in these contexts; only handle undefined for partial or input payloads".Also applies to: 244-244, 272-272, 301-301
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/ui-components/src/components/Catalog/InstalledSoftware.tsx` at line 221, Remove the unnecessary "|| ''" fallbacks when passing resource.metadata.name to the edit handlers: locate the onClick usages in InstalledSoftware.tsx that call onEdit (references: onEdit(osItem.metadata.catalog, osItem.metadata.name || '')) and similar calls at the other occurrences (lines where onEdit is invoked with osItem.metadata.name). Replace those calls to pass osItem.metadata.name directly (e.g., onEdit(osItem.metadata.catalog, osItem.metadata.name)) so you don't coerce a possibly-valid resource ID to an empty string; leave handling of undefined only where dealing with partial/input payloads per guidelines.libs/ui-components/src/components/Catalog/EditWizard/steps/UpdateStep.tsx (1)
85-93:⚠️ Potential issue | 🟠 MajorPreserve existing app context when reinitializing config on channel change.
Line 91 recomputes config with only
catalogItemandnewVersion. In edit flow, that drops the existing-app context used during initial load, so channel changes can clear merged config and label-derivedselectedAssets.🔧 Proposed fix
+import { ApplicationProviderSpec } from '@flightctl/types'; ... type UpdateStepProps = { currentVersion: CatalogItemVersion; catalogItem: CatalogItem; isEdit: boolean; + appSpec?: ApplicationProviderSpec; + currentLabels?: Record<string, string>; }; -const UpdateStep = ({ currentVersion, catalogItem, isEdit }: UpdateStepProps) => { +const UpdateStep = ({ currentVersion, catalogItem, isEdit, appSpec, currentLabels }: UpdateStepProps) => { ... - const appConfig = getInitialAppConfig(catalogItem, newVersion); + const appConfig = getInitialAppConfig(catalogItem, newVersion, appSpec, currentLabels); applyInitialConfig(setFieldValue, appConfig);// libs/ui-components/src/components/Catalog/EditWizard/EditAppWizard.tsx - <UpdateStep catalogItem={catalogItem} currentVersion={currentVersion} isEdit={!!appSpec} /> + <UpdateStep + catalogItem={catalogItem} + currentVersion={currentVersion} + isEdit={!!appSpec} + appSpec={appSpec} + currentLabels={currentLabels} + />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/ui-components/src/components/Catalog/EditWizard/steps/UpdateStep.tsx` around lines 85 - 93, Update the onChange handler so reinitializing the app config preserves the existing app context instead of discarding it; fetch the current app context from the form/state (e.g. values.appContext or the existing merged config used in the edit flow) and pass that context into getInitialAppConfig and/or applyInitialConfig when computing/applying the new config in the onChange block that currently computes latestVersion and calls getInitialAppConfig(catalogItem, newVersion). Modify the call sites for getInitialAppConfig/applyInitialConfig in UpdateStep.tsx so they accept and use the existing app context (keeping setFieldValue usage intact) to avoid clearing merged config and selectedAssets on channel changes.
🧹 Nitpick comments (1)
libs/ui-components/src/components/Catalog/EditWizard/steps/UpdateGraph.tsx (1)
103-103: Cloneupdatesbefore sorting to avoid mutating props.Line 103 sorts
updatesin place. Sinceupdatesis a prop, this can leak side effects to other consumers and make render behavior harder to reason about.♻️ Proposed fix
- const sortedUpdates = updates.sort((a, b) => semver.compare(a.version, b.version)); + const sortedUpdates = [...updates].sort((a, b) => semver.compare(a.version, b.version));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/ui-components/src/components/Catalog/EditWizard/steps/UpdateGraph.tsx` at line 103, The code in UpdateGraph.tsx currently calls updates.sort(...) which mutates the updates prop; fix this by cloning the array before sorting (e.g., create a shallow copy of updates and call sort on that) so the prop is not mutated—update the declaration of sortedUpdates (the variable used where updates are currently sorted) to operate on a copied array (e.g., spread or Array.from) and then call semver.compare in the sort callback as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@libs/ui-components/src/components/Catalog/EditWizard/steps/UpdateStep.tsx`:
- Around line 85-93: Update the onChange handler so reinitializing the app
config preserves the existing app context instead of discarding it; fetch the
current app context from the form/state (e.g. values.appContext or the existing
merged config used in the edit flow) and pass that context into
getInitialAppConfig and/or applyInitialConfig when computing/applying the new
config in the onChange block that currently computes latestVersion and calls
getInitialAppConfig(catalogItem, newVersion). Modify the call sites for
getInitialAppConfig/applyInitialConfig in UpdateStep.tsx so they accept and use
the existing app context (keeping setFieldValue usage intact) to avoid clearing
merged config and selectedAssets on channel changes.
In `@libs/ui-components/src/components/Catalog/InstalledSoftware.tsx`:
- Line 221: Remove the unnecessary "|| ''" fallbacks when passing
resource.metadata.name to the edit handlers: locate the onClick usages in
InstalledSoftware.tsx that call onEdit (references:
onEdit(osItem.metadata.catalog, osItem.metadata.name || '')) and similar calls
at the other occurrences (lines where onEdit is invoked with
osItem.metadata.name). Replace those calls to pass osItem.metadata.name directly
(e.g., onEdit(osItem.metadata.catalog, osItem.metadata.name)) so you don't
coerce a possibly-valid resource ID to an empty string; leave handling of
undefined only where dealing with partial/input payloads per guidelines.
---
Nitpick comments:
In `@libs/ui-components/src/components/Catalog/EditWizard/steps/UpdateGraph.tsx`:
- Line 103: The code in UpdateGraph.tsx currently calls updates.sort(...) which
mutates the updates prop; fix this by cloning the array before sorting (e.g.,
create a shallow copy of updates and call sort on that) so the prop is not
mutated—update the declaration of sortedUpdates (the variable used where updates
are currently sorted) to operate on a copied array (e.g., spread or Array.from)
and then call semver.compare in the sort callback as before.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: df9e45e9-08f5-40d4-af99-aab94d265681
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
apps/ocp-plugin/package.jsonapps/standalone/package.jsonlibs/i18n/locales/en/translation.jsonlibs/ui-components/package.jsonlibs/ui-components/src/components/Catalog/EditWizard/EditAppWizard.tsxlibs/ui-components/src/components/Catalog/EditWizard/steps/UpdateGraph.csslibs/ui-components/src/components/Catalog/EditWizard/steps/UpdateGraph.tsxlibs/ui-components/src/components/Catalog/EditWizard/steps/UpdateStep.tsxlibs/ui-components/src/components/Catalog/InstallWizard/steps/SpecificationsStep.tsxlibs/ui-components/src/components/Catalog/InstalledSoftware.tsx
💤 Files with no reviewable changes (3)
- libs/ui-components/package.json
- apps/ocp-plugin/package.json
- apps/standalone/package.json
🚧 Files skipped from review as they are similar to previous changes (3)
- libs/ui-components/src/components/Catalog/InstallWizard/steps/SpecificationsStep.tsx
- libs/i18n/locales/en/translation.json
- libs/ui-components/src/components/Catalog/EditWizard/steps/UpdateGraph.css
celdrake
approved these changes
Mar 4, 2026
| className = `${className} fctl-channel-version-dot--current`; | ||
| } else if (isSelected) { | ||
| className = `${className} fctl-channel-version-dot--selected`; | ||
| icon = <ArrowCircleUpIcon />; |
Collaborator
There was a problem hiding this comment.
The SVG is looking off for me. It seems due to the vertical-align from the .pf-v6-svg (both in Chrome and Firefox)
Collaborator
Author
There was a problem hiding this comment.
should be fixed now
| ); | ||
| }} | ||
| > | ||
| <Button variant="secondary" className="fctl-channel-more-versions" onClick={() => {}}> |
Collaborator
There was a problem hiding this comment.
The handler can be removed.
Suggested change
| <Button variant="secondary" className="fctl-channel-more-versions" onClick={() => {}}> | |
| <Button variant="secondary" className="fctl-channel-more-versions"> |
| @@ -265,7 +265,7 @@ const NewDeviceTarget = ({ catalogItem }: NewDeviceTargetProps) => { | |||
| <FlightCtlForm> | |||
Collaborator
There was a problem hiding this comment.
This component is a bit weird when there are no possible artifacts.
I think rather than showing the dropdown with "No items", it should show an error or message indicating that.
And in this case, IMO clicking on the "Close" button shouldn't show the warning about the form being dirty
Collaborator
Author
There was a problem hiding this comment.
Im now disabling the new device radio if there are no items to show in next step.
note that this is probably just temporary as we could provide steps on how to use the container img and convert it. So we would alway be able to have something in that dropdown. TBD
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
libs/ui-components/src/components/Catalog/InstalledSoftware.tsx (1)
221-221:⚠️ Potential issue | 🟡 MinorRemove empty-string fallbacks for
metadata.namein edit handlers.Passing
metadata.name || ''can silently route edit actions with an invalid ID. Usemetadata.namedirectly for API-backed resources.As per coding guidelines: "Treat `resource.metadata.name` as always defined for resources returned from the API or created resources, and avoid optional chaining in these contexts; only handle undefined for partial or input payloads".🔧 Proposed fix
- onClick={() => onEdit(osItem.metadata.catalog, osItem.metadata.name || '')} + onClick={() => onEdit(osItem.metadata.catalog, osItem.metadata.name)} ... - onClick: () => onEdit(osItem.metadata.catalog, osItem.metadata.name || ''), + onClick: () => onEdit(osItem.metadata.catalog, osItem.metadata.name), ... - onClick: () => onEdit(app.item.metadata.catalog, app.item.metadata.name || '', app.name), + onClick: () => onEdit(app.item.metadata.catalog, app.item.metadata.name, app.name), ... - onClick={() => onEdit(app.item.metadata.catalog, app.item.metadata.name || '', app.name)} + onClick={() => onEdit(app.item.metadata.catalog, app.item.metadata.name, app.name)}Also applies to: 244-244, 272-272, 301-301
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/ui-components/src/components/Catalog/InstalledSoftware.tsx` at line 221, The click handlers are using a fallback empty string (e.g., onEdit(osItem.metadata.catalog, osItem.metadata.name || '')) which can mask missing IDs; update each handler in InstalledSoftware to pass the resource name directly (use osItem.metadata.name) and remove the "|| ''" fallback so the onEdit call receives the actual metadata.name value; adjust all occurrences mentioned (the onEdit calls at the lines referenced and any similar handlers) to call onEdit(osItem.metadata.catalog, osItem.metadata.name) and ensure the onEdit signature/consumers expect a defined name.libs/ui-components/src/components/Catalog/EditWizard/steps/UpdateStep.tsx (1)
85-93:⚠️ Potential issue | 🟠 MajorPreserve existing form config when recomputing defaults on channel change.
This handler still reapplies initial config directly after version recalculation, which can wipe user edits in the current session and edit-time customized values.
libs/i18n/locales/en/translation.json (1)
321-321:⚠️ Potential issue | 🟡 MinorFix wording: use “besides” instead of “beside”.
This string reads awkwardly in UI. Please fix the source string to “The Operating system does not contain any additional formats besides bootc” and regenerate locale artifacts.
As per coding guidelines: "Do not manually edit translation.json in libs/i18n/locales/en/; regenerate with
npm run i18nby adding or fixing keys in source code".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/i18n/locales/en/translation.json` at line 321, The translation key value "The Operating system does not contain any additional formats beside bootc" uses the wrong word "beside" and must be corrected at the source string (not by editing libs/i18n/locales/en/translation.json directly); find the source location that emits the exact string "The Operating system does not contain any additional formats beside bootc", change it to "The Operating system does not contain any additional formats besides bootc", then regenerate locale artifacts by running npm run i18n so the updated string is propagated into translation.json.
🧹 Nitpick comments (1)
libs/ui-components/src/components/Catalog/EditWizard/steps/UpdateGraph.tsx (1)
103-103: Avoid mutating theupdatesprop while sorting.
Array.sortmutates in place, so this mutates a prop array during render. Sort a copy instead.♻️ Proposed fix
- const sortedUpdates = updates.sort((a, b) => semver.compare(a.version, b.version)); + const sortedUpdates = [...updates].sort((a, b) => semver.compare(a.version, b.version));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/ui-components/src/components/Catalog/EditWizard/steps/UpdateGraph.tsx` at line 103, The line computing sortedUpdates mutates the updates prop because Array.sort is in-place; change it to sort a shallow copy instead (e.g., use [...updates] or updates.slice() before calling .sort) so the prop is not mutated; update the expression that defines sortedUpdates (the variable named sortedUpdates that uses semver.compare) to operate on the copied array.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@libs/ui-components/src/components/Catalog/EditWizard/steps/UpdateGraph.tsx`:
- Line 155: The hardcoded "(current)" suffix should be localized: in the
UpdateGraph component replace the template literal ChannelVersion element that
uses `${currentVersion.version} (current)` with a call to the i18n translation
function (e.g., t) that uses a single translation string with a placeholder for
the version (e.g., t('{{version}} (current)', { version: currentVersion.version
})) so the "(current)" part is extractable; update the ChannelVersion rendering
to use that t(...) call and ensure the translation key contains the "(current)"
text, not a variable-only key.
In `@libs/ui-components/src/components/Catalog/EditWizard/steps/UpdateStep.tsx`:
- Around line 159-165: The Modal showing the readme in UpdateStep lacks an
accessible name; update the Modal usage in UpdateStep.tsx to include a
ModalHeader as the first child (above ModalBody) containing a descriptive title
(e.g., "Readme" or the version name) and ensure the ModalHeader import is added
to the existing imports from `@patternfly/react-core`; keep the current
ModalBody/Content/ReactMarkdown usage that renders updateVersion.readme and
preserve the onClose handler setShowReadme(false).
In
`@libs/ui-components/src/components/Catalog/InstallWizard/steps/SpecificationsStep.tsx`:
- Line 306: Update the user-facing tooltip copy in SpecificationsStep.tsx where
the content prop calls t('The Operating system does not contain any additional
formats beside bootc') — fix the grammar and wording to a clearer sentence such
as "The operating system contains no additional formats besides boot" (or "No
additional formats found for this operating system besides boot") and replace
the existing translation key/string passed to t(...) accordingly so the tooltip
reads naturally.
---
Duplicate comments:
In `@libs/i18n/locales/en/translation.json`:
- Line 321: The translation key value "The Operating system does not contain any
additional formats beside bootc" uses the wrong word "beside" and must be
corrected at the source string (not by editing
libs/i18n/locales/en/translation.json directly); find the source location that
emits the exact string "The Operating system does not contain any additional
formats beside bootc", change it to "The Operating system does not contain any
additional formats besides bootc", then regenerate locale artifacts by running
npm run i18n so the updated string is propagated into translation.json.
In `@libs/ui-components/src/components/Catalog/InstalledSoftware.tsx`:
- Line 221: The click handlers are using a fallback empty string (e.g.,
onEdit(osItem.metadata.catalog, osItem.metadata.name || '')) which can mask
missing IDs; update each handler in InstalledSoftware to pass the resource name
directly (use osItem.metadata.name) and remove the "|| ''" fallback so the
onEdit call receives the actual metadata.name value; adjust all occurrences
mentioned (the onEdit calls at the lines referenced and any similar handlers) to
call onEdit(osItem.metadata.catalog, osItem.metadata.name) and ensure the onEdit
signature/consumers expect a defined name.
---
Nitpick comments:
In `@libs/ui-components/src/components/Catalog/EditWizard/steps/UpdateGraph.tsx`:
- Line 103: The line computing sortedUpdates mutates the updates prop because
Array.sort is in-place; change it to sort a shallow copy instead (e.g., use
[...updates] or updates.slice() before calling .sort) so the prop is not
mutated; update the expression that defines sortedUpdates (the variable named
sortedUpdates that uses semver.compare) to operate on the copied array.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e4c2b731-51bc-43d4-9c9b-421ba5b11929
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (17)
apps/ocp-plugin/package.jsonapps/standalone/package.jsonlibs/i18n/locales/en/translation.jsonlibs/ui-components/package.jsonlibs/ui-components/src/components/Catalog/EditWizard/EditAppWizard.tsxlibs/ui-components/src/components/Catalog/EditWizard/EditOsWizard.tsxlibs/ui-components/src/components/Catalog/EditWizard/steps/ReviewStep.tsxlibs/ui-components/src/components/Catalog/EditWizard/steps/UpdateGraph.csslibs/ui-components/src/components/Catalog/EditWizard/steps/UpdateGraph.tsxlibs/ui-components/src/components/Catalog/EditWizard/steps/UpdateStep.tsxlibs/ui-components/src/components/Catalog/InstallWizard/InstallAppWizard.tsxlibs/ui-components/src/components/Catalog/InstallWizard/InstallOsWizard.tsxlibs/ui-components/src/components/Catalog/InstallWizard/InstallWizard.tsxlibs/ui-components/src/components/Catalog/InstallWizard/steps/ReviewStep.tsxlibs/ui-components/src/components/Catalog/InstallWizard/steps/SelectTargetStep.tsxlibs/ui-components/src/components/Catalog/InstallWizard/steps/SpecificationsStep.tsxlibs/ui-components/src/components/Catalog/InstalledSoftware.tsx
💤 Files with no reviewable changes (3)
- apps/ocp-plugin/package.json
- libs/ui-components/package.json
- apps/standalone/package.json
✅ Files skipped from review due to trivial changes (1)
- libs/ui-components/src/components/Catalog/InstallWizard/InstallAppWizard.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/ui-components/src/components/Catalog/EditWizard/EditAppWizard.tsx
libs/ui-components/src/components/Catalog/EditWizard/steps/UpdateGraph.tsx
Outdated
Show resolved
Hide resolved
libs/ui-components/src/components/Catalog/EditWizard/steps/UpdateStep.tsx
Show resolved
Hide resolved
| /> | ||
| <WithTooltip | ||
| showTooltip={!catalogItem.spec.reference.artifacts?.length} | ||
| content={t('The Operating system does not contain any additional formats beside bootc')} |
There was a problem hiding this comment.
Fix tooltip copy grammar for user-facing text.
The current string reads awkwardly (“Operating system”, “beside bootc”). Consider tightening the phrasing.
✏️ Proposed copy tweak
- content={t('The Operating system does not contain any additional formats beside bootc')}
+ content={t('This operating system does not contain any additional formats besides bootc')}📝 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.
Suggested change
| content={t('The Operating system does not contain any additional formats beside bootc')} | |
| content={t('This operating system does not contain any additional formats besides bootc')} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@libs/ui-components/src/components/Catalog/InstallWizard/steps/SpecificationsStep.tsx`
at line 306, Update the user-facing tooltip copy in SpecificationsStep.tsx where
the content prop calls t('The Operating system does not contain any additional
formats beside bootc') — fix the grammar and wording to a clearer sentence such
as "The operating system contains no additional formats besides boot" (or "No
additional formats found for this operating system besides boot") and replace
the existing translation key/string passed to t(...) accordingly so the tooltip
reads naturally.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
libs/ui-components/src/components/Catalog/InstallWizard/steps/SpecificationsStep.tsx (1)
306-309:⚠️ Potential issue | 🟡 MinorFix tooltip copy grammar/casing.
The user-facing text at Line 308 still reads awkwardly (“Operating system” capitalization and phrasing). Please tighten it for clarity.
✏️ Proposed copy tweak
- content={t('This Operating system does not contain any additional formats besides bootc')} + content={t('This operating system does not contain any additional formats besides bootc')}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/ui-components/src/components/Catalog/InstallWizard/steps/SpecificationsStep.tsx` around lines 306 - 309, The tooltip copy used in the WithTooltip component is awkwardly capitalized and phrased; update the content prop on WithTooltip in SpecificationsStep (the instance using showTooltip={!catalogItem.spec.reference.artifacts?.length} and triggerRef={newDeviceRadioRef}) to a tighter sentence such as "This operating system does not contain any additional formats besides boot." and ensure "operating system" is lower-case and the message is grammatically correct.libs/ui-components/src/components/Catalog/EditWizard/steps/UpdateStep.tsx (1)
86-94:⚠️ Potential issue | 🟠 MajorPreserve edit-time app state when channel changes.
At Line 92-93, config is rebuilt from defaults and immediately applied, which can drop existing custom config/asset selections in edit flow.
🔧 Proposed fix pattern
+import { ApplicationProviderSpec } from '@flightctl/types'; ... type UpdateStepProps = { currentVersion: CatalogItemVersion; catalogItem: CatalogItem; isEdit: boolean; + appSpec?: ApplicationProviderSpec; + currentLabels?: Record<string, string>; }; -const UpdateStep = ({ currentVersion, catalogItem, isEdit }: UpdateStepProps) => { +const UpdateStep = ({ currentVersion, catalogItem, isEdit, appSpec, currentLabels }: UpdateStepProps) => { ... - const appConfig = getInitialAppConfig(catalogItem, newVersion); + const appConfig = getInitialAppConfig(catalogItem, newVersion, appSpec, currentLabels); applyInitialConfig(setFieldValue, appConfig);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/ui-components/src/components/Catalog/EditWizard/steps/UpdateStep.tsx` around lines 86 - 94, The onChange handler for channel/version selection currently discards user edits by calling getInitialAppConfig(catalogItem, newVersion) and applyInitialConfig(setFieldValue, appConfig) which overwrites existing form state; instead preserve edit-time state by merging existing form values/app config with the new defaults: fetch current form/app config (e.g., via the form's getFieldValue or the component state that holds the current config), compute defaults = getInitialAppConfig(catalogItem, newVersion), then shallow/deep-merge defaults into the existing config so only missing fields are filled and user selections (assets, custom fields) are retained, and finally call setFieldValue('version', newVersion) and applyInitialConfig or an updated helper that accepts the merged config to apply only the missing/default values (or modify applyInitialConfig to accept a merge flag).
🧹 Nitpick comments (1)
libs/ui-components/src/components/Catalog/InstallWizard/steps/SpecificationsStep.tsx (1)
39-46: Use the sharedInstallSpecFormiktype forVersionDropdown.
VersionDropdownis now reused outside this file, but it still binds Formik context to a localInstallSpecFormikdefinition. This creates avoidable drift risk versus the shared wizard type.♻️ Proposed refactor
-import { InstallAppFormik } from '../types'; +import { InstallAppFormik, InstallSpecFormik } from '../types'; ... -export type InstallSpecFormik = { - version: string; - channel: string; -};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/ui-components/src/components/Catalog/InstallWizard/steps/SpecificationsStep.tsx` around lines 39 - 46, VersionDropdown is still tied to a local InstallSpecFormik type; update it to use the shared wizard type instead: import the shared InstallSpecFormik type and replace the local type usage in the useFormikContext generic (useFormikContext<InstallSpecFormik>) and any other local references, and remove the local duplicate InstallSpecFormik definition if present so the component consistently uses the shared InstallSpecFormik type across the codebase.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@libs/ui-components/src/components/Catalog/EditWizard/steps/UpdateStep.tsx`:
- Around line 148-155: The JSX currently always renders an outer GridItem and
then conditionally renders another GridItem inside it (see UpdateStep.tsx where
updateVersion?.deprecation and GridItem/Alert are used), which causes nested
GridItems and breaks layout; fix by removing the nested GridItem: either move
the conditional onto the outer GridItem so the outer GridItem is only rendered
when updateVersion?.deprecation is truthy, or render the Alert directly inside
the existing outer GridItem (keep the conditional around the Alert content) and
remove the inner GridItem so only a single GridItem wraps the Alert.
---
Duplicate comments:
In `@libs/ui-components/src/components/Catalog/EditWizard/steps/UpdateStep.tsx`:
- Around line 86-94: The onChange handler for channel/version selection
currently discards user edits by calling getInitialAppConfig(catalogItem,
newVersion) and applyInitialConfig(setFieldValue, appConfig) which overwrites
existing form state; instead preserve edit-time state by merging existing form
values/app config with the new defaults: fetch current form/app config (e.g.,
via the form's getFieldValue or the component state that holds the current
config), compute defaults = getInitialAppConfig(catalogItem, newVersion), then
shallow/deep-merge defaults into the existing config so only missing fields are
filled and user selections (assets, custom fields) are retained, and finally
call setFieldValue('version', newVersion) and applyInitialConfig or an updated
helper that accepts the merged config to apply only the missing/default values
(or modify applyInitialConfig to accept a merge flag).
In
`@libs/ui-components/src/components/Catalog/InstallWizard/steps/SpecificationsStep.tsx`:
- Around line 306-309: The tooltip copy used in the WithTooltip component is
awkwardly capitalized and phrased; update the content prop on WithTooltip in
SpecificationsStep (the instance using
showTooltip={!catalogItem.spec.reference.artifacts?.length} and
triggerRef={newDeviceRadioRef}) to a tighter sentence such as "This operating
system does not contain any additional formats besides boot." and ensure
"operating system" is lower-case and the message is grammatically correct.
---
Nitpick comments:
In
`@libs/ui-components/src/components/Catalog/InstallWizard/steps/SpecificationsStep.tsx`:
- Around line 39-46: VersionDropdown is still tied to a local InstallSpecFormik
type; update it to use the shared wizard type instead: import the shared
InstallSpecFormik type and replace the local type usage in the useFormikContext
generic (useFormikContext<InstallSpecFormik>) and any other local references,
and remove the local duplicate InstallSpecFormik definition if present so the
component consistently uses the shared InstallSpecFormik type across the
codebase.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2d8f989f-ac3d-4e44-954e-b2bc64530d2d
📒 Files selected for processing (5)
libs/i18n/locales/en/translation.jsonlibs/ui-components/src/components/Catalog/EditWizard/steps/UpdateGraph.csslibs/ui-components/src/components/Catalog/EditWizard/steps/UpdateGraph.tsxlibs/ui-components/src/components/Catalog/EditWizard/steps/UpdateStep.tsxlibs/ui-components/src/components/Catalog/InstallWizard/steps/SpecificationsStep.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- libs/ui-components/src/components/Catalog/EditWizard/steps/UpdateGraph.css
- libs/i18n/locales/en/translation.json
Comment on lines
148
to
+155
| <GridItem> | ||
| <FormGroup label={t('{{ channel }} channel', { channel: values.channel })}> | ||
| <UpdateGraph | ||
| selectedVersion={values.version} | ||
| currentVersion={currentVersion} | ||
| currentChannel={values.channel} | ||
| onSelectionChange={(version) => { | ||
| onVersionChange?.(version); | ||
| setFieldValue('version', version); | ||
| }} | ||
| updates={updates} | ||
| /> | ||
| </FormGroup> | ||
| {updateVersion?.deprecation && ( | ||
| <GridItem span={6}> | ||
| <Alert isInline variant="warning" title={t('This version is deprecated')}> | ||
| {updateVersion.deprecation.message} | ||
| </Alert> | ||
| </GridItem> | ||
| )} |
There was a problem hiding this comment.
Avoid nesting GridItem inside another GridItem.
This block always renders an outer GridItem and conditionally renders another GridItem inside it, which can produce unintended grid spacing/layout behavior.
🧹 Proposed cleanup
- <GridItem>
- {updateVersion?.deprecation && (
- <GridItem span={6}>
- <Alert isInline variant="warning" title={t('This version is deprecated')}>
- {updateVersion.deprecation.message}
- </Alert>
- </GridItem>
- )}
- </GridItem>
+ {updateVersion?.deprecation && (
+ <GridItem span={6}>
+ <Alert isInline variant="warning" title={t('This version is deprecated')}>
+ {updateVersion.deprecation.message}
+ </Alert>
+ </GridItem>
+ )}📝 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.
Suggested change
| <GridItem> | |
| <FormGroup label={t('{{ channel }} channel', { channel: values.channel })}> | |
| <UpdateGraph | |
| selectedVersion={values.version} | |
| currentVersion={currentVersion} | |
| currentChannel={values.channel} | |
| onSelectionChange={(version) => { | |
| onVersionChange?.(version); | |
| setFieldValue('version', version); | |
| }} | |
| updates={updates} | |
| /> | |
| </FormGroup> | |
| {updateVersion?.deprecation && ( | |
| <GridItem span={6}> | |
| <Alert isInline variant="warning" title={t('This version is deprecated')}> | |
| {updateVersion.deprecation.message} | |
| </Alert> | |
| </GridItem> | |
| )} | |
| {updateVersion?.deprecation && ( | |
| <GridItem span={6}> | |
| <Alert isInline variant="warning" title={t('This version is deprecated')}> | |
| {updateVersion.deprecation.message} | |
| </Alert> | |
| </GridItem> | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@libs/ui-components/src/components/Catalog/EditWizard/steps/UpdateStep.tsx`
around lines 148 - 155, The JSX currently always renders an outer GridItem and
then conditionally renders another GridItem inside it (see UpdateStep.tsx where
updateVersion?.deprecation and GridItem/Alert are used), which causes nested
GridItems and breaks layout; fix by removing the nested GridItem: either move
the conditional onto the outer GridItem so the outer GridItem is only rendered
when updateVersion?.deprecation is truthy, or render the Alert directly inside
the existing outer GridItem (keep the conditional around the Alert content) and
remove the inner GridItem so only a single GridItem wraps the Alert.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Aligns the chart with OCP Console
Summary by CodeRabbit
New Features
Improvements
Localization
Chores