-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix: add visual indicators for custom mode sources (#6502) #6520
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
fix: add visual indicators for custom mode sources (#6502) #6520
Conversation
- Add (G)/(P) indicators in mode dropdown for custom modes - Add (Global)/(Project) text in selected mode button - Built-in modes show no indicators - Add helper functions getModeSource() and getSourceDisplayText() - Use text-vscode-descriptionForeground for subtle styling - Add comprehensive tests for source indicator functionality - Add translation keys to all 18 supported languages
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 visual indicators for custom mode sources will definitely help users distinguish between global and project-level modes. I've left some suggestions inline to improve accessibility, performance, and type safety.
| )} | ||
| data-testid="mode-selector-item"> | ||
| <div className="flex-1 min-w-0"> | ||
| <div className="font-bold truncate"> |
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 adding aria-label attributes for better accessibility. Screen reader users might not understand what "(G)" or "(P)" means. You could add:
| <div className="font-bold truncate"> | |
| <div className="font-bold truncate"> | |
| {mode.name} | |
| {modeSource && ( | |
| <span | |
| className="text-vscode-descriptionForeground font-normal" | |
| aria-label={modeSource === "global" ? "Global custom mode" : "Project custom mode"} | |
| > | |
| {getSourceDisplayText(modeSource, true)} | |
| </span> | |
| )} | |
| </div> |
| const { t } = useAppTranslation() | ||
|
|
||
| // Helper function to get mode source | ||
| const getModeSource = React.useCallback( |
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 getModeSource function is called for every mode during rendering. Since custom modes are passed as props and don't change frequently, consider memoizing the source mapping:
| const getModeSource = React.useCallback( | |
| // Helper function to get mode source | |
| const modeSourceMap = React.useMemo(() => { | |
| const map = new Map<string, "global" | "project">(); | |
| customModes?.forEach(mode => { | |
| map.set(mode.slug, mode.source || "global"); | |
| }); | |
| return map; | |
| }, [customModes]); | |
| const getModeSource = React.useCallback( | |
| (modeSlug: string): "global" | "project" | null => { | |
| return modeSourceMap.get(modeSlug) || null; | |
| }, | |
| [modeSourceMap], | |
| ) |
| const { hasOpenedModeSelector, setHasOpenedModeSelector } = useExtensionState() | ||
| const { t } = useAppTranslation() | ||
|
|
||
| // Helper function to get mode source |
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 adding JSDoc comments to explain the default behavior when source is undefined:
| // Helper function to get mode source | |
| /** | |
| * Gets the source of a custom mode (global or project). | |
| * @param modeSlug - The slug of the mode to check | |
| * @returns "global" or "project" for custom modes, null for built-in modes. | |
| * Defaults to "global" if a custom mode has no source field. | |
| */ | |
| const getModeSource = React.useCallback( |
| // Check trigger shows global source (default) | ||
| const trigger = screen.getByTestId("mode-selector-trigger") | ||
| expect(trigger.textContent).toContain("(common:customModes.scope.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.
Consider adding a test case for invalid source values to ensure graceful handling:
| }) | |
| test("handles invalid source values gracefully", () => { | |
| const customModes: ModeConfig[] = [ | |
| { | |
| slug: "custom-invalid-source", | |
| name: "Custom Invalid Source", | |
| description: "A custom mode with invalid source", | |
| roleDefinition: "Role definition", | |
| groups: ["read"], | |
| source: "invalid" as any, // Invalid source value | |
| }, | |
| ]; | |
| mockModes = [...customModes]; | |
| render( | |
| <ModeSelector | |
| value={"custom-invalid-source" as Mode} | |
| onChange={vi.fn()} | |
| modeShortcutText="Ctrl+M" | |
| customModes={customModes} | |
| />, | |
| ); | |
| // Should handle gracefully - perhaps default to no indicator or "global" | |
| const trigger = screen.getByTestId("mode-selector-trigger"); | |
| expect(trigger.textContent).toContain("Custom Invalid Source"); | |
| // Add assertion for expected behavior with invalid source | |
| }); |
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 distinguish between global and project-level custom modes in the mode selector, addressing user confusion when working with modes at both levels.
Key implementation details:
getModeSource()andgetSourceDisplayText()to identify custom modes and format their source displaytext-vscode-descriptionForegroundcolor class for subtle, non-intrusive stylingTest Procedure
Automated Testing:
ModeSelector.spec.tsxcovering all scenarioscd webview-ui && npm test -- src/components/chat/__tests__/ModeSelector.spec.tsxManual Testing:
Pre-Submission Checklist
Screenshots / Videos
UI changes implemented but screenshots not available in this environment. The changes add text indicators "(G)"/"(P)" in dropdown and "(Global)"/"(Project)" in the selector button for custom modes.
Documentation Updates
Additional Notes
Get in Touch
@MuriloFP
Important
Adds visual indicators in
ModeSelector.tsxto distinguish global and project-level custom modes, with tests and localization updates.ModeSelector.tsxto distinguish between global and project-level modes.getModeSource()andgetSourceDisplayText()to determine and display mode source.ModeSelector.spec.tsxto verify source indicators for custom modes, absence for built-in modes, and default to global when source is undefined.This description was created by
for 8e396b9. You can customize this summary. It will automatically update as commits are pushed.