Skip to content

Commit fc64fe6

Browse files
Marek DanoMarek Dano
authored andcommitted
fix(ui-rewrite): prevent prompt group key collision and add shared CardTag
Signed-off-by: Marek Dano <Marek.Dano@ibm.com>
1 parent d5d4f17 commit fc64fe6

7 files changed

Lines changed: 144 additions & 47 deletions

File tree

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
import { describe, it, expect } from "vitest";
2+
import { render, screen } from "@testing-library/react";
3+
import { CardTag } from "./card-tag";
4+
5+
describe("CardTag", () => {
6+
it("renders a plain chip with no tooltip trigger by default", () => {
7+
render(<CardTag>label</CardTag>);
8+
9+
const tag = screen.getByText("label");
10+
expect(tag).toBeInTheDocument();
11+
// No tooltip => not wrapped in a focusable trigger.
12+
expect(tag.closest("button")).toBeNull();
13+
});
14+
15+
it("applies the neutral variant classes", () => {
16+
render(<CardTag variant="neutral">mime</CardTag>);
17+
18+
expect(screen.getByText("mime")).toHaveClass("bg-neutral-100");
19+
});
20+
21+
it("exposes an accessible tooltip when tooltip text is provided", async () => {
22+
render(<CardTag tooltip="More info">chip</CardTag>);
23+
24+
const trigger = screen.getByText("chip").closest("button");
25+
expect(trigger).not.toBeNull();
26+
27+
// Radix opens the tooltip on focus, giving keyboard users the hint.
28+
trigger!.focus();
29+
expect(await screen.findByRole("tooltip")).toHaveTextContent("More info");
30+
});
31+
});
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
import * as React from "react";
2+
import { cva, type VariantProps } from "class-variance-authority";
3+
4+
import { cn } from "@/lib/utils";
5+
import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from "@/components/ui/tooltip";
6+
7+
const cardTagVariants = cva(
8+
"inline-flex items-center rounded px-1.5 py-1 text-[10px] font-medium leading-none",
9+
{
10+
variants: {
11+
variant: {
12+
default: "bg-tool-badge-bg text-white",
13+
neutral: "bg-neutral-100 text-neutral-700 dark:bg-neutral-800 dark:text-white",
14+
},
15+
},
16+
defaultVariants: {
17+
variant: "default",
18+
},
19+
},
20+
);
21+
22+
export interface CardTagProps
23+
extends React.HTMLAttributes<HTMLSpanElement>, VariantProps<typeof cardTagVariants> {
24+
/** Optional text shown in an accessible tooltip on hover/focus. */
25+
tooltip?: string | null;
26+
}
27+
28+
/**
29+
* A compact tag chip used inside the tool/resource/prompt cards. Matches the
30+
* Figma card-tag style (a small squared chip, not the pill-shaped `Badge`).
31+
* When `tooltip` is provided the chip becomes an accessible tooltip trigger, so
32+
* the hint works for keyboard and touch users instead of relying on native
33+
* `title`.
34+
*/
35+
export function CardTag({ className, variant, tooltip, children, ...props }: CardTagProps) {
36+
const tag = (
37+
<span className={cn(cardTagVariants({ variant }), className)} {...props}>
38+
{children}
39+
</span>
40+
);
41+
42+
if (!tooltip) {
43+
return tag;
44+
}
45+
46+
return (
47+
<TooltipProvider>
48+
<Tooltip>
49+
<TooltipTrigger className="rounded focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring">
50+
{tag}
51+
</TooltipTrigger>
52+
<TooltipContent>{tooltip}</TooltipContent>
53+
</Tooltip>
54+
</TooltipProvider>
55+
);
56+
}
57+
58+
export { cardTagVariants };

client/src/pages/Prompts.test.tsx

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -169,12 +169,15 @@ describe("Prompts", () => {
169169
expect(within(card).queryByText("Prompt 8")).not.toBeInTheDocument();
170170
expect(within(card).getByText("+2")).toBeInTheDocument();
171171

172-
// A real description becomes the badge tooltip; filtered "None" descriptions do not.
173-
expect(within(card).getByText("Prompt 0")).toHaveAttribute(
174-
"title",
175-
"First prompt description.",
176-
);
177-
expect(within(card).getByText("Prompt 1")).not.toHaveAttribute("title");
172+
// A real description becomes an accessible tooltip; filtered "None" descriptions do not.
173+
const describedTrigger = within(card).getByText("Prompt 0").closest("button");
174+
expect(describedTrigger).not.toBeNull();
175+
// Radix opens the tooltip on focus, giving keyboard users the description.
176+
describedTrigger?.focus();
177+
expect(await screen.findByRole("tooltip")).toHaveTextContent("First prompt description.");
178+
179+
// "Prompt 1" has description "None", so it renders as a plain tag with no tooltip trigger.
180+
expect(within(card).getByText("Prompt 1").closest("button")).toBeNull();
178181
});
179182

180183
it("renders error state when prompts fail to load", async () => {

client/src/pages/Prompts.tsx

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { useMemo } from "react";
22
import { MessageSquareCode, MoreHorizontal, Plus } from "lucide-react";
33
import { useIntl } from "react-intl";
44
import { Card, CardContent, CardHeader } from "@/components/ui/card";
5+
import { CardTag } from "@/components/ui/card-tag";
56
import { Button } from "@/components/ui/button";
67
import { useQuery } from "@/hooks/useQuery";
78
import {
@@ -39,16 +40,20 @@ function buildPromptGroups(prompts: Prompt[], restPromptsLabel: string): PromptG
3940
const map = new Map<string, PromptGroup>();
4041

4142
for (const prompt of prompts) {
42-
const slug = prompt.gatewaySlug?.trim() || restPromptsLabel;
43-
let group = map.get(slug);
43+
const slug = prompt.gatewaySlug?.trim();
44+
// Namespace the map key so gateway-less prompts (keyed "rest") can never
45+
// merge into a real gateway whose slug happens to equal the localized
46+
// "REST prompts" label. `label` stays purely for display.
47+
const key = slug ? `gateway:${slug}` : "rest";
48+
let group = map.get(key);
4449
if (!group) {
4550
group = {
46-
key: slug,
47-
label: slug,
51+
key,
52+
label: slug || restPromptsLabel,
4853
gatewayId: prompt.gatewayId,
4954
prompts: [],
5055
};
51-
map.set(slug, group);
56+
map.set(key, group);
5257
}
5358
group.prompts.push(prompt);
5459
}
@@ -102,24 +107,19 @@ function PromptGroupCard({ group }: { group: PromptGroup }) {
102107
<CardContent>
103108
<div className="flex flex-wrap gap-1">
104109
{visiblePrompts.map((prompt) => (
105-
<span
106-
key={prompt.id}
107-
className="inline-flex items-center rounded bg-tool-badge-bg px-1.5 py-1 text-[10px] font-medium leading-none text-white"
108-
title={getPromptDescription(prompt) ?? undefined}
109-
>
110+
<CardTag key={prompt.id} tooltip={getPromptDescription(prompt)}>
110111
{getPromptLabel(prompt)}
111-
</span>
112+
</CardTag>
112113
))}
113114
{remainingCount > 0 && (
114-
<span
115-
className="inline-flex items-center rounded bg-tool-badge-bg px-1.5 py-1 text-[10px] font-medium leading-none text-white"
116-
title={intl.formatMessage(
115+
<CardTag
116+
tooltip={intl.formatMessage(
117117
{ id: "prompts.card.morePromptsTitle" },
118118
{ count: remainingCount },
119119
)}
120120
>
121121
+{remainingCount}
122-
</span>
122+
</CardTag>
123123
)}
124124
</div>
125125
</CardContent>

client/src/pages/Resources.tsx

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import { ResourceReadVisibility } from "@/generated/types";
1616
import type { ResourceGroup } from "@/types/resource";
1717
import { Button } from "@/components/ui/button";
1818
import { Card, CardContent, CardHeader } from "@/components/ui/card";
19+
import { CardTag } from "@/components/ui/card-tag";
1920
import {
2021
DropdownMenu,
2122
DropdownMenuContent,
@@ -139,24 +140,24 @@ const ResourceGroupCard = memo(function ResourceGroupCard({
139140
<CardContent>
140141
<div className="flex flex-wrap gap-1">
141142
{visibleResources.map((resource) => (
142-
<span
143+
<CardTag
143144
key={resource.id}
144-
className="inline-flex items-center rounded bg-tool-badge-bg px-1.5 py-1 text-[10px] font-medium leading-none text-white"
145-
title={resource.description ?? undefined}
145+
variant="neutral"
146+
tooltip={resource.description ?? undefined}
146147
>
147148
{resource.name}
148-
</span>
149+
</CardTag>
149150
))}
150151
{remainingCount > 0 && (
151-
<span
152-
className="inline-flex items-center rounded bg-tool-badge-bg px-1.5 py-1 text-[10px] font-medium leading-none text-white"
153-
title={intl.formatMessage(
152+
<CardTag
153+
variant="neutral"
154+
tooltip={intl.formatMessage(
154155
{ id: "resources.card.moreResources" },
155156
{ count: remainingCount },
156157
)}
157158
>
158159
+{remainingCount}
159-
</span>
160+
</CardTag>
160161
)}
161162
</div>
162163
</CardContent>
@@ -490,3 +491,4 @@ export function Resources() {
490491
</div>
491492
);
492493
}
494+

client/src/pages/Tools.test.tsx

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -322,8 +322,11 @@ describe("Tools", () => {
322322
expect(screen.getByText("Tool 1")).toBeInTheDocument();
323323
});
324324

325-
const toolBadge = screen.getByText("Tool 1");
326-
expect(toolBadge).toHaveAttribute("title", "Description for tool 1");
325+
const trigger = screen.getByText("Tool 1").closest("button");
326+
expect(trigger).not.toBeNull();
327+
// Radix opens the tooltip on focus, giving keyboard users the description.
328+
trigger?.focus();
329+
expect(await screen.findByRole("tooltip")).toHaveTextContent("Description for tool 1");
327330
});
328331

329332
it("renders more options button for each tool group", async () => {
@@ -505,9 +508,11 @@ describe("Tools", () => {
505508
expect(screen.queryByText("Tool 11")).not.toBeInTheDocument();
506509
expect(screen.queryByText("Tool 12")).not.toBeInTheDocument();
507510

508-
// Should show +4 tag
509-
expect(screen.getByText("+4")).toBeInTheDocument();
510-
expect(screen.getByTitle("4 more tools")).toBeInTheDocument();
511+
// Should show +4 tag, with the remaining count exposed as an accessible tooltip
512+
const overflow = screen.getByText("+4");
513+
expect(overflow).toBeInTheDocument();
514+
overflow.closest("button")?.focus();
515+
expect(await screen.findByRole("tooltip")).toHaveTextContent("4 more tools");
511516
});
512517

513518
it("displays all tools when count is 8 or less without +N tag", async () => {
@@ -544,9 +549,11 @@ describe("Tools", () => {
544549
expect(screen.getByText("gateway-with-nine-tools")).toBeInTheDocument();
545550
});
546551

547-
// Should show +1 tag
548-
expect(screen.getByText("+1")).toBeInTheDocument();
549-
expect(screen.getByTitle("1 more tool")).toBeInTheDocument();
552+
// Should show +1 tag, with the singular "tool" wording in the tooltip
553+
const overflow = screen.getByText("+1");
554+
expect(overflow).toBeInTheDocument();
555+
overflow.closest("button")?.focus();
556+
expect(await screen.findByRole("tooltip")).toHaveTextContent("1 more tool");
550557
});
551558

552559
it("handles multiple gateways with different tool counts correctly", async () => {

client/src/pages/Tools.tsx

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { useRouter } from "@/router";
99
import { extractApiErrorDetail, sanitizeError } from "@/utils/errors";
1010
import type { Tool, ToolGroup } from "@/types/tool";
1111
import { Card, CardHeader, CardContent } from "@/components/ui/card";
12+
import { CardTag } from "@/components/ui/card-tag";
1213
import { Button } from "@/components/ui/button";
1314
import {
1415
DropdownMenu,
@@ -103,24 +104,19 @@ function ToolGroupCard({
103104
<CardContent>
104105
<div className="flex flex-wrap gap-1">
105106
{visibleTools.map((tool) => (
106-
<span
107-
key={tool.id}
108-
className="inline-flex items-center rounded bg-tool-badge-bg px-1.5 py-1 text-[10px] font-medium leading-none text-white"
109-
title={tool.description}
110-
>
107+
<CardTag key={tool.id} tooltip={tool.description}>
111108
{tool.name}
112-
</span>
109+
</CardTag>
113110
))}
114111
{remainingCount > 0 && (
115-
<span
116-
className="inline-flex items-center rounded bg-tool-badge-bg px-1.5 py-1 text-[10px] font-medium leading-none text-white"
117-
title={intl.formatMessage(
112+
<CardTag
113+
tooltip={intl.formatMessage(
118114
{ id: "tools.card.moreToolsTitle" },
119115
{ count: remainingCount },
120116
)}
121117
>
122118
+{remainingCount}
123-
</span>
119+
</CardTag>
124120
)}
125121
</div>
126122
</CardContent>

0 commit comments

Comments
 (0)