-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: add visual indicators for global/project custom modes (#6502) #6524
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: add visual indicators for global/project custom modes (#6502) #6524
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.
Thank you for your contribution! I've reviewed the changes and the implementation looks solid. The visual indicators for global/project custom modes are well-implemented with good backward compatibility and comprehensive i18n support. I've left some suggestions inline that could improve type safety and maintainability.
| const getModeSource = (mode: ModeConfig): string | null => { | ||
| const isCustom = customModes?.some((m) => m.slug === mode.slug) | ||
| if (!isCustom) return null | ||
| return mode.source || "global" // Default to global if source is undefined |
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.
Consider extracting the source type for better type safety. You could define:
Then use it in the ModeConfig interface. This would prevent typos and make the code more maintainable.
| const getModeSource = (mode: ModeConfig): string | null => { | ||
| const isCustom = customModes?.some((m) => m.slug === mode.slug) | ||
| if (!isCustom) return null | ||
| return mode.source || "global" // Default to global if source is undefined |
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.
Consider extracting this default value to a constant:
This would make it easier to maintain and update if needed.
| const renderSourceIndicator = (mode: ModeConfig, isShort: boolean = false) => { | ||
| const source = getModeSource(mode) | ||
| if (!source) return null | ||
| return <span className="ml-1 text-vscode-descriptionForeground">({getSourceDisplayText(source, isShort)})</span> |
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.
For better accessibility, consider adding an aria-label to help screen reader users:
custom mode
| <div className="font-bold truncate">{mode.name}</div> | ||
| <div className="font-bold truncate"> | ||
| {mode.name} | ||
| {renderSourceIndicator(mode, true)} |
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.
If the mode list grows significantly, you might want to consider memoizing these source indicators to avoid recalculating them on every render. Though the current performance impact is minimal.
| expect(trigger).toHaveTextContent("Old Custom Mode") | ||
| expect(trigger).toHaveTextContent("(Global)") | ||
| }) | ||
| }) |
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.
Good test coverage! Consider adding an edge case test for when the prop is undefined/null to ensure the helper functions handle this gracefully.
Related GitHub Issue
Closes: #6502
Roo Code Task Context (Optional)
No Roo Code task context for this PR
Description
This PR adds visual indicators to the ModeSelector component to distinguish between global and project-based custom modes, addressing user confusion when multiple modes share the same name.
Implementation approach:
getModeSource(): Determines if a mode is custom and returns its sourcerenderSourceIndicator(): Returns localized indicator JSX based on mode sourcetext-vscode-descriptionForegroundfor indicatorsKey design decisions:
customMode.sourcefield - no backend changes requiredReview Fixes Applied:
renderSourceIndicatorhelper function for better readabilityTest Procedure
Unit Testing:
cd webview-ui && npx vitest run src/components/chat/__tests__/ModeSelector.spec.tsxManual Testing Steps:
Test Environment:
Pre-Submission Checklist
Screenshots / Videos
Screenshots would be added here showing:
Documentation Updates
Additional Notes
All code quality issues identified during internal review have been addressed:
renderSourceIndicatorhelper functionGet in Touch
Discord username to be provided
Important
Adds visual indicators to distinguish global and project custom modes in
ModeSelector, with updated translations and tests.ModeSelectorfor global and project custom modes usinggetModeSource()andrenderSourceIndicator()functions.chat.jsonfiles.ModeSelector.spec.tsxfor custom modes with indicators, built-in modes without indicators, and backward compatibility.renderSourceIndicatorfunction for readability.This description was created by
for 9a7a2a7. You can customize this summary. It will automatically update as commits are pushed.