[dashboard] fix: allow clearing instanceType and preserve secret copy newlines#2135
[dashboard] fix: allow clearing instanceType and preserve secret copy newlines#2135
Conversation
Update openapi-k8s-toolkit to release/1.4.0 (d6b9e4ad). The previous
value-binding layout refactor is already included upstream, so drop the
formlistinput-value-binding.diff patch.
Add formlistinput-allow-empty.diff patch which introduces two props to
the listInput component:
- allowEmpty: when set, auto-persists the field so BFF sends an empty
value instead of falling back to the schema default
- persistType: controls the type of empty value ('str' | 'number' | 'arr'
| 'obj'), allowing the feature to work correctly for any field type
Set allowEmpty: true on the VMInstance instanceType field so users can
explicitly clear the selection and override the default instance type.
Co-Authored-By: Claude <[email protected]>
Signed-off-by: Kirill Ilin <[email protected]>
… toolkit Update openapi-k8s-toolkit commit to d6b9e4ad (release/1.4.0) which includes the FormListInput layout refactor, making formlistinput-value-binding.diff obsolete. Set allowEmpty: true on the VMInstance instanceType field so users can explicitly clear the selection and override the default instance type. Co-Authored-By: Claude <[email protected]> Signed-off-by: Kirill Ilin <[email protected]>
Add onCopy handler to SecretBase64Plain inputs to intercept native browser copy events and explicitly write the full decoded text (including newlines) to the clipboard. Without this, input[type=text] strips newlines on copy. Upstream PR: PRO-Robotech/openapi-k8s-toolkit#339 Co-Authored-By: Claude <[email protected]> Signed-off-by: Kirill Ilin <[email protected]>
📝 WalkthroughWalkthroughUpdates the VMInstance configuration to support empty values in dropdown fields by adding an "allowEmpty" flag to the override schema. Includes modifications to patch files for form components to handle empty value persistence and type coercion, removes an obsolete FormListInput component, and updates Docker build configuration. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses two user experience issues within the dashboard's Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](proxy.php?url=https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses two user experience issues in the dashboard. First, it allows clearing the instanceType field in the VMInstance form by adding an allowEmpty property. Second, it ensures newlines are preserved when copying secrets. The changes are well-structured, with backend adjustments in Go and frontend fixes delivered as patches. The corresponding tests and dependency updates are also included. I've provided a couple of minor suggestions in the frontend patch files to enhance code clarity and reduce duplication. Overall, this is a solid improvement.
| hasFeedback={designNewLayout ? { icons: feedbackIcons } : true} | ||
| style={{ flex: 1 }} | ||
| + normalize={(value: unknown) => { | ||
| + if (customProps.allowEmpty && (value === undefined || value === null)) { |
There was a problem hiding this comment.
For conciseness, you can use value == null to check for both undefined and null. This is a common JavaScript/TypeScript idiom.
if (customProps.allowEmpty && value == null) {
| + onCopy={e => { | ||
| + if (!effectiveHidden) { | ||
| + e.preventDefault() | ||
| + e.clipboardData?.setData('text/plain', value) | ||
| + } | ||
| + }} |
There was a problem hiding this comment.
This onCopy handler logic is identical to the one added on lines 8-13. To avoid code duplication and improve maintainability, consider extracting this logic into a shared function within the component.
For example:
const handleCopy = (e: React.ClipboardEvent<HTMLInputElement>) => {
if (!effectiveHidden) {
e.preventDefault();
e.clipboardData?.setData('text/plain', value);
}
};Then you can use onCopy={handleCopy} in both places where Styled.DisabledInput is used.
sircthulhu
requested review from
IvanHunters,
androndo,
kvaps,
lexfrei and
lllamnyp
as code owners
dosubot
bot
added
size:L
![coderabbitai[bot]](proxy.php?url=https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/system/dashboard/images/openapi-ui/openapi-k8s-toolkit/patches/secret-copy-preserve-newlines.diff (1)
8-13: Extract duplicatedonCopylogic into a shared handler.Same handler body is repeated twice; centralizing it will reduce drift and keep secret-copy behavior consistent.
♻️ Suggested refactor
+ const handleCopy = (e: React.ClipboardEvent<HTMLInputElement>) => { + if (effectiveHidden) return + e.preventDefault() + e.clipboardData?.setData('text/plain', value) + } ... - onCopy={e => { - if (!effectiveHidden) { - e.preventDefault() - e.clipboardData?.setData('text/plain', value) - } - }} + onCopy={handleCopy} ... - onCopy={e => { - if (!effectiveHidden) { - e.preventDefault() - e.clipboardData?.setData('text/plain', value) - } - }} + onCopy={handleCopy}Also applies to: 21-26
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/system/dashboard/images/openapi-ui/openapi-k8s-toolkit/patches/secret-copy-preserve-newlines.diff` around lines 8 - 13, The duplicated onCopy handler logic (the inline arrow that checks effectiveHidden, calls e.preventDefault() and sets clipboardData with value) should be extracted into a single function (e.g., handleSecretCopy or onCopyHandler) and reused in both places where the same body appears; implement the handler so it closes over or accepts effectiveHidden and value (or receives them as parameters) and replace the two inline onCopy props with a reference to this shared handler, removing the duplicated code to keep secret-copy behavior consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@packages/system/dashboard/images/openapi-ui/openapi-k8s-toolkit/patches/secret-copy-preserve-newlines.diff`:
- Around line 8-13: The duplicated onCopy handler logic (the inline arrow that
checks effectiveHidden, calls e.preventDefault() and sets clipboardData with
value) should be extracted into a single function (e.g., handleSecretCopy or
onCopyHandler) and reused in both places where the same body appears; implement
the handler so it closes over or accepts effectiveHidden and value (or receives
them as parameters) and replace the two inline onCopy props with a reference to
this shared handler, removing the duplicated code to keep secret-copy behavior
consistent.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
internal/controller/dashboard/customformsoverride.gointernal/controller/dashboard/customformsoverride_test.gopackages/system/dashboard/images/openapi-ui/Dockerfilepackages/system/dashboard/images/openapi-ui/openapi-k8s-toolkit/patches/formlistinput-allow-empty.diffpackages/system/dashboard/images/openapi-ui/openapi-k8s-toolkit/patches/formlistinput-value-binding.diffpackages/system/dashboard/images/openapi-ui/openapi-k8s-toolkit/patches/secret-copy-preserve-newlines.diff
💤 Files with no reviewable changes (1)
- packages/system/dashboard/images/openapi-ui/openapi-k8s-toolkit/patches/formlistinput-value-binding.diff
|
Successfully created backport PR for |
2 participants
What this PR does
Updates the openapi-k8s-toolkit integration in the dashboard to fix two UX issues:
1. Allow clearing the instanceType field in VMInstance form
When
instanceTypehas a default value, clearing the field in the form UI wouldsilently revert to the default, making it impossible to explicitly send an empty
value. This blocked use of custom KubeVirt resources without a named instance type.
Adds
allowEmpty: trueto the instanceType listInput field so the BFF preservesan explicit empty value. Also introduces a generic
persistTypeprop(
'str' | 'number' | 'arr' | 'obj') to the listInput component, so theallow-empty behaviour works correctly for any field type, not just strings.
Updates openapi-k8s-toolkit to release/1.4.0 (d6b9e4ad), which already includes
the FormListInput layout refactor — the previous formlistinput-value-binding.diff
patch is no longer needed.
Upstream PR: PRO-Robotech/openapi-k8s-toolkit#340
2. Preserve newlines when copying secrets with CMD+C
Native
input[type=text]strips newlines on copy. Adds anonCopyhandler tothe SecretBase64Plain component that intercepts the copy event and writes the full
decoded value (including newlines) to the clipboard.
Upstream PR: PRO-Robotech/openapi-k8s-toolkit#339
Release note
Summary by CodeRabbit
New Features
Bug Fixes
Chores