-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: Add mode display indicators on task cards (#6493) #6501
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
f258f49
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 |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| import React from "react" | ||
| import { getModeBySlug } from "@roo/modes" | ||
| import { Badge } from "@/components/ui/badge" | ||
| import { StandardTooltip } from "@/components/ui" | ||
| import { cn } from "@/lib/utils" | ||
| import { useExtensionState } from "@/context/ExtensionStateContext" | ||
|
|
||
| interface ModeBadgeProps { | ||
| modeSlug: string | undefined | ||
| className?: string | ||
| } | ||
|
|
||
| export const ModeBadge: React.FC<ModeBadgeProps> = ({ modeSlug, className }) => { | ||
| const { customModes } = useExtensionState() | ||
|
|
||
| if (!modeSlug) { | ||
| return null | ||
| } | ||
|
|
||
| const mode = getModeBySlug(modeSlug, customModes) | ||
|
|
||
| // If mode is not found (e.g., deleted custom mode), show the slug as fallback | ||
| const displayName = mode?.name || modeSlug | ||
|
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. The fallback to showing the raw slug might be confusing for users. Could we consider a more user-friendly fallback like "Unknown mode" or hide the badge entirely for deleted modes? |
||
|
|
||
| // Truncate long mode names | ||
| const truncatedName = displayName.length > 20 ? `${displayName.substring(0, 17)}...` : displayName | ||
|
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. The hard-coded truncation logic (20 characters, 17 + "...") might not work well for all languages or use cases. Could we consider making this configurable or using CSS for better performance and consistency? |
||
|
|
||
| return ( | ||
| <StandardTooltip content={displayName}> | ||
| <Badge | ||
| variant="outline" | ||
| className={cn( | ||
|
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. The styling overrides here conflict with the Badge variant's default styling. This could break visual consistency across the app. Is there a way to work with the existing Badge variants instead of overriding them? |
||
| "text-xs font-normal px-1.5 py-0 h-5", | ||
| "bg-vscode-badge-background text-vscode-badge-foreground", | ||
| "border-vscode-badge-background", | ||
| className, | ||
| )}> | ||
| {truncatedName} | ||
| </Badge> | ||
| </StandardTooltip> | ||
| ) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,172 @@ | ||
| import { render, screen } from "@testing-library/react" | ||
| import { ModeBadge } from "../ModeBadge" | ||
| import { getModeBySlug } from "@roo/modes" | ||
| import { useExtensionState } from "@/context/ExtensionStateContext" | ||
|
|
||
| // Mock dependencies | ||
| vi.mock("@roo/modes") | ||
| vi.mock("@/context/ExtensionStateContext") | ||
| vi.mock("@/components/ui", () => ({ | ||
|
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. Mocking the entire module could be brittle. Could we consider more targeted mocks for just the components we need to test? |
||
| StandardTooltip: ({ children, content }: any) => <div title={content}>{children}</div>, | ||
| Badge: ({ children, className }: any) => <div className={className}>{children}</div>, | ||
| })) | ||
|
|
||
| const mockGetModeBySlug = vi.mocked(getModeBySlug) | ||
| const mockUseExtensionState = vi.mocked(useExtensionState) | ||
|
|
||
| describe("ModeBadge", () => { | ||
| beforeEach(() => { | ||
| vi.clearAllMocks() | ||
| mockUseExtensionState.mockReturnValue({ | ||
| customModes: [], | ||
| } as any) | ||
| }) | ||
|
|
||
| it("should not render when modeSlug is undefined", () => { | ||
| const { container } = render(<ModeBadge modeSlug={undefined} />) | ||
| expect(container.firstChild).toBeNull() | ||
| }) | ||
|
|
||
| it("should render mode name for built-in mode", () => { | ||
| mockGetModeBySlug.mockReturnValue({ | ||
| slug: "code", | ||
| name: "💻 Code", | ||
| roleDefinition: "You are a code assistant", | ||
| groups: ["read", "edit"] as const, | ||
| } as any) | ||
|
|
||
| render(<ModeBadge modeSlug="code" />) | ||
|
|
||
| expect(screen.getByText("💻 Code")).toBeInTheDocument() | ||
| expect(mockGetModeBySlug).toHaveBeenCalledWith("code", []) | ||
| }) | ||
|
|
||
| it("should render mode name for custom mode", () => { | ||
| const customModes = [ | ||
| { | ||
| slug: "custom-mode", | ||
| name: "🎨 Custom Mode", | ||
| roleDefinition: "Custom role", | ||
| groups: ["read"] as const, | ||
| }, | ||
| ] | ||
|
|
||
| mockUseExtensionState.mockReturnValue({ | ||
| customModes, | ||
| } as any) | ||
|
|
||
| mockGetModeBySlug.mockReturnValue(customModes[0] as any) | ||
|
|
||
| render(<ModeBadge modeSlug="custom-mode" />) | ||
|
|
||
| expect(screen.getByText("🎨 Custom Mode")).toBeInTheDocument() | ||
| expect(mockGetModeBySlug).toHaveBeenCalledWith("custom-mode", customModes) | ||
| }) | ||
|
|
||
| it("should render mode slug as fallback for deleted custom mode", () => { | ||
| mockGetModeBySlug.mockReturnValue(undefined) | ||
|
|
||
| render(<ModeBadge modeSlug="deleted-mode" />) | ||
|
|
||
| expect(screen.getByText("deleted-mode")).toBeInTheDocument() | ||
| }) | ||
|
|
||
| it("should truncate long mode names", () => { | ||
| mockGetModeBySlug.mockReturnValue({ | ||
| slug: "long-mode", | ||
| name: "This is a very long mode name that should be truncated", | ||
| roleDefinition: "Long mode", | ||
| groups: ["read"] as const, | ||
| } as any) | ||
|
|
||
| render(<ModeBadge modeSlug="long-mode" />) | ||
|
|
||
| expect(screen.getByText("This is a very lo...")).toBeInTheDocument() | ||
| }) | ||
|
|
||
| it("should show full name in tooltip", () => { | ||
| const longName = "This is a very long mode name that should be truncated" | ||
| mockGetModeBySlug.mockReturnValue({ | ||
| slug: "long-mode", | ||
| name: longName, | ||
| roleDefinition: "Long mode", | ||
| groups: ["read"] as const, | ||
| } as any) | ||
|
|
||
| render(<ModeBadge modeSlug="long-mode" />) | ||
|
|
||
| // The StandardTooltip component should have the full name as content | ||
| const badge = screen.getByText("This is a very lo...") | ||
| expect(badge.closest("[title]")).toHaveAttribute("title", longName) | ||
| }) | ||
|
|
||
| it("should apply custom className", () => { | ||
| mockGetModeBySlug.mockReturnValue({ | ||
| slug: "code", | ||
| name: "Code", | ||
| roleDefinition: "Code mode", | ||
| groups: ["read"] as const, | ||
| } as any) | ||
|
|
||
| render(<ModeBadge modeSlug="code" className="custom-class" />) | ||
|
|
||
| const badge = screen.getByText("Code") | ||
| expect(badge).toHaveClass("custom-class") | ||
| }) | ||
|
|
||
| it("should handle mode without emoji", () => { | ||
| mockGetModeBySlug.mockReturnValue({ | ||
| slug: "plain-mode", | ||
| name: "Plain Mode", | ||
| roleDefinition: "Plain mode", | ||
| groups: ["read"] as const, | ||
| } as any) | ||
|
|
||
| render(<ModeBadge modeSlug="plain-mode" />) | ||
|
|
||
| expect(screen.getByText("Plain Mode")).toBeInTheDocument() | ||
| }) | ||
|
|
||
| it("should use correct styling classes", () => { | ||
| mockGetModeBySlug.mockReturnValue({ | ||
| slug: "test-mode", | ||
| name: "Test Mode", | ||
| roleDefinition: "Test mode", | ||
| groups: ["read"] as const, | ||
| } as any) | ||
|
|
||
| render(<ModeBadge modeSlug="test-mode" />) | ||
|
|
||
| const badge = screen.getByText("Test Mode") | ||
| expect(badge).toHaveClass("bg-vscode-badge-background") | ||
| expect(badge).toHaveClass("text-vscode-badge-foreground") | ||
| expect(badge).toHaveClass("border-vscode-badge-background") | ||
| expect(badge).toHaveClass("text-xs") | ||
| expect(badge).toHaveClass("font-normal") | ||
| expect(badge).toHaveClass("px-1.5") | ||
| expect(badge).toHaveClass("py-0") | ||
| expect(badge).toHaveClass("h-5") | ||
| }) | ||
|
|
||
| it("should handle all built-in modes", () => { | ||
| const builtInModes = [ | ||
| { slug: "architect", name: "🏗️ Architect" }, | ||
| { slug: "code", name: "💻 Code" }, | ||
| { slug: "ask", name: "❓ Ask" }, | ||
| { slug: "debug", name: "🪲 Debug" }, | ||
| { slug: "test", name: "🧪 Test" }, | ||
| ] | ||
|
|
||
| builtInModes.forEach((mode) => { | ||
| mockGetModeBySlug.mockReturnValue({ | ||
| ...mode, | ||
| roleDefinition: "Test", | ||
| groups: ["read"] as const, | ||
| } as any) | ||
|
|
||
| const { unmount } = render(<ModeBadge modeSlug={mode.slug} />) | ||
| expect(screen.getByText(mode.name)).toBeInTheDocument() | ||
| unmount() | ||
| }) | ||
| }) | ||
| }) | ||
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.
Import path inconsistency detected. This uses while the rest of the codebase uses or patterns. Could we update this to match the project's import conventions?