-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: automatically switch to imported mode after successful import (#6491) #6507
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
feat: automatically switch to imported mode after successful import (#6491) #6507
Conversation
…ooCodeInc#6491) - Add importedModes field to ImportResult interface to track imported mode slugs - Modify importModeWithRules to collect and return imported mode slugs - Update webview message handler to automatically switch to first imported mode - Add proper validation and error handling for mode switching - Update ModesView to handle automatic mode switching in UI - Add comprehensive tests for all components Fixes RooCodeInc#6491
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.
Thank you for your contribution! I've reviewed the changes and the implementation looks solid overall. The automatic mode switching after import is working as expected with proper error handling and backward compatibility.
I have a few suggestions for improvement that could make the implementation even more robust.
| return () => window.removeEventListener("message", handler) | ||
| }, []) // Empty dependency array - only register once | ||
| // Re-register handler when dependencies change to ensure it has access to latest values | ||
| }, [customModes, findModeBySlug, handleModeSwitch, modes]) |
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.
The component re-registers the message event handler when dependencies change, which could potentially cause race conditions if multiple imports happen quickly. Consider using a more stable event handling pattern, perhaps with a cleanup function that properly removes the previous handler before adding a new one, or using a ref to maintain handler stability.
|
|
||
| if (validMode) { | ||
| // Validate that the mode slug is a valid string before type assertion | ||
| if (typeof modeToSwitch === "string" && modeToSwitch.length > 0) { |
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.
The mode slug validation uses type assertion without proper type guards. Consider adding a type guard function for safer validation:
| const relativePath = path.relative(modeRulesDir, filePath) | ||
| // Normalize path to use forward slashes for cross-platform compatibility | ||
| const normalizedRelativePath = relativePath.replace(/\\/g, '/') | ||
| const normalizedRelativePath = relativePath.replace(/\\/g, "/") |
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.
Minor style inconsistency - this line uses double quotes while the rest of the file uses single quotes. Consider maintaining consistent quote style:
Related GitHub Issue
Closes: #6491
Roo Code Task Context (Optional)
No Roo Code task context for this PR
Description
This PR implements automatic mode switching after importing custom modes. When users import a mode via the import button, they are now automatically switched to the imported mode, eliminating the need for manual switching.
Key implementation details:
importedModesfield toImportResultinterface to track imported mode slugsimportModeWithRulesmethod to collect and return imported mode slugsTest Procedure
Automated Tests:
cd src && npx vitest run core/config/__tests__/CustomModesManager.spec.tscd src && npx vitest run core/webview/__tests__/webviewMessageHandler.spec.tscd webview-ui && npx vitest run src/components/modes/__tests__/ModesView.spec.tsxAll 72 tests pass (49 + 12 + 11 across three test suites).
Manual Testing Steps:
Pre-Submission Checklist
Screenshots / Videos
No UI changes in this PR
Documentation Updates
Additional Notes
This feature improves user workflow by reducing manual steps when importing custom modes. The implementation:
Get in Touch
@MuriloFP
Important
Automatically switch to the first imported mode after a successful import, with UI and backend updates to handle mode switching and error handling.
webviewMessageHandler.ts.importModeWithRulesinCustomModesManager.tsnow returnsimportedModes.webviewMessageHandler.tsupdated to switch modes post-import and handle errors.ModesView.tsxupdated to handle import results and switch modes.CustomModesManager.spec.tsandwebviewMessageHandler.spec.tsfor import functionality.ModesView.spec.tsxfor mode switching on import.This description was created by
for 0a70720. You can customize this summary. It will automatically update as commits are pushed.