CNV-73006: refactor duplicated DescriptionItem usage #3254
Conversation
|
@adamviktora: This pull request references CNV-73006 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 task to target the "4.21.0" version, but no target version was set. In 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. |
📝 WalkthroughWalkthroughChanges add customization props ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
|
@adamviktora: This pull request references CNV-73006 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 task to target the "4.21.0" version, but no target version was set. In 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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/utils/components/MetadataLabels/MetadataLabels.tsx (1)
16-24: Tighten typing aroundlabelsand simplify empty-check.
- In the mapper (lines 28–39)
labelsis typed as optional but accessed aslabels[key]. This is safe at runtime becauselabelsKeysisObject.keys(labels || {}), but TS won’t narrowlabelsfrom that. Consider either makinglabelsnon‑optional in props or using optional chaining:
const value = labels?.[key]; const labelText = value ? \${key}=${value}` : key;`- For the empty state (lines 20–24),
isEmpty(labelsKeys)works becauselabelsKeysis an array, butlabelsKeys.length === 0is more explicit and avoids relying on a generic helper for a simple array length check.These tweaks improve type-safety and readability without changing behavior.
Also applies to: 28-39
src/views/topology/components/vm/VMDetailsPanel/components/VMDetailsPanelLeftColumn/components/VMAnnotationsDetailsItem.tsx (1)
3-7: Annotations refactor looks correct; consider memoizing handlers if this becomes hot.
- Wiring
DescriptionItemAnnotationswithmodel={VirtualMachineModel},resource={vm},descriptionHeaderWrapperusingSearchItem, andonAnnotationsSubmit={(annotations) => updateAnnotation(vm, annotations)}matches the expectedupdateAnnotationsignature and removes the modal complexity cleanly.- If this panel re-renders frequently and
DescriptionItemAnnotationsis memoized, you might later consider wrapping the submit/header callbacks withuseCallbackto keep props stable, but this is optional given current complexity. As per coding guidelines, memoization tools are encouraged where they materially reduce re-renders.Also applies to: 17-23
src/views/virtualmachines/details/tabs/configuration/metadata/MetadataTab.tsx (1)
3-5: Good use of shared DescriptionItem components; double‑checkSearchItemidsThe switch to
DescriptionItemLabels/DescriptionItemAnnotationswired withVirtualMachineModel,resource={vm}, and theupdateLabels/updateAnnotationcallbacks looks correct and should centralize metadata editing nicely.One thing to verify:
SearchItem id="metadata"is now used both for the main tab title and again in the annotations header wrapper, while labels useid="labels". If the search/navigation logic expects ids to be unique per logical section, you might want a more specific id for annotations (for examplemetadata-annotations) to avoid ambiguity.Also applies to: 23-36
src/utils/components/DescriptionItem/components/DescriptionItemAnnotations.tsx (1)
1-1: Behavioral extension looks good; consider tightening types for header wrapper and submitThe new props and fallback submit handler keep the old behavior while enabling external submit overrides and header customization. The
onSubmit={onAnnotationsSubmit ?? onAnnotationsSubmitInternal}wiring is clean.Two small type-level improvements:
descriptionHeaderWrappercurrently takes astring, but conceptually it’s a wrapper around “whatever we use as the header”. Making it acceptReactNodegives you more flexibility without breaking current callers.onAnnotationsSubmitis typed asPromise<any>, which goes against the “avoidany” guideline;Promise<unknown>(or a more concrete type likePromise<void>if callers don’t use the result) would be safer.You can implement both with a local-only change:
-type DescriptionItemAnnotationsProps = { - className?: string; - descriptionHeaderWrapper?: (children: string) => ReactNode; +type DescriptionItemAnnotationsProps = { + className?: string; + descriptionHeaderWrapper?: (children: ReactNode) => ReactNode; editable?: boolean; label?: string; model: K8sModel; - onAnnotationsSubmit?: (annotations: { [key: string]: string }) => Promise<any>; + onAnnotationsSubmit?: (annotations: { [key: string]: string }) => Promise<unknown>; resource: K8sResourceCommon; };
annotationsHeaderis still a string, which is valid as aReactNode, so no runtime change.Also applies to: 12-18, 22-29, 37-38, 60-62, 70-72
src/utils/components/DescriptionItem/components/DescriptionItemLabels.tsx (1)
1-1: Nice generalization of label handling; align types with annotations and guidelinesThe refactor to a shared
DescriptionItemLabelswith optionalclassName,descriptionHeaderWrapper, and externalonLabelsSubmitis well-structured. The internalonLabelsSubmitInternalfallback preserves the originalk8sPatchbehavior, and passingisLabelEditorintoDescriptionItemis a clear way to toggle label-specific styling.Similar to
DescriptionItemAnnotations, you can tighten types a bit:
- Allow
descriptionHeaderWrapperto wrap aReactNoderather than only astring.- Prefer
Promise<unknown>(or another concrete type) overPromise<anyforonLabelsSubmit.Suggested diff:
-type DescriptionItemLabelsProps = { - className?: string; - descriptionHeaderWrapper?: (children: string) => ReactNode; +type DescriptionItemLabelsProps = { + className?: string; + descriptionHeaderWrapper?: (children: ReactNode) => ReactNode; editable?: boolean; label?: string; model: K8sModel; - onLabelsSubmit?: (labels: { [key: string]: string }) => Promise<any>; + onLabelsSubmit?: (labels: { [key: string]: string }) => Promise<unknown>; resource: K8sResourceCommon; };This keeps current callers valid while improving type safety and flexibility.
Also applies to: 13-19, 23-30, 34-35, 53-54, 57-59, 67-68, 70-73
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
src/utils/components/DescriptionItem/DescriptionItem.tsx(5 hunks)src/utils/components/DescriptionItem/components/DescriptionItemAnnotations.tsx(4 hunks)src/utils/components/DescriptionItem/components/DescriptionItemLabels.tsx(3 hunks)src/utils/components/DescriptionItem/components/DescriptionItemNamespace.tsx(3 hunks)src/utils/components/MetadataLabels/MetadataLabels.scss(0 hunks)src/utils/components/MetadataLabels/MetadataLabels.tsx(1 hunks)src/views/catalog/CustomizeInstanceType/tabs/configuration/utils/tabs/CustomizeInstanceTypeMetadataTab.tsx(2 hunks)src/views/catalog/wizard/components/WizardDescriptionItem/WizardDescriptionItem.tsx(4 hunks)src/views/catalog/wizard/tabs/metadata/WizardMetadataTab.tsx(1 hunks)src/views/topology/components/vm/VMDetailsPanel/components/VMDetailsPanelLeftColumn/components/VMAnnotationsDetailsItem.tsx(2 hunks)src/views/topology/components/vm/VMDetailsPanel/components/VMDetailsPanelLeftColumn/components/VMLabelsDetailsItem.tsx(2 hunks)src/views/virtualmachines/details/tabs/configuration/metadata/MetadataTab.tsx(2 hunks)src/views/virtualmachines/details/tabs/configuration/metadata/components/MetadataTabAnnotations/MetadataTabAnnotations.tsx(0 hunks)src/views/virtualmachines/details/tabs/configuration/metadata/components/MetadataTabLabels/MetadataTabLabels.tsx(0 hunks)src/views/virtualmachines/details/tabs/configuration/metadata/components/MetadataTabLabels/metadata-tab-labels.scss(0 hunks)src/views/virtualmachines/details/tabs/overview/components/VirtualMachinesOverviewTabGeneral/VirtualMachinesOverviewTabGeneral.tsx(2 hunks)
💤 Files with no reviewable changes (4)
- src/utils/components/MetadataLabels/MetadataLabels.scss
- src/views/virtualmachines/details/tabs/configuration/metadata/components/MetadataTabLabels/metadata-tab-labels.scss
- src/views/virtualmachines/details/tabs/configuration/metadata/components/MetadataTabAnnotations/MetadataTabAnnotations.tsx
- src/views/virtualmachines/details/tabs/configuration/metadata/components/MetadataTabLabels/MetadataTabLabels.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*.tsx
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.tsx: Use.tsxfile extension for all React components. One component per file. No nested components.
Use PascalCase for all component names (e.g.,HeaderTop.tsx).
Use functional components by default. Use class components only for specific lifecycle methods unavailable in functional components (e.g.,componentDidCatch).
Use default exports for all components.
Use React's memoization tools (React.memo,useMemo,useCallback) to avoid unnecessary re-renders. Lazy load components withReact.lazyandSuspense.
Always specify dependencies inuseEffectto avoid unnecessary re-renders or missed updates. Pass an empty array[]to run the effect only once if no dependencies are required.
Files:
src/views/virtualmachines/details/tabs/overview/components/VirtualMachinesOverviewTabGeneral/VirtualMachinesOverviewTabGeneral.tsxsrc/views/topology/components/vm/VMDetailsPanel/components/VMDetailsPanelLeftColumn/components/VMLabelsDetailsItem.tsxsrc/views/catalog/CustomizeInstanceType/tabs/configuration/utils/tabs/CustomizeInstanceTypeMetadataTab.tsxsrc/views/catalog/wizard/tabs/metadata/WizardMetadataTab.tsxsrc/views/catalog/wizard/components/WizardDescriptionItem/WizardDescriptionItem.tsxsrc/views/topology/components/vm/VMDetailsPanel/components/VMDetailsPanelLeftColumn/components/VMAnnotationsDetailsItem.tsxsrc/utils/components/DescriptionItem/components/DescriptionItemAnnotations.tsxsrc/utils/components/MetadataLabels/MetadataLabels.tsxsrc/views/virtualmachines/details/tabs/configuration/metadata/MetadataTab.tsxsrc/utils/components/DescriptionItem/DescriptionItem.tsxsrc/utils/components/DescriptionItem/components/DescriptionItemLabels.tsxsrc/utils/components/DescriptionItem/components/DescriptionItemNamespace.tsx
**/*.{tsx,ts}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{tsx,ts}: Extract as much logic as possible from components into custom hooks or utility files to improve testability. Avoid bloated components.
Hooks should contain only logic and side effects, not return JSX. Keep JSX in components while using hooks for extracting reusable or unit-testable logic.
Files:
src/views/virtualmachines/details/tabs/overview/components/VirtualMachinesOverviewTabGeneral/VirtualMachinesOverviewTabGeneral.tsxsrc/views/topology/components/vm/VMDetailsPanel/components/VMDetailsPanelLeftColumn/components/VMLabelsDetailsItem.tsxsrc/views/catalog/CustomizeInstanceType/tabs/configuration/utils/tabs/CustomizeInstanceTypeMetadataTab.tsxsrc/views/catalog/wizard/tabs/metadata/WizardMetadataTab.tsxsrc/views/catalog/wizard/components/WizardDescriptionItem/WizardDescriptionItem.tsxsrc/views/topology/components/vm/VMDetailsPanel/components/VMDetailsPanelLeftColumn/components/VMAnnotationsDetailsItem.tsxsrc/utils/components/DescriptionItem/components/DescriptionItemAnnotations.tsxsrc/utils/components/MetadataLabels/MetadataLabels.tsxsrc/views/virtualmachines/details/tabs/configuration/metadata/MetadataTab.tsxsrc/utils/components/DescriptionItem/DescriptionItem.tsxsrc/utils/components/DescriptionItem/components/DescriptionItemLabels.tsxsrc/utils/components/DescriptionItem/components/DescriptionItemNamespace.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{ts,tsx}: Use descriptive names for variables, functions, and components. Avoid abbreviations unless widely recognized.
Prefer usingtypeinstead ofinterfacefor defining the shapes of objects or functions in TypeScript.
Avoid usinganytype in TypeScript as it compromises type safety. Useunknowninstead and narrow the type as needed.
Always explicitly define return types for functions in TypeScript rather than relying on type inference.
Files:
src/views/virtualmachines/details/tabs/overview/components/VirtualMachinesOverviewTabGeneral/VirtualMachinesOverviewTabGeneral.tsxsrc/views/topology/components/vm/VMDetailsPanel/components/VMDetailsPanelLeftColumn/components/VMLabelsDetailsItem.tsxsrc/views/catalog/CustomizeInstanceType/tabs/configuration/utils/tabs/CustomizeInstanceTypeMetadataTab.tsxsrc/views/catalog/wizard/tabs/metadata/WizardMetadataTab.tsxsrc/views/catalog/wizard/components/WizardDescriptionItem/WizardDescriptionItem.tsxsrc/views/topology/components/vm/VMDetailsPanel/components/VMDetailsPanelLeftColumn/components/VMAnnotationsDetailsItem.tsxsrc/utils/components/DescriptionItem/components/DescriptionItemAnnotations.tsxsrc/utils/components/MetadataLabels/MetadataLabels.tsxsrc/views/virtualmachines/details/tabs/configuration/metadata/MetadataTab.tsxsrc/utils/components/DescriptionItem/DescriptionItem.tsxsrc/utils/components/DescriptionItem/components/DescriptionItemLabels.tsxsrc/utils/components/DescriptionItem/components/DescriptionItemNamespace.tsx
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: kubevirt-ui/kubevirt-plugin PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-04T14:08:48.257Z
Learning: Add a main PR comment that explains referenced links, Jira, documentation, use cases, and current/required state. Include videos and/or screenshots for visual changes.
📚 Learning: 2025-12-04T14:08:48.257Z
Learnt from: CR
Repo: kubevirt-ui/kubevirt-plugin PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-04T14:08:48.257Z
Learning: Applies to **/*.tsx : Use React's memoization tools (`React.memo`, `useMemo`, `useCallback`) to avoid unnecessary re-renders. Lazy load components with `React.lazy` and `Suspense`.
Applied to files:
src/utils/components/DescriptionItem/DescriptionItem.tsx
🧬 Code graph analysis (6)
src/views/catalog/CustomizeInstanceType/tabs/configuration/utils/tabs/CustomizeInstanceTypeMetadataTab.tsx (1)
src/views/virtualmachines/details/tabs/configuration/details/utils/utils.ts (1)
updateMetadata(96-112)
src/views/topology/components/vm/VMDetailsPanel/components/VMDetailsPanelLeftColumn/components/VMAnnotationsDetailsItem.tsx (2)
src/views/virtualmachines/details/tabs/configuration/utils/search.ts (1)
SearchItem(6-11)src/views/virtualmachines/details/tabs/configuration/details/utils/utils.ts (1)
updateAnnotation(114-115)
src/utils/components/MetadataLabels/MetadataLabels.tsx (2)
src/utils/utils/utils.ts (1)
isEmpty(32-33)src/utils/components/Labels/LabelList.tsx (1)
Label(17-35)
src/views/virtualmachines/details/tabs/configuration/metadata/MetadataTab.tsx (2)
src/views/virtualmachines/details/tabs/configuration/utils/search.ts (1)
SearchItem(6-11)src/views/virtualmachines/details/tabs/configuration/details/utils/utils.ts (2)
updateLabels(117-118)updateAnnotation(114-115)
src/utils/components/DescriptionItem/components/DescriptionItemLabels.tsx (1)
cypress/views/selector-common.ts (1)
label(110-110)
src/utils/components/DescriptionItem/components/DescriptionItemNamespace.tsx (1)
src/multicluster/helpers/selectors.ts (1)
getCluster(6-7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: unit-test
- GitHub Check: i18n
- GitHub Check: build
🔇 Additional comments (7)
src/views/catalog/wizard/tabs/metadata/WizardMetadataTab.tsx (1)
32-62:isLabelEditorwiring for wizard labels looks consistent.Using
isLabelEditoron the labelsWizardDescriptionItemaligns with the new styling behavior inWizardDescriptionItemwithout affecting the existing modal-based edit flow.src/views/virtualmachines/details/tabs/overview/components/VirtualMachinesOverviewTabGeneral/VirtualMachinesOverviewTabGeneral.tsx (1)
9-12: Good reuse ofDescriptionItemNamespacefor VM namespace.Swapping the inline namespace
DescriptionItemforDescriptionItemNamespacewithmodel={VirtualMachineModel}andresource={vm}is a nice consolidation and ensures consistent, cluster-aware namespace linking across views.Also applies to: 52-53
src/views/catalog/wizard/components/WizardDescriptionItem/WizardDescriptionItem.tsx (1)
2-3:isLabelEditorintegration inWizardDescriptionItemis clean and backward compatible.The new
isLabelEditorprop defaults tofalseand only affects CSS (co-editable-label-group) on the description area, leaving existing consumers unchanged and enabling the wizard labels view to opt in explicitly.Also applies to: 37-39, 61-62, 142-148
src/views/catalog/CustomizeInstanceType/tabs/configuration/utils/tabs/CustomizeInstanceTypeMetadataTab.tsx (1)
3-5: CustomizeInstanceType metadata refactor to shared DescriptionItems looks solid.
updateMetadatareturningPromise.resolve(updateCustomizeInstanceType(...))fits the expected asyncon...Submitcontract without changingupdateCustomizeInstanceTypesemantics.- Using
DescriptionItemLabels/DescriptionItemAnnotationswithmodel={VirtualMachineModel},resource={vm}, and the appropriateSearchItemwrappers keeps behavior consistent while removing modal plumbing.Also applies to: 20-28, 37-50
src/utils/components/DescriptionItem/DescriptionItem.tsx (1)
2-3:isLabelEditoraddition toDescriptionItemcleanly scopes label-editor styling.The new
isLabelEditorflag andclassNamesusage correctly adjust the header layout (space-between + full width) and applyco-editable-label-groupto the description wrapper only when needed, without affecting existing consumers that don’t opt in.Also applies to: 32-33, 52-53, 96-100, 132-135
src/utils/components/DescriptionItem/components/DescriptionItemNamespace.tsx (1)
8-10: EnsureK8sResourceCommonis available and verifyclusterhandling onFleetResourceLink.
- Line 15 uses
resource: K8sResourceCommonbut there is no import forK8sResourceCommonin this file. If it is not provided via global typings, this will fail TS compilation; please import it from@openshift-console/dynamic-plugin-sdk(e.g.import { K8sModel, K8sResourceCommon, getGroupVersionKindForModel } from '@openshift-console/dynamic-plugin-sdk';) or confirm it is globally declared.- Lines 25–37:
clustermay beundefinedwhengetCluster(resource)returns nothing. Please confirmFleetResourceLinkaccepts an undefined/optionalclusterprop and falls back correctly, or gate the prop (e.g. passcluster={cluster}only when truthy).As per coding guidelines, this keeps typings and external component usage robust.
Also applies to: 12-16, 25-39
src/views/topology/components/vm/VMDetailsPanel/components/VMDetailsPanelLeftColumn/components/VMLabelsDetailsItem.tsx (1)
3-6: Labels refactor toDescriptionItemLabelslooks soundUsing
DescriptionItemLabelswithmodel={VirtualMachineModel},resource={vm}, andonLabelsSubmit={(labels) => updateLabels(vm, labels)}preserves the previous behavior while de‑duplicating the logic and aligning the header withSearchItem id="labels". No issues from a typing or behavioral perspective here.Also applies to: 17-23
galkremer1
left a comment
galkremer1
left a comment
There was a problem hiding this comment.
Looking good, just a couple of nits.
src/utils/components/DescriptionItem/components/DescriptionItemAnnotations.tsx
Show resolved
Hide resolved
src/utils/components/DescriptionItem/components/DescriptionItemLabels.tsx
Show resolved
Hide resolved
| <LabelGroup className="metadata-labels-group" numLabels={10}> | ||
| {Object.keys(labels || {})?.map((key) => { | ||
| <LabelGroup numLabels={10}> | ||
| {labelsKeys?.map((key) => { |
There was a problem hiding this comment.
According to the code labelsKeys will always be an array so there is no need for the optional chaining here.
adamviktora
force-pushed
the
refactoring-desc-items
branch
from
a837759 to
a72711e
Compare
|
@adamviktora: This pull request references CNV-73006 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 task to target the "4.21.0" version, but no target version was set. In 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. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/utils/components/MetadataLabels/MetadataLabels.tsx (1)
6-6: Consider simplifying the empty-state check for labels
labelsKeysis always an array, so usingisEmptyis a bit indirect. A direct length check is simpler and removes the extra dependency.-import { isEmpty } from '@kubevirt-utils/utils/utils'; +// isEmpty no longer needed const MetadataLabels: FC<MetadataLabelsProps> = ({ labels, model }) => { @@ - const labelsKeys = Object.keys(labels || {}); - - if (isEmpty(labelsKeys)) { + const labelsKeys = Object.keys(labels || {}); + + if (labelsKeys.length === 0) { return <div className="pf-v6-u-text-color-subtle">{t('No labels')}</div>; }Also applies to: 20-24
src/utils/components/DescriptionItem/components/DescriptionItemNamespace.tsx (1)
25-39: Consider guarding for missing namespace/cluster before rendering FleetResourceLinkIf
getNamespace(resource)orgetCluster(resource)can beundefined(e.g., for cluster-scoped resources or single-cluster cases),FleetResourceLinkmay render a broken link. Consider a small guard and fallback text (or a non-clickable element) when either value is missing, so the UI degrades gracefully instead of showing an invalid link.src/utils/components/DescriptionItem/DescriptionItem.tsx (1)
64-90: AddisEditto theuseMemodependency list for correctnessThe memoized
descriptionusesisEditin its condition butisEditis not in the dependency array. IfisEditcan change over the lifecycle of this component, the memoized value may become stale. Including it keeps the memo in sync with the actual edit state.const description = useMemo( @@ - ), - [ + ), + [ descriptionData, isDisabled, + isEdit, onEditClick, messageOnDisabled, testId, additionalContent, showEditOnTitle, ], );src/views/virtualmachines/details/tabs/configuration/metadata/MetadataTab.tsx (1)
23-35: Double-checkSearchItemid used for annotations headerFor labels you use
SearchItem id="labels", but for annotations the wrapper usesid="metadata":<SearchItem id="metadata">{t('Metadata')}</SearchItem> // title ... descriptionHeaderWrapper={(children) => ( <SearchItem id="metadata">{children}</SearchItem> // annotations header )}If
idis used as a search/filter key, a more specific value like"annotations"might be clearer and avoid collisions with the top-level metadata section. Worth confirming this aligns with howSearchItemids are consumed.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
src/utils/components/DescriptionItem/DescriptionItem.tsx(5 hunks)src/utils/components/DescriptionItem/components/DescriptionItemAnnotations.tsx(4 hunks)src/utils/components/DescriptionItem/components/DescriptionItemLabels.tsx(3 hunks)src/utils/components/DescriptionItem/components/DescriptionItemNamespace.tsx(3 hunks)src/utils/components/MetadataLabels/MetadataLabels.scss(0 hunks)src/utils/components/MetadataLabels/MetadataLabels.tsx(1 hunks)src/views/catalog/CustomizeInstanceType/tabs/configuration/utils/tabs/CustomizeInstanceTypeMetadataTab.tsx(2 hunks)src/views/catalog/wizard/components/WizardDescriptionItem/WizardDescriptionItem.tsx(4 hunks)src/views/catalog/wizard/tabs/metadata/WizardMetadataTab.tsx(1 hunks)src/views/topology/components/vm/VMDetailsPanel/components/VMDetailsPanelLeftColumn/components/VMAnnotationsDetailsItem.tsx(2 hunks)src/views/topology/components/vm/VMDetailsPanel/components/VMDetailsPanelLeftColumn/components/VMLabelsDetailsItem.tsx(2 hunks)src/views/virtualmachines/details/tabs/configuration/metadata/MetadataTab.tsx(2 hunks)src/views/virtualmachines/details/tabs/configuration/metadata/components/MetadataTabAnnotations/MetadataTabAnnotations.tsx(0 hunks)src/views/virtualmachines/details/tabs/configuration/metadata/components/MetadataTabLabels/MetadataTabLabels.tsx(0 hunks)src/views/virtualmachines/details/tabs/configuration/metadata/components/MetadataTabLabels/metadata-tab-labels.scss(0 hunks)src/views/virtualmachines/details/tabs/overview/components/VirtualMachinesOverviewTabGeneral/VirtualMachinesOverviewTabGeneral.tsx(2 hunks)
💤 Files with no reviewable changes (4)
- src/views/virtualmachines/details/tabs/configuration/metadata/components/MetadataTabLabels/metadata-tab-labels.scss
- src/utils/components/MetadataLabels/MetadataLabels.scss
- src/views/virtualmachines/details/tabs/configuration/metadata/components/MetadataTabLabels/MetadataTabLabels.tsx
- src/views/virtualmachines/details/tabs/configuration/metadata/components/MetadataTabAnnotations/MetadataTabAnnotations.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- src/views/catalog/wizard/tabs/metadata/WizardMetadataTab.tsx
- src/views/topology/components/vm/VMDetailsPanel/components/VMDetailsPanelLeftColumn/components/VMAnnotationsDetailsItem.tsx
- src/views/virtualmachines/details/tabs/overview/components/VirtualMachinesOverviewTabGeneral/VirtualMachinesOverviewTabGeneral.tsx
- src/utils/components/DescriptionItem/components/DescriptionItemLabels.tsx
- src/views/catalog/CustomizeInstanceType/tabs/configuration/utils/tabs/CustomizeInstanceTypeMetadataTab.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*.tsx
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.tsx: Use.tsxfile extension for all React components. One component per file. No nested components.
Use PascalCase for all component names (e.g.,HeaderTop.tsx).
Use functional components by default. Use class components only for specific lifecycle methods unavailable in functional components (e.g.,componentDidCatch).
Use default exports for all components.
Use React's memoization tools (React.memo,useMemo,useCallback) to avoid unnecessary re-renders. Lazy load components withReact.lazyandSuspense.
Always specify dependencies inuseEffectto avoid unnecessary re-renders or missed updates. Pass an empty array[]to run the effect only once if no dependencies are required.
Files:
src/utils/components/MetadataLabels/MetadataLabels.tsxsrc/views/topology/components/vm/VMDetailsPanel/components/VMDetailsPanelLeftColumn/components/VMLabelsDetailsItem.tsxsrc/utils/components/DescriptionItem/components/DescriptionItemAnnotations.tsxsrc/utils/components/DescriptionItem/DescriptionItem.tsxsrc/utils/components/DescriptionItem/components/DescriptionItemNamespace.tsxsrc/views/catalog/wizard/components/WizardDescriptionItem/WizardDescriptionItem.tsxsrc/views/virtualmachines/details/tabs/configuration/metadata/MetadataTab.tsx
**/*.{tsx,ts}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{tsx,ts}: Extract as much logic as possible from components into custom hooks or utility files to improve testability. Avoid bloated components.
Hooks should contain only logic and side effects, not return JSX. Keep JSX in components while using hooks for extracting reusable or unit-testable logic.
Files:
src/utils/components/MetadataLabels/MetadataLabels.tsxsrc/views/topology/components/vm/VMDetailsPanel/components/VMDetailsPanelLeftColumn/components/VMLabelsDetailsItem.tsxsrc/utils/components/DescriptionItem/components/DescriptionItemAnnotations.tsxsrc/utils/components/DescriptionItem/DescriptionItem.tsxsrc/utils/components/DescriptionItem/components/DescriptionItemNamespace.tsxsrc/views/catalog/wizard/components/WizardDescriptionItem/WizardDescriptionItem.tsxsrc/views/virtualmachines/details/tabs/configuration/metadata/MetadataTab.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{ts,tsx}: Use descriptive names for variables, functions, and components. Avoid abbreviations unless widely recognized.
Prefer usingtypeinstead ofinterfacefor defining the shapes of objects or functions in TypeScript.
Avoid usinganytype in TypeScript as it compromises type safety. Useunknowninstead and narrow the type as needed.
Always explicitly define return types for functions in TypeScript rather than relying on type inference.
Files:
src/utils/components/MetadataLabels/MetadataLabels.tsxsrc/views/topology/components/vm/VMDetailsPanel/components/VMDetailsPanelLeftColumn/components/VMLabelsDetailsItem.tsxsrc/utils/components/DescriptionItem/components/DescriptionItemAnnotations.tsxsrc/utils/components/DescriptionItem/DescriptionItem.tsxsrc/utils/components/DescriptionItem/components/DescriptionItemNamespace.tsxsrc/views/catalog/wizard/components/WizardDescriptionItem/WizardDescriptionItem.tsxsrc/views/virtualmachines/details/tabs/configuration/metadata/MetadataTab.tsx
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: kubevirt-ui/kubevirt-plugin PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-04T14:08:48.257Z
Learning: Add a main PR comment that explains referenced links, Jira, documentation, use cases, and current/required state. Include videos and/or screenshots for visual changes.
📚 Learning: 2025-12-04T14:08:48.257Z
Learnt from: CR
Repo: kubevirt-ui/kubevirt-plugin PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-04T14:08:48.257Z
Learning: Applies to **/*.tsx : Use React's memoization tools (`React.memo`, `useMemo`, `useCallback`) to avoid unnecessary re-renders. Lazy load components with `React.lazy` and `Suspense`.
Applied to files:
src/utils/components/DescriptionItem/DescriptionItem.tsx
🧬 Code graph analysis (4)
src/utils/components/MetadataLabels/MetadataLabels.tsx (2)
src/utils/utils/utils.ts (1)
isEmpty(32-33)src/utils/components/Labels/LabelList.tsx (1)
Label(17-35)
src/views/topology/components/vm/VMDetailsPanel/components/VMDetailsPanelLeftColumn/components/VMLabelsDetailsItem.tsx (2)
src/views/virtualmachines/details/tabs/configuration/utils/search.ts (1)
SearchItem(6-11)src/views/virtualmachines/details/tabs/configuration/details/utils/utils.ts (1)
updateLabels(117-118)
src/utils/components/DescriptionItem/components/DescriptionItemNamespace.tsx (1)
src/multicluster/helpers/selectors.ts (1)
getCluster(6-7)
src/views/virtualmachines/details/tabs/configuration/metadata/MetadataTab.tsx (2)
src/views/virtualmachines/details/tabs/configuration/utils/search.ts (1)
SearchItem(6-11)src/views/virtualmachines/details/tabs/configuration/details/utils/utils.ts (2)
updateLabels(117-118)updateAnnotation(114-115)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: unit-test
- GitHub Check: i18n
- GitHub Check: build
🔇 Additional comments (4)
src/views/topology/components/vm/VMDetailsPanel/components/VMDetailsPanelLeftColumn/components/VMLabelsDetailsItem.tsx (1)
17-22: Labels wiring via DescriptionItemLabels looks consistent
DescriptionItemLabelsreceives the VM model, resource,SearchItem-wrapped header, andonLabelsSubmittied toupdateLabels(vm, labels); this aligns cleanly with the new shared labels editor pattern.src/views/catalog/wizard/components/WizardDescriptionItem/WizardDescriptionItem.tsx (1)
37-39: Well-scoped isLabelEditor styling hookAdding
isLabelEditorwith a default offalseand conditionally applying theco-editable-label-groupclass onDescriptionListDescriptionkeeps label-editor-specific styling isolated without affecting existing WizardDescriptionItem usages.Also applies to: 61-62, 142-144
src/utils/components/DescriptionItem/DescriptionItem.tsx (1)
32-33: isLabelEditor-driven layout changes look correctUsing
isLabelEditorto:
- switch
justifyContenttospace-between,- add
pf-v6-u-w-100on the headerFlex, and- apply
co-editable-label-grouponDescriptionListDescriptionprovides the needed full-width, editable-label layout while leaving non-label usages unchanged.
Also applies to: 52-53, 96-100, 132-135
src/utils/components/DescriptionItem/components/DescriptionItemAnnotations.tsx (1)
1-3: Flexible annotations header and submit wiring looks goodThe additions of:
classNamepassthrough toDescriptionItem,descriptionHeaderWrapperto optionally wrap the"Annotations"header, andonAnnotationsSubmitwith a sensible fallback toonAnnotationsSubmitInternalmake this component reusable across contexts (e.g., VM details, metadata tab) without breaking existing behavior. The
onSubmit={onAnnotationsSubmit ?? onAnnotationsSubmitInternal}pattern keeps the default patch flow intact while allowing callers to override it.Also applies to: 11-18, 21-29, 37-38, 50-57, 60-62, 70-73
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: adamviktora, galkremer1 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
3 participants
📝 Description
🎥 Demo
Before:
After:
Summary by CodeRabbit
Refactor
Style
UX
✏️ Tip: You can customize this high-level summary in your review settings.