CNV-74672: improve tooltip of "Select threshold" in Load balance feature #3319
Conversation
|
@adamviktora: This pull request references CNV-74672 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 bug 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. |
📝 WalkthroughWalkthroughAdded a new localization key "Install load balance feature first to select a threshold." to six locale files and updated DeschedulerSection to accept an Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (9)
🚧 Files skipped from review as they are similar to previous changes (5)
🧰 Additional context used📓 Path-based instructions (6)**/*.tsx📄 CodeRabbit inference engine (CODING_STANDARDS.md)
Files:
**/*.{tsx,scss}📄 CodeRabbit inference engine (CODING_STANDARDS.md)
Files:
**/*.{tsx,ts}📄 CodeRabbit inference engine (CODING_STANDARDS.md)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (CODING_STANDARDS.md)
Files:
**/*.{ts,tsx,js}📄 CodeRabbit inference engine (CODING_STANDARDS.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (CODING_STANDARDS.md)
Files:
🧠 Learnings (7)📓 Common learnings📚 Learning: 2025-11-06T13:11:11.791ZApplied to files:
📚 Learning: 2025-11-20T20:28:17.043ZApplied to files:
📚 Learning: 2025-12-09T17:30:56.131ZApplied to files:
📚 Learning: 2025-12-24T13:50:10.254ZApplied to files:
📚 Learning: 2025-12-24T13:50:10.254ZApplied to files:
📚 Learning: 2025-12-12T17:26:25.615ZApplied to files:
⏰ 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). (1)
🔇 Additional comments (6)
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-74672 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 bug 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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In
@src/views/clusteroverview/SettingsTab/ClusterTab/components/VirtualizationFeaturesSection/VirtualizationFeaturesList/components/LoadBalanceSection/components/DeschedulerSection.tsx:
- Around line 46-53: The useMemo creating tooltipContent references the
translation function t but doesn’t include it in the dependency array; update
the useMemo call (tooltipContent = useMemo(..., [isOperatorInstalled,
hasDescheduler])) to include t so the memo recalculates when the active
translation function changes (i.e., use [isOperatorInstalled, hasDescheduler,
t]).
🧹 Nitpick comments (1)
locales/en/plugin__kubevirt-plugin.json (1)
803-803: Tooltip copy is clear; consider a small grammar tweakThe new key is well‑scoped and matches the Load balance / “Select threshold” interaction. For slightly smoother English, you might consider:
- “Install the Load balance feature first to select a threshold.” or
- “Install the Load balance feature before selecting a threshold.”
Here’s a minimal diff using the first option:
Proposed wording tweak (optional)
- "Install Load balance feature first to select a threshold.": "Install Load balance feature first to select a threshold.", + "Install the Load balance feature first to select a threshold.": "Install the Load balance feature first to select a threshold.",
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
locales/en/plugin__kubevirt-plugin.jsonlocales/es/plugin__kubevirt-plugin.jsonlocales/fr/plugin__kubevirt-plugin.jsonlocales/ja/plugin__kubevirt-plugin.jsonlocales/ko/plugin__kubevirt-plugin.jsonlocales/zh/plugin__kubevirt-plugin.jsonsrc/views/clusteroverview/SettingsTab/ClusterTab/components/VirtualizationFeaturesSection/VirtualizationFeaturesList/components/LoadBalanceSection/LoadBalanceSection.tsxsrc/views/clusteroverview/SettingsTab/ClusterTab/components/VirtualizationFeaturesSection/VirtualizationFeaturesList/components/LoadBalanceSection/components/DeschedulerSection.tsxsrc/views/clusteroverview/SettingsTab/ClusterTab/components/VirtualizationFeaturesSection/VirtualizationFeaturesWizard/components/ConfigurationStep/components/LoadBalanceConfigurationSection/LoadBalanceConfigurationSection.tsx
🧰 Additional context used
📓 Path-based instructions (6)
**/*.tsx
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.tsx: Use.tsxfile extension for all React components. One component per file with no nested components.
Use PascalCase for all React component names (e.g.,HeaderTop.tsx)
Use functional components by default. Only use class components for specific lifecycle methods unavailable in functional components (e.g.,componentDidCatch)
Use default exports for all React components
UseReact.lazyandSuspensefor lazy loading components
Files:
src/views/clusteroverview/SettingsTab/ClusterTab/components/VirtualizationFeaturesSection/VirtualizationFeaturesWizard/components/ConfigurationStep/components/LoadBalanceConfigurationSection/LoadBalanceConfigurationSection.tsxsrc/views/clusteroverview/SettingsTab/ClusterTab/components/VirtualizationFeaturesSection/VirtualizationFeaturesList/components/LoadBalanceSection/components/DeschedulerSection.tsxsrc/views/clusteroverview/SettingsTab/ClusterTab/components/VirtualizationFeaturesSection/VirtualizationFeaturesList/components/LoadBalanceSection/LoadBalanceSection.tsx
**/*.{tsx,scss}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
Use project-based class names on components as anchors for styling rules rather than relying on PatternFly class names
Files:
src/views/clusteroverview/SettingsTab/ClusterTab/components/VirtualizationFeaturesSection/VirtualizationFeaturesWizard/components/ConfigurationStep/components/LoadBalanceConfigurationSection/LoadBalanceConfigurationSection.tsxsrc/views/clusteroverview/SettingsTab/ClusterTab/components/VirtualizationFeaturesSection/VirtualizationFeaturesList/components/LoadBalanceSection/components/DeschedulerSection.tsxsrc/views/clusteroverview/SettingsTab/ClusterTab/components/VirtualizationFeaturesSection/VirtualizationFeaturesList/components/LoadBalanceSection/LoadBalanceSection.tsx
**/*.{tsx,ts}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{tsx,ts}: Extract logic from components into custom hooks or utility files to improve testability and component maintainability
Use React memoization tools (React.memo,useMemo,useCallback) to avoid unnecessary re-renders
Always specify dependencies inuseEffectto avoid unnecessary re-renders or missed updates. Use an empty array[]if no dependencies are required
Files:
src/views/clusteroverview/SettingsTab/ClusterTab/components/VirtualizationFeaturesSection/VirtualizationFeaturesWizard/components/ConfigurationStep/components/LoadBalanceConfigurationSection/LoadBalanceConfigurationSection.tsxsrc/views/clusteroverview/SettingsTab/ClusterTab/components/VirtualizationFeaturesSection/VirtualizationFeaturesList/components/LoadBalanceSection/components/DeschedulerSection.tsxsrc/views/clusteroverview/SettingsTab/ClusterTab/components/VirtualizationFeaturesSection/VirtualizationFeaturesList/components/LoadBalanceSection/LoadBalanceSection.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{ts,tsx,js,jsx}: Keep files under 150 lines whenever possible
Use descriptive names for variables, functions, and components. Avoid abbreviations unless widely recognized
Keep functions short and focused on one action. Apply Red → Green → Refactor methodology
Avoid hardcoded values (magic numbers) and define them as constants for easy adjustments and readability
Files:
src/views/clusteroverview/SettingsTab/ClusterTab/components/VirtualizationFeaturesSection/VirtualizationFeaturesWizard/components/ConfigurationStep/components/LoadBalanceConfigurationSection/LoadBalanceConfigurationSection.tsxsrc/views/clusteroverview/SettingsTab/ClusterTab/components/VirtualizationFeaturesSection/VirtualizationFeaturesList/components/LoadBalanceSection/components/DeschedulerSection.tsxsrc/views/clusteroverview/SettingsTab/ClusterTab/components/VirtualizationFeaturesSection/VirtualizationFeaturesList/components/LoadBalanceSection/LoadBalanceSection.tsx
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
Define constants in utility files with uppercase and underscore-separated naming (e.g.,
API_URL)
Files:
src/views/clusteroverview/SettingsTab/ClusterTab/components/VirtualizationFeaturesSection/VirtualizationFeaturesWizard/components/ConfigurationStep/components/LoadBalanceConfigurationSection/LoadBalanceConfigurationSection.tsxsrc/views/clusteroverview/SettingsTab/ClusterTab/components/VirtualizationFeaturesSection/VirtualizationFeaturesList/components/LoadBalanceSection/components/DeschedulerSection.tsxsrc/views/clusteroverview/SettingsTab/ClusterTab/components/VirtualizationFeaturesSection/VirtualizationFeaturesList/components/LoadBalanceSection/LoadBalanceSection.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{ts,tsx}: Prefer usingtypeinstead ofinterfacefor defining the shapes of objects or functions in TypeScript
If a type is exported, add it to a utility file
Avoid usinganytype in TypeScript. Useunknowninstead and narrow the type as needed
Always explicitly define return types for functions rather than relying on TypeScript type inference
Files:
src/views/clusteroverview/SettingsTab/ClusterTab/components/VirtualizationFeaturesSection/VirtualizationFeaturesWizard/components/ConfigurationStep/components/LoadBalanceConfigurationSection/LoadBalanceConfigurationSection.tsxsrc/views/clusteroverview/SettingsTab/ClusterTab/components/VirtualizationFeaturesSection/VirtualizationFeaturesList/components/LoadBalanceSection/components/DeschedulerSection.tsxsrc/views/clusteroverview/SettingsTab/ClusterTab/components/VirtualizationFeaturesSection/VirtualizationFeaturesList/components/LoadBalanceSection/LoadBalanceSection.tsx
🧠 Learnings (5)
📚 Learning: 2025-11-06T13:11:11.791Z
Learnt from: rszwajko
Repo: kubevirt-ui/kubevirt-plugin PR: 3089
File: locales/es/plugin__kubevirt-plugin.json:102-102
Timestamp: 2025-11-06T13:11:11.791Z
Learning: For the kubevirt-ui/kubevirt-plugin repository: Do not flag or comment on missing translations (English values) in locale files for languages other than English (e.g., locales/es/, locales/fr/, locales/ja/, locales/ko/, locales/zh/). Only review English locale files (locales/en/) for translation keys.
Applied to files:
locales/en/plugin__kubevirt-plugin.jsonlocales/zh/plugin__kubevirt-plugin.jsonlocales/ko/plugin__kubevirt-plugin.jsonlocales/ja/plugin__kubevirt-plugin.jsonlocales/fr/plugin__kubevirt-plugin.jsonlocales/es/plugin__kubevirt-plugin.json
📚 Learning: 2025-11-20T20:28:17.043Z
Learnt from: galkremer1
Repo: kubevirt-ui/kubevirt-plugin PR: 3118
File: src/views/checkups/self-validation/utils/constants.ts:62-68
Timestamp: 2025-11-20T20:28:17.043Z
Learning: For the kubevirt-ui/kubevirt-plugin repository: The test suite labels in TEST_SUITE_OPTIONS (Compute, Network, Storage, SSP, Tier2) in src/views/checkups/self-validation/utils/constants.ts should not be internationalized, even though they are displayed in the UI.
Applied to files:
locales/en/plugin__kubevirt-plugin.jsonlocales/zh/plugin__kubevirt-plugin.jsonlocales/ja/plugin__kubevirt-plugin.json
📚 Learning: 2025-12-09T17:30:56.131Z
Learnt from: rszwajko
Repo: kubevirt-ui/kubevirt-plugin PR: 3235
File: src/views/checkups/self-validation/details/tabs/details/components/CheckupsSelfValidationDetailsDescriptionList.tsx:26-93
Timestamp: 2025-12-09T17:30:56.131Z
Learning: When using the Timestamp component from openshift-console/dynamic-plugin-sdk, you can pass sentinel values like NO_DATA_DASH directly to the timestamp prop without wrapping in conditional rendering. The component handles invalid/missing values gracefully. This applies to all TSX files that render the Timestamp component; ensure you do not add extra conditional logic for such values and rely on the component's internal handling. Reference: OpenShift Console Timestamp.tsx implementation.
Applied to files:
src/views/clusteroverview/SettingsTab/ClusterTab/components/VirtualizationFeaturesSection/VirtualizationFeaturesWizard/components/ConfigurationStep/components/LoadBalanceConfigurationSection/LoadBalanceConfigurationSection.tsxsrc/views/clusteroverview/SettingsTab/ClusterTab/components/VirtualizationFeaturesSection/VirtualizationFeaturesList/components/LoadBalanceSection/components/DeschedulerSection.tsxsrc/views/clusteroverview/SettingsTab/ClusterTab/components/VirtualizationFeaturesSection/VirtualizationFeaturesList/components/LoadBalanceSection/LoadBalanceSection.tsx
📚 Learning: 2025-12-24T13:50:10.254Z
Learnt from: CR
Repo: kubevirt-ui/kubevirt-plugin PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-24T13:50:10.254Z
Learning: Applies to **/*.{tsx,ts} : Use React memoization tools (`React.memo`, `useMemo`, `useCallback`) to avoid unnecessary re-renders
Applied to files:
src/views/clusteroverview/SettingsTab/ClusterTab/components/VirtualizationFeaturesSection/VirtualizationFeaturesList/components/LoadBalanceSection/components/DeschedulerSection.tsx
📚 Learning: 2025-12-12T17:26:25.615Z
Learnt from: galkremer1
Repo: kubevirt-ui/kubevirt-plugin PR: 3165
File: src/views/checkups/storage/components/hooks/useCheckupsStorageListColumns.ts:25-27
Timestamp: 2025-12-12T17:26:25.615Z
Learning: In the kubevirt-ui/kubevirt-plugin repository: The `useActiveNamespace` hook has two different implementations with different return types:
1. When imported from `'openshift-console/dynamic-plugin-sdk'`, it returns a tuple and should be destructured: `const [namespace] = useActiveNamespace()`
2. When imported from `'kubevirt-utils/hooks/useActiveNamespace'`, it returns a single value: `const namespace = useActiveNamespace()`
Applied to files:
src/views/clusteroverview/SettingsTab/ClusterTab/components/VirtualizationFeaturesSection/VirtualizationFeaturesList/components/LoadBalanceSection/components/DeschedulerSection.tsx
🧬 Code graph analysis (1)
src/views/clusteroverview/SettingsTab/ClusterTab/components/VirtualizationFeaturesSection/VirtualizationFeaturesWizard/components/ConfigurationStep/components/LoadBalanceConfigurationSection/LoadBalanceConfigurationSection.tsx (3)
src/views/clusteroverview/SettingsTab/ClusterTab/components/VirtualizationFeaturesSection/utils/VirtualizationFeaturesContext/VirtualizationFeaturesContext.tsx (1)
useVirtualizationFeaturesContext(19-19)src/views/clusteroverview/SettingsTab/ClusterTab/components/VirtualizationFeaturesSection/utils/constants.ts (1)
DESCHEDULER_OPERATOR_NAME(5-5)src/views/clusteroverview/SettingsTab/ClusterTab/components/VirtualizationFeaturesSection/utils/utils.ts (1)
isInstalled(5-6)
⏰ 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). (1)
- GitHub Check: build
🔇 Additional comments (11)
locales/ja/plugin__kubevirt-plugin.json (1)
799-799: Non‑English locale change acknowledged (no review per repo guidelines)The new key matches the English locale entry. Per kubevirt‑plugin review guidelines, I’m not reviewing or suggesting changes to non‑English translations in this file.
locales/zh/plugin__kubevirt-plugin.json (1)
799-799: New Load balance tooltip string looks correct and consistentKey and value match the intended tooltip text and align with surrounding feature-related entries. No structural or naming issues from an i18n-key perspective. Based on learnings, leaving the English value in this non-English locale is acceptable.
locales/es/plugin__kubevirt-plugin.json (1)
813-813: Tooltip key for Load balance added correctlyThe new key mirrors other locales and keeps the i18n key set consistent; text matches the intended UI tooltip semantics. English value in this non-English locale is acceptable per repository conventions. Based on learnings, no changes needed.
locales/ko/plugin__kubevirt-plugin.json (1)
799-799: Non-English locale key acknowledged (no translation review)Change is limited to adding a new key; per repo guidelines we intentionally do not review or comment on non-English locale content.
Based on learnings, non-English locale files are not reviewed for translation quality.locales/fr/plugin__kubevirt-plugin.json (1)
813-813: Non-English locale key acknowledged (no translation review)This addition just introduces a new i18n key; in line with repo guidance, non-English locale strings are not reviewed for translation content.
Based on learnings, only locales/en is reviewed for wording.src/views/clusteroverview/SettingsTab/ClusterTab/components/VirtualizationFeaturesSection/VirtualizationFeaturesList/components/LoadBalanceSection/LoadBalanceSection.tsx (1)
52-52: Correctly threadingisOperatorInstalledintoDeschedulerSectionReusing the locally derived
isOperatorInstalledhere is clean and keeps responsibility for install-state checks in one place. No issues from this change.src/views/clusteroverview/SettingsTab/ClusterTab/components/VirtualizationFeaturesSection/VirtualizationFeaturesWizard/components/ConfigurationStep/components/LoadBalanceConfigurationSection/LoadBalanceConfigurationSection.tsx (1)
7-9: Consistent use of virtualization context to deriveisOperatorInstalledUsing
useVirtualizationFeaturesContext+DESCHEDULER_OPERATOR_NAME+isInstalledmirrors the list view logic and cleanly passesisOperatorInstalledintoDeschedulerSection. Optional chaining onoperatorDetailsMapis safe, and a missing entry simply treats the operator as not installed, which matches the intended UX. AllDeschedulerSectionusages across both the list and wizard views have been properly updated with the new prop.src/views/clusteroverview/SettingsTab/ClusterTab/components/VirtualizationFeaturesSection/VirtualizationFeaturesList/components/LoadBalanceSection/components/DeschedulerSection.tsx (4)
22-26: LGTM!The type definition correctly uses
typeoverinterfaceper project guidelines, and the component signature is properly typed with the new prop.
44-45: LGTM!Clean derivation of the disabled state. Memoization would be overkill for this simple boolean expression.
81-81: LGTM!Clean usage of the derived
isSelectDisabledvariable.
78-86: LGTM!Good use of the
triggerRefpattern for the tooltip on a potentially disabled element, and conditional rendering ensures the tooltip only appears when the select is disabled.
...n/VirtualizationFeaturesList/components/LoadBalanceSection/components/DeschedulerSection.tsx
Outdated
Show resolved
Hide resolved
galkremer1
left a comment
galkremer1
left a comment
There was a problem hiding this comment.
Looking good - one small nit.
adamviktora
force-pushed
the
descheduler-configure-features-info
branch
from
f70a39f to
a8356ec
Compare
|
@adamviktora: This pull request references CNV-74672 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 bug 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. |
|
[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 DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
3 participants
📝 Description
🎥 Demo
After:
Screen.Recording.2026-01-05.at.13.41.13.mov
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.