Mirror: fix: prevent Create New Mode form fields from resetting (#5826)#34
Mirror: fix: prevent Create New Mode form fields from resetting (#5826)#34jeremylongshore merged 1 commit intomainfrom
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ✨ 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 @jeremylongshore, 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 significantly enhances the user experience within the 'Create New Mode' functionality and navigation. By replacing problematic web components with native HTML elements, it eliminates frustrating form field resets during input. Additionally, it fixes a critical navigation bug that previously led to a blank screen when accessing the 'modes' tab, ensuring a smoother and more reliable user journey. Highlights
Changelog
Activity
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
|
Review Summary by QodoFix Create New Mode form fields resetting and modes tab navigation
WalkthroughsDescription• Replace VS Code web components with native HTML elements to prevent form field resets during re-renders • Add CSS styling to native inputs for VS Code theme consistency • Redirect "modes" tab navigation to settings with modes section selected • Fix blank screen issue when navigating to modes tab from marketplace Diagramflowchart LR
A["VS Code Web Components"] -->|"Replace with native HTML"| B["Native textarea/input elements"]
B -->|"Add CSS styling"| C["VS Code themed inputs"]
D["Modes tab navigation"] -->|"Redirect to settings"| E["Settings with modes section"]
C -->|"Prevent re-renders"| F["Form fields persist while typing"]
E -->|"Fix blank screen"| F
File Changes1. .changeset/fix-create-mode-form-reset.md
|
![gemini-code-assist[bot]](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 issues: it prevents form fields in the 'Create New Mode' dialog from resetting during input, and it fixes a blank screen when navigating to the modes tab. The form reset issue is resolved by replacing VS Code's web components (VSCodeTextArea, VSCodeTextField) with native HTML elements and custom controlled components, which is a solid approach. The navigation fix correctly redirects the 'modes' tab to the appropriate section within the settings view.
My main feedback is on code duplication. The new textarea elements have a long, duplicated string of CSS classes for styling. I've suggested creating a reusable Textarea component to encapsulate these styles, improving maintainability and consistency, similar to how the Input component is already being used.
| <textarea | ||
| value={newModeRoleDefinition} | ||
| onChange={(e) => { | ||
| setNewModeRoleDefinition((e.target as HTMLTextAreaElement).value) | ||
| setNewModeRoleDefinition(e.target.value) | ||
| }} | ||
| rows={4} | ||
| className="w-full" | ||
| className="w-full resize-y bg-vscode-input-background text-vscode-input-foreground border border-vscode-input-border rounded p-2 font-[var(--vscode-font-family)] text-[13px]" | ||
| /> |
There was a problem hiding this comment.
While replacing VSCodeTextArea with a native <textarea> is a good fix for the state reset issue, the styling is applied via a long className string that is duplicated across three text areas in this form (here, and on lines 1609 and 1655).
To improve maintainability and consistency, consider creating a reusable Textarea component in @src/components/ui that encapsulates these styles, similar to how the Input component is used. This would make the code cleaner and ensure all text areas in the form have a consistent appearance.
If the existing Textarea component in webview-ui/src/components/ui/textarea.tsx doesn't meet the styling needs, it could be updated, or a new, more specific component could be created.
Code Review by Qodo
1. ModesView edits lack marker
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| <textarea | ||
| value={newModeRoleDefinition} | ||
| onChange={(e) => { | ||
| setNewModeRoleDefinition((e.target as HTMLTextAreaElement).value) | ||
| setNewModeRoleDefinition(e.target.value) | ||
| }} | ||
| rows={4} | ||
| className="w-full" | ||
| className="w-full resize-y bg-vscode-input-background text-vscode-input-foreground border border-vscode-input-border rounded p-2 font-[var(--vscode-font-family)] text-[13px]" | ||
| /> |
There was a problem hiding this comment.
1. Modesview edits lack marker 📘 Rule violation ✓ Correctness
Edits in webview-ui/src/components/modes/ModesView.tsx were made without adding kilocode_change markers around the modified blocks, increasing upstream sync merge-conflict risk. Upstream-shared code changes under webview-ui/ must be explicitly marked.
Agent Prompt
## Issue description
Edits were made in an upstream-shared `webview-ui/` file without adding `kilocode_change` markers around the modified blocks.
## Issue Context
Per repo compliance rules, Kilo-specific edits in upstream-shared paths must be clearly marked to reduce merge conflicts during upstream syncs.
## Fix Focus Areas
- webview-ui/src/components/modes/ModesView.tsx[1571-1662]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| let targetTab = message.tab as Tab | ||
| // kilocode_change start - Redirect "modes" tab to settings with modes section | ||
| if (targetTab === "modes") { | ||
| targetTab = "settings" | ||
| setCurrentSection("modes") | ||
| } | ||
| // kilocode_change end | ||
| // kilocode_change start - Handle auth tab with returnTo and profileName parameters | ||
| if (targetTab === "auth") { | ||
| if (message.values?.returnTo) { |
There was a problem hiding this comment.
2. Modes redirect drops section 🐞 Bug ✓ Correctness
When handling action:switchTab(tab="modes"), App.tsx sets the section but then switchTab() clears it and the handler overwrites it with message.values.section (undefined for the marketplace link). Additionally, "modes" is not a valid SettingsView section id, so even preserving it would not show the Modes UI.
Agent Prompt
### Issue description
`action:switchTab` handling for `tab="modes"` currently attempts to set `currentSection("modes")`, but that value is cleared by `switchTab()` and then overwritten by `setCurrentSection(message.values?.section)` (typically `undefined`). Even if it weren’t overwritten, `SettingsView` does not recognize `"modes"` as a valid section/tab; the Modes UI is under the `"agentBehaviour"` settings section.
### Issue Context
- Marketplace sends `vscode.postMessage({ type: "switchTab", tab: "modes" })` without `values.section`.
- `switchTab()` clears `currentSection`.
- `SettingsView` only renders `AgentBehaviourView` when `activeTab === "agentBehaviour"`, and `AgentBehaviourView` defaults to its internal `"modes"` sub-tab.
### Fix Focus Areas
- webview-ui/src/App.tsx[132-153]
- webview-ui/src/App.tsx[176-202]
- webview-ui/src/components/settings/SettingsView.tsx[106-151]
- webview-ui/src/components/settings/SettingsView.tsx[1252-1254]
### Suggested approach
- In the `switchTab` action handler, introduce a local `let targetSection = message.values?.section as string | undefined`.
- If `message.tab === "modes"`, set `targetTab = "settings"` and `targetSection = "agentBehaviour"`.
- Call `switchTab(targetTab)` first, then `setCurrentSection(targetSection)` once (avoid setting it twice).
- Optionally: if `targetTab === "settings"` and `targetSection` is `"modes"` or `"mcp"`, normalize it to `"agentBehaviour"` to support existing senders.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Failed to generate code suggestions for PR |
Review Summary
Checklist
AnalysisFix 1: Form Reset (ModesView.tsx)Replaces VS Code web components with native HTML elements:
VS Code web components reset their internal value on re-render. Native elements don't. Same pattern as Kilo-Org#5634. The native elements use proper VS Code theme variables ( Fix 2: Blank Screen on Navigation (App.tsx)Redirects RecommendationAPPROVE — Clean fixes for two related UX bugs. Merge. |
Review Journal: kilocode Kilo-Org#5826
SummaryFixes two related UX bugs: (1) VS Code web components ( What ChangedFix 1: Form Reset (ModesView.tsx)Replaced 3 This is the same root cause as PR Kilo-Org#5634 — VS Code web components have internal state that conflicts with React's controlled input pattern. Fix 2: Blank Screen Navigation (App.tsx)When navigating from the marketplace to the "modes" tab, the app redirected to a non-existent standalone view. The fix redirects to the settings view with Lessons Learned
Review #15 of 75 | Methodology |
1 participant
Mirror of upstream PR #5826
Verdict: APPROVE | Tier 2
Review: kilocode-5826-review.md