-
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
Changes from all commits
efaaed9
57d3fbe
ef61905
f5a51c4
bcbf329
80413c0
ab10140
39c5cf7
00a0b63
080b61b
7a5ad14
2c73ff2
05ccf57
fdb1f35
10ce509
ab1f9fc
74fd8b4
6745c8f
faf2ee5
b2dadf9
f648e4c
a6d1e60
be90907
ed3a077
856313f
4dd68ea
b10fa5e
f016d7b
23855f2
3803c29
9a7a2a7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -46,6 +46,22 @@ export const ModeSelector = ({ | |
| const { hasOpenedModeSelector, setHasOpenedModeSelector } = useExtensionState() | ||
| const { t } = useAppTranslation() | ||
|
|
||
| // Helper to determine if a mode is custom and get its source | ||
| 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 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
| } | ||
|
|
||
| // Helper to get display text for source | ||
| const getSourceDisplayText = (source: string | null, isShort: boolean = false): string => { | ||
| if (!source) return "" | ||
| if (isShort) { | ||
| return source === "global" ? t("chat:modeSelector.globalShort") : t("chat:modeSelector.projectShort") | ||
| } | ||
| return source === "global" ? t("chat:modeSelector.global") : t("chat:modeSelector.project") | ||
| } | ||
|
|
||
| const trackModeSelectorOpened = React.useCallback(() => { | ||
| // Track telemetry every time the mode selector is opened | ||
| telemetryClient.capture(TelemetryEventName.MODE_SELECTOR_OPENED) | ||
|
|
@@ -159,6 +175,13 @@ export const ModeSelector = ({ | |
| // Combine instruction text for tooltip | ||
| const instructionText = `${t("chat:modeSelector.description")} ${modeShortcutText}` | ||
|
|
||
| // Helper function to render source indicator | ||
| 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> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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: |
||
| } | ||
|
|
||
| const trigger = ( | ||
| <PopoverTrigger | ||
| disabled={disabled} | ||
|
|
@@ -176,7 +199,10 @@ export const ModeSelector = ({ | |
| : null, | ||
| )}> | ||
| <ChevronUp className="pointer-events-none opacity-80 flex-shrink-0 size-3" /> | ||
| <span className="truncate">{selectedMode?.name || ""}</span> | ||
| <span className="truncate"> | ||
| {selectedMode?.name || ""} | ||
| {selectedMode && renderSourceIndicator(selectedMode, false)} | ||
| </span> | ||
| </PopoverTrigger> | ||
| ) | ||
|
|
||
|
|
@@ -238,7 +264,10 @@ export const ModeSelector = ({ | |
| )} | ||
| data-testid="mode-selector-item"> | ||
| <div className="flex-1 min-w-0"> | ||
| <div className="font-bold truncate">{mode.name}</div> | ||
| <div className="font-bold truncate"> | ||
| {mode.name} | ||
| {renderSourceIndicator(mode, true)} | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
| </div> | ||
| {mode.description && ( | ||
| <div className="text-xs text-vscode-descriptionForeground truncate"> | ||
| {mode.description} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,7 +21,21 @@ vi.mock("@/context/ExtensionStateContext", () => ({ | |
|
|
||
| vi.mock("@/i18n/TranslationContext", () => ({ | ||
| useAppTranslation: () => ({ | ||
| t: (key: string) => key, | ||
| t: (key: string) => { | ||
| const translations: Record<string, string> = { | ||
| "chat:modeSelector.global": "Global", | ||
| "chat:modeSelector.project": "Project", | ||
| "chat:modeSelector.globalShort": "G", | ||
| "chat:modeSelector.projectShort": "P", | ||
| "chat:modeSelector.description": "Select a mode to change how the assistant responds.", | ||
| "chat:modeSelector.searchPlaceholder": "Search modes...", | ||
| "chat:modeSelector.noResults": "No modes found", | ||
| "chat:modeSelector.marketplace": "Browse Marketplace", | ||
| "chat:modeSelector.settings": "Mode Settings", | ||
| "chat:modeSelector.title": "Modes", | ||
| } | ||
| return translations[key] || key | ||
| }, | ||
| }), | ||
| })) | ||
|
|
||
|
|
@@ -93,7 +107,7 @@ describe("ModeSelector", () => { | |
| expect(screen.getByTestId("mode-search-input")).toBeInTheDocument() | ||
|
|
||
| // Info icon should be visible | ||
| expect(screen.getByText("chat:modeSelector.title")).toBeInTheDocument() | ||
| expect(screen.getByText("Modes")).toBeInTheDocument() | ||
| const infoIcon = document.querySelector(".codicon-info") | ||
| expect(infoIcon).toBeInTheDocument() | ||
| }) | ||
|
|
@@ -117,7 +131,7 @@ describe("ModeSelector", () => { | |
| expect(screen.queryByTestId("mode-search-input")).not.toBeInTheDocument() | ||
|
|
||
| // Info blurb should be visible | ||
| expect(screen.getByText(/chat:modeSelector.description/)).toBeInTheDocument() | ||
| expect(screen.getByText(/Select a mode to change how the assistant responds./)).toBeInTheDocument() | ||
|
|
||
| // Info icon should NOT be visible | ||
| const infoIcon = document.querySelector(".codicon-info") | ||
|
|
@@ -169,7 +183,7 @@ describe("ModeSelector", () => { | |
| expect(screen.queryByTestId("mode-search-input")).not.toBeInTheDocument() | ||
|
|
||
| // Info blurb should be visible instead | ||
| expect(screen.getByText(/chat:modeSelector.description/)).toBeInTheDocument() | ||
| expect(screen.getByText(/Select a mode to change how the assistant responds./)).toBeInTheDocument() | ||
|
|
||
| // Info icon should NOT be visible | ||
| const infoIcon = document.querySelector(".codicon-info") | ||
|
|
@@ -199,4 +213,129 @@ describe("ModeSelector", () => { | |
| const infoIcon = document.querySelector(".codicon-info") | ||
| expect(infoIcon).toBeInTheDocument() | ||
| }) | ||
|
|
||
| test("shows source indicator for custom modes", () => { | ||
| const customModesWithSource: ModeConfig[] = [ | ||
| { | ||
| slug: "custom-global", | ||
| name: "Custom Global Mode", | ||
| roleDefinition: "Role", | ||
| groups: ["read"] as ModeConfig["groups"], | ||
| source: "global", | ||
| }, | ||
| { | ||
| slug: "custom-project", | ||
| name: "Custom Project Mode", | ||
| roleDefinition: "Role", | ||
| groups: ["read"] as ModeConfig["groups"], | ||
| source: "project", | ||
| }, | ||
| ] | ||
|
|
||
| // Set up mock to return custom modes | ||
| mockModes = [ | ||
| ...customModesWithSource, | ||
| { | ||
| slug: "code", | ||
| name: "Code", | ||
| roleDefinition: "Role", | ||
| groups: ["read"] as ModeConfig["groups"], | ||
| }, | ||
| ] | ||
|
|
||
| render( | ||
| <ModeSelector | ||
| value={"custom-global" as Mode} | ||
| onChange={vi.fn()} | ||
| modeShortcutText="Ctrl+M" | ||
| customModes={customModesWithSource} | ||
| />, | ||
| ) | ||
|
|
||
| // Check selected mode shows full indicator | ||
| const trigger = screen.getByTestId("mode-selector-trigger") | ||
| expect(trigger).toHaveTextContent("Custom Global Mode") | ||
| expect(trigger).toHaveTextContent("(Global)") | ||
|
|
||
| // Open dropdown | ||
| fireEvent.click(trigger) | ||
|
|
||
| // Check dropdown shows short indicators | ||
| const items = screen.getAllByTestId("mode-selector-item") | ||
| const globalItem = items.find((item) => item.textContent?.includes("Custom Global Mode")) | ||
| const projectItem = items.find((item) => item.textContent?.includes("Custom Project Mode")) | ||
|
|
||
| expect(globalItem).toHaveTextContent("(G)") | ||
| expect(projectItem).toHaveTextContent("(P)") | ||
| }) | ||
|
|
||
| test("shows no indicator for built-in modes", () => { | ||
| // Set up mock to return only built-in modes | ||
| mockModes = [ | ||
| { | ||
| slug: "code", | ||
| name: "Code", | ||
| roleDefinition: "Role", | ||
| groups: ["read"] as ModeConfig["groups"], | ||
| }, | ||
| { | ||
| slug: "architect", | ||
| name: "Architect", | ||
| roleDefinition: "Role", | ||
| groups: ["read"] as ModeConfig["groups"], | ||
| }, | ||
| ] | ||
|
|
||
| render(<ModeSelector value={"code" as Mode} onChange={vi.fn()} modeShortcutText="Ctrl+M" />) | ||
|
|
||
| const trigger = screen.getByTestId("mode-selector-trigger") | ||
| expect(trigger).toHaveTextContent("Code") | ||
| expect(trigger).not.toHaveTextContent("(") | ||
|
|
||
| // Open dropdown | ||
| fireEvent.click(trigger) | ||
|
|
||
| // Check that no items have indicators | ||
| const items = screen.getAllByTestId("mode-selector-item") | ||
| items.forEach((item) => { | ||
| expect(item).not.toHaveTextContent("(Global") | ||
| expect(item).not.toHaveTextContent("(Project") | ||
| }) | ||
| }) | ||
|
|
||
| test("defaults to global for custom modes without source", () => { | ||
| const customModesNoSource: ModeConfig[] = [ | ||
| { | ||
| slug: "custom-old", | ||
| name: "Old Custom Mode", | ||
| roleDefinition: "Role", | ||
| groups: ["read"] as ModeConfig["groups"], | ||
| // No source field | ||
| }, | ||
| ] | ||
|
|
||
| // Set up mock to include the custom mode | ||
| mockModes = [ | ||
| ...customModesNoSource, | ||
| { | ||
| slug: "code", | ||
| name: "Code", | ||
| roleDefinition: "Role", | ||
| groups: ["read"] as ModeConfig["groups"], | ||
| }, | ||
| ] | ||
|
|
||
| render( | ||
| <ModeSelector | ||
| value={"custom-old" as Mode} | ||
| onChange={vi.fn()} | ||
| modeShortcutText="Ctrl+M" | ||
| customModes={customModesNoSource} | ||
| />, | ||
| ) | ||
|
|
||
| const trigger = screen.getByTestId("mode-selector-trigger") | ||
| expect(trigger).toHaveTextContent("Old Custom Mode") | ||
| expect(trigger).toHaveTextContent("(Global)") | ||
| }) | ||
| }) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.