-
Notifications
You must be signed in to change notification settings - Fork 2.4k
feat(mode): auto select imported mode #8506
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found some issues that need attention related to performance and consistency around the new auto-select behavior.
| // Import rules files (this also handles cleanup of existing rules folders) | ||
| await this.importRulesFiles(importMode, rulesFiles || [], source) | ||
|
|
||
| importedSlugs.push(importMode.slug) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P2] Performance: importModeWithRules performs updateCustomMode inside the loop (each call reads/merges/writes and triggers state refresh). With multiple modes this causes redundant work plus an extra refresh after the loop. Consider batching: write all mode updates in-memory and perform a single write/refresh at the end of the loop to reduce IO and recompute overhead.
| // Switch to the first imported mode if available | ||
| if (result.importedSlugs && result.importedSlugs.length > 0) { | ||
| const firstImportedSlug = result.importedSlugs[0] | ||
| await updateGlobalState("mode", firstImportedSlug) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P3] Consistency: The mode is set both via updateGlobalState("mode", firstImportedSlug) and by emitting importedSlug for the UI to set visualMode. This dual-path selection works but splits responsibility between backend state and frontend UI. Consider driving selection from one path (e.g., only emit a mode change message and rely on postStateToWebview), so side-effects and telemetry remain centralized.
|
I incorrectly linked the PR lol so someone else did another implementation, see: #8521 |
Related GitHub Issue
Closes: #8234
Roo Code Task Context (Optional)
Description
This PR adds ability to automatically select the imported mode, making it more obvious if the mode has been successfully imported or not.
Test Procedure
Pre-Submission Checklist
Screenshots / Videos
Documentation Updates
Additional Notes
Get in Touch
@elianiva
Important
Automatically select the first imported mode after a successful import, updating global state and UI accordingly.
CustomModesManagerandwebviewMessageHandler.importedSlugsand switches to the first imported mode.ModesView.tsxupdated to handle mode import dialog and switch to the imported mode.CustomModesManager.spec.tsto verify mode import and selection behavior.This description was created by
for ce64978. You can customize this summary. It will automatically update as commits are pushed.