Skip to content

Commit 7d7f8ac

Browse files
committed
fix(ui): addresses PR review findings for tracked items feature
1 parent 8812da6 commit 7d7f8ac

File tree

14 files changed

+330
-88
lines changed

14 files changed

+330
-88
lines changed

e2e/smoke.spec.ts

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,9 @@ test("unknown path redirects to dashboard when authenticated", async ({ page })
192192
// ── Tracked items ─────────────────────────────────────────────────────────────
193193

194194
test("tracked items tab appears when enabled", async ({ page }) => {
195-
// Override the GraphQL mock to return an issue
195+
// setupAuth FIRST — registers catch-all and default GraphQL handler
196+
await setupAuth(page, { enableTracking: true });
197+
// Override GraphQL AFTER setupAuth — Playwright matches routes LIFO, so this wins
196198
await page.route("https://api.github.com/graphql", (route) =>
197199
route.fulfill({
198200
status: 200,
@@ -224,11 +226,27 @@ test("tracked items tab appears when enabled", async ({ page }) => {
224226
},
225227
})
226228
);
227-
await setupAuth(page, { enableTracking: true });
228229
await page.goto("/dashboard");
229230

230231
// Verify Tracked tab is visible
231232
await expect(page.getByRole("tab", { name: /tracked/i })).toBeVisible();
233+
234+
// Wait for issue data to render, then expand the repo group
235+
await page.getByText("testorg/testrepo").click();
236+
await expect(page.getByText("Test tracked issue")).toBeVisible();
237+
238+
// Hover the row's group container to reveal the pin button (opacity-0 → group-hover:opacity-100)
239+
const row = page.locator(".group").filter({ hasText: "Test tracked issue" });
240+
await row.hover();
241+
const pinBtn = page.getByRole("button", { name: /^Pin #42/i });
242+
await expect(pinBtn).toBeVisible();
243+
await pinBtn.click();
244+
245+
// Switch to Tracked tab
246+
await page.getByRole("tab", { name: /tracked/i }).click();
247+
248+
// Verify the pinned item appears in the Tracked tab
249+
await expect(page.getByText("Test tracked issue")).toBeVisible();
232250
});
233251

234252
test("tracked items tab hidden when disabled", async ({ page }) => {

src/app/components/dashboard/DashboardPage.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -259,8 +259,8 @@ export default function DashboardPage() {
259259

260260
// Auto-prune tracked items that are closed/merged (absent from is:open results)
261261
createEffect(() => {
262-
// IMPORTANT: Access reactive store fields BEFORE non-reactive guards
263-
// so SolidJS registers them as dependencies
262+
// IMPORTANT: Access reactive store fields BEFORE early-return guards
263+
// so SolidJS registers them as dependencies even when the guard short-circuits
264264
const issues = dashboardData.issues;
265265
const prs = dashboardData.pullRequests;
266266
if (!config.enableTracking || viewState.trackedItems.length === 0 || !hasFetchedFresh()) return;

src/app/components/dashboard/IssuesTab.tsx

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,14 @@ export default function IssuesTab(props: IssuesTabProps) {
251251
if (config.enableTracking) untrackItem(issue.id, "issue");
252252
}
253253

254+
function handleTrack(issue: Issue) {
255+
if (trackedIssueIds().has(issue.id)) {
256+
untrackItem(issue.id, "issue");
257+
} else {
258+
trackItem({ id: issue.id, number: issue.number, type: "issue", repoFullName: issue.repoFullName, title: issue.title, addedAt: Date.now() });
259+
}
260+
}
261+
254262
return (
255263
<div class="flex flex-col h-full">
256264
{/* Sort dropdown + filter chips + ignore badge toolbar */}
@@ -415,13 +423,7 @@ export default function IssuesTab(props: IssuesTabProps) {
415423
url={issue.htmlUrl}
416424
labels={issue.labels}
417425
onIgnore={() => handleIgnore(issue)}
418-
onTrack={config.enableTracking ? () => {
419-
if (trackedIssueIds().has(issue.id)) {
420-
untrackItem(issue.id, "issue");
421-
} else {
422-
trackItem({ id: issue.id, type: "issue", repoFullName: issue.repoFullName, title: issue.title, addedAt: Date.now() });
423-
}
424-
} : undefined}
426+
onTrack={config.enableTracking ? () => handleTrack(issue) : undefined}
425427
isTracked={config.enableTracking ? trackedIssueIds().has(issue.id) : undefined}
426428
density={config.viewDensity}
427429
commentCount={issue.comments}

src/app/components/dashboard/ItemRow.tsx

Lines changed: 36 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { isSafeGitHubUrl } from "../../lib/url";
33
import { relativeTime, shortRelativeTime, formatCount } from "../../lib/format";
44
import { expandEmoji } from "../../lib/emoji";
55
import { labelColorClass } from "../../lib/label-colors";
6+
import { Tooltip } from "../shared/Tooltip";
67

78
export interface ItemRowProps {
89
repo: string;
@@ -172,43 +173,44 @@ export default function ItemRow(props: ItemRowProps) {
172173

173174
{/* Pin button — visible on hover, always visible when tracked */}
174175
<Show when={props.onTrack !== undefined}>
175-
<button
176-
onClick={() => props.onTrack!()}
177-
class={`relative z-10 shrink-0 self-center rounded p-1
178-
transition-opacity focus:outline-none focus:ring-2 focus:ring-primary
179-
${props.isTracked
180-
? "text-primary opacity-100"
181-
: "text-base-content/30 hover:text-primary opacity-0 group-hover:opacity-100 focus:opacity-100"
182-
}`}
183-
title={props.isTracked ? "Untrack this item" : "Track this item"}
184-
aria-label={props.isTracked ? `Unpin #${props.number} ${props.title}` : `Pin #${props.number} ${props.title}`}
185-
>
186-
<Show
187-
when={props.isTracked}
188-
fallback={
189-
/* Outline bookmark (not tracked) */
190-
<svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" stroke-width="1.5" stroke="currentColor" class="h-4 w-4"><path stroke-linecap="round" stroke-linejoin="round" d="M17.593 3.322c1.1.128 1.907 1.077 1.907 2.185V21L12 17.25 4.5 21V5.507c0-1.108.806-2.057 1.907-2.185a48.507 48.507 0 0 1 11.186 0Z" /></svg>
191-
}
176+
<Tooltip content={props.isTracked ? "Untrack" : "Track"} class="relative z-10 shrink-0 self-center">
177+
<button
178+
onClick={() => props.onTrack!()}
179+
class={`rounded p-1
180+
transition-opacity focus:outline-none focus:ring-2 focus:ring-primary
181+
${props.isTracked
182+
? "text-primary opacity-100"
183+
: "text-base-content/30 hover:text-primary opacity-0 group-hover:opacity-100 focus:opacity-100"
184+
}`}
185+
aria-label={props.isTracked ? `Unpin #${props.number} ${props.title}` : `Pin #${props.number} ${props.title}`}
192186
>
193-
{/* Solid bookmark (tracked) */}
194-
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" fill="currentColor" class="h-4 w-4"><path fill-rule="evenodd" d="M6.32 2.577a49.255 49.255 0 0 1 11.36 0c1.497.174 2.57 1.46 2.57 2.93V21a.75.75 0 0 1-1.085.67L12 18.089l-7.165 3.583A.75.75 0 0 1 3.75 21V5.507c0-1.47 1.073-2.756 2.57-2.93Z" clip-rule="evenodd" /></svg>
195-
</Show>
196-
</button>
187+
<Show
188+
when={props.isTracked}
189+
fallback={
190+
/* Outline bookmark (not tracked) */
191+
<svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" stroke-width="1.5" stroke="currentColor" class="h-4 w-4"><path stroke-linecap="round" stroke-linejoin="round" d="M17.593 3.322c1.1.128 1.907 1.077 1.907 2.185V21L12 17.25 4.5 21V5.507c0-1.108.806-2.057 1.907-2.185a48.507 48.507 0 0 1 11.186 0Z" /></svg>
192+
}
193+
>
194+
{/* Solid bookmark (tracked) */}
195+
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" fill="currentColor" class="h-4 w-4"><path fill-rule="evenodd" d="M6.32 2.577a49.255 49.255 0 0 1 11.36 0c1.497.174 2.57 1.46 2.57 2.93V21a.75.75 0 0 1-1.085.67L12 18.089l-7.165 3.583A.75.75 0 0 1 3.75 21V5.507c0-1.47 1.073-2.756 2.57-2.93Z" clip-rule="evenodd" /></svg>
196+
</Show>
197+
</button>
198+
</Tooltip>
197199
</Show>
198200

199201
{/* Ignore button — visible on hover */}
200202
<Show when={props.onIgnore !== undefined}>
201-
<button
202-
data-ignore-btn
203-
onClick={() => props.onIgnore!()}
204-
class={`relative z-10 shrink-0 self-center rounded p-1
205-
text-base-content/30
206-
hover:text-error
207-
opacity-0 group-hover:opacity-100 focus:opacity-100
208-
transition-opacity focus:outline-none focus:ring-2 focus:ring-error`}
209-
title="Ignore this item"
210-
aria-label={`Ignore #${props.number} ${props.title}`}
211-
>
203+
<Tooltip content="Ignore" class="relative z-10 shrink-0 self-center">
204+
<button
205+
data-ignore-btn
206+
onClick={() => props.onIgnore!()}
207+
class={`rounded p-1
208+
text-base-content/30
209+
hover:text-error
210+
opacity-0 group-hover:opacity-100 focus:opacity-100
211+
transition-opacity focus:outline-none focus:ring-2 focus:ring-error`}
212+
aria-label={`Ignore #${props.number} ${props.title}`}
213+
>
212214
{/* Eye-slash icon */}
213215
<svg
214216
xmlns="http://www.w3.org/2000/svg"
@@ -224,7 +226,8 @@ export default function ItemRow(props: ItemRowProps) {
224226
/>
225227
<path d="M12.454 16.697L9.75 13.992a4 4 0 01-3.742-3.741L2.335 6.578A9.98 9.98 0 00.458 10c1.274 4.057 5.065 7 9.542 7 .847 0 1.669-.105 2.454-.303z" />
226228
</svg>
227-
</button>
229+
</button>
230+
</Tooltip>
228231
</Show>
229232
</div>
230233
);

src/app/components/dashboard/PullRequestsTab.tsx

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -359,6 +359,14 @@ export default function PullRequestsTab(props: PullRequestsTabProps) {
359359
if (config.enableTracking) untrackItem(pr.id, "pullRequest");
360360
}
361361

362+
function handleTrack(pr: PullRequest) {
363+
if (trackedPrIds().has(pr.id)) {
364+
untrackItem(pr.id, "pullRequest");
365+
} else {
366+
trackItem({ id: pr.id, number: pr.number, type: "pullRequest", repoFullName: pr.repoFullName, title: pr.title, addedAt: Date.now() });
367+
}
368+
}
369+
362370
return (
363371
<div class="flex flex-col h-full">
364372
{/* Filter toolbar with SortDropdown */}
@@ -577,13 +585,7 @@ export default function PullRequestsTab(props: PullRequestsTabProps) {
577585
labels={pr.labels}
578586
commentCount={pr.enriched !== false ? pr.comments + pr.reviewThreads : undefined}
579587
onIgnore={() => handleIgnore(pr)}
580-
onTrack={config.enableTracking ? () => {
581-
if (trackedPrIds().has(pr.id)) {
582-
untrackItem(pr.id, "pullRequest");
583-
} else {
584-
trackItem({ id: pr.id, type: "pullRequest", repoFullName: pr.repoFullName, title: pr.title, addedAt: Date.now() });
585-
}
586-
} : undefined}
588+
onTrack={config.enableTracking ? () => handleTrack(pr) : undefined}
587589
isTracked={config.enableTracking ? trackedPrIds().has(pr.id) : undefined}
588590
density={config.viewDensity}
589591
surfacedByBadge={

src/app/components/dashboard/TrackedTab.tsx

Lines changed: 30 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1-
import { For, Show, createMemo } from "solid-js";
1+
import { For, Show, Switch, Match, createMemo } from "solid-js";
22
import { config } from "../../stores/config";
33
import { viewState, ignoreItem, untrackItem, moveTrackedItem } from "../../stores/view";
44
import type { Issue, PullRequest } from "../../services/api";
55
import ItemRow from "./ItemRow";
6+
import { Tooltip } from "../shared/Tooltip";
67

78
export interface TrackedTabProps {
89
issues: Issue[];
@@ -23,28 +24,26 @@ export default function TrackedTab(props: TrackedTabProps) {
2324
return { issueMap, prMap };
2425
});
2526

26-
const trackedItems = () => viewState.trackedItems;
27-
2827
return (
2928
<div class="flex flex-col h-full">
3029
<Show
31-
when={trackedItems().length > 0}
30+
when={viewState.trackedItems.length > 0}
3231
fallback={
3332
<div class="flex items-center justify-center py-16 text-center text-base-content/50 text-sm px-4">
3433
No tracked items. Pin issues or PRs from the Issues and Pull Requests tabs.
3534
</div>
3635
}
3736
>
3837
<div class="divide-y divide-base-300">
39-
<For each={trackedItems()}>
38+
<For each={viewState.trackedItems}>
4039
{(item, index) => {
4140
const liveData = () =>
4241
item.type === "issue"
4342
? maps().issueMap.get(item.id)
4443
: maps().prMap.get(item.id);
4544

4645
const isFirst = () => index() === 0;
47-
const isLast = () => index() === trackedItems().length - 1;
46+
const isLast = () => index() === viewState.trackedItems.length - 1;
4847

4948
return (
5049
<div class="flex items-center gap-1">
@@ -80,27 +79,30 @@ export default function TrackedTab(props: TrackedTabProps) {
8079
<span class="font-medium text-sm text-base-content truncate">
8180
{item.title}
8281
</span>
83-
<Show when={item.type === "issue"}>
84-
<span class="badge badge-outline badge-sm badge-info">Issue</span>
85-
</Show>
86-
<Show when={item.type === "pullRequest"}>
87-
<span class="badge badge-outline badge-sm badge-success">PR</span>
88-
</Show>
82+
<Switch>
83+
<Match when={item.type === "issue"}>
84+
<span class="badge badge-outline badge-sm badge-info">Issue</span>
85+
</Match>
86+
<Match when={item.type === "pullRequest"}>
87+
<span class="badge badge-outline badge-sm badge-success">PR</span>
88+
</Match>
89+
</Switch>
8990
</div>
9091
<div class="text-xs text-base-content/60 mt-0.5">
9192
{item.repoFullName}{" "}
9293
<span class="text-base-content/40">(not in current data)</span>
9394
</div>
9495
</div>
95-
<button
96-
class="relative z-10 shrink-0 self-center rounded p-1 text-primary transition-opacity focus:outline-none focus:ring-2 focus:ring-primary"
97-
aria-label={`Unpin ${item.title}`}
98-
title="Untrack this item"
99-
onClick={() => untrackItem(item.id, item.type)}
100-
>
96+
<Tooltip content="Untrack this item">
97+
<button
98+
class="relative z-10 shrink-0 self-center rounded p-1 text-primary transition-opacity focus:outline-none focus:ring-2 focus:ring-primary"
99+
aria-label={`Unpin #${item.number} ${item.title}`}
100+
onClick={() => untrackItem(item.id, item.type)}
101+
>
101102
{/* Solid bookmark */}
102-
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" fill="currentColor" class="h-4 w-4"><path fill-rule="evenodd" d="M6.32 2.577a49.255 49.255 0 0 1 11.36 0c1.497.174 2.57 1.46 2.57 2.93V21a.75.75 0 0 1-1.085.67L12 18.089l-7.165 3.583A.75.75 0 0 1 3.75 21V5.507c0-1.47 1.073-2.756 2.57-2.93Z" clip-rule="evenodd" /></svg>
103-
</button>
103+
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" fill="currentColor" class="h-4 w-4"><path fill-rule="evenodd" d="M6.32 2.577a49.255 49.255 0 0 1 11.36 0c1.497.174 2.57 1.46 2.57 2.93V21a.75.75 0 0 1-1.085.67L12 18.089l-7.165 3.583A.75.75 0 0 1 3.75 21V5.507c0-1.47 1.073-2.756 2.57-2.93Z" clip-rule="evenodd" /></svg>
104+
</button>
105+
</Tooltip>
104106
</div>
105107
}
106108
>
@@ -130,12 +132,14 @@ export default function TrackedTab(props: TrackedTabProps) {
130132
}}
131133
density={config.viewDensity}
132134
>
133-
<Show when={item.type === "issue"}>
134-
<span class="badge badge-outline badge-sm badge-info">Issue</span>
135-
</Show>
136-
<Show when={item.type === "pullRequest"}>
137-
<span class="badge badge-outline badge-sm badge-success">PR</span>
138-
</Show>
135+
<Switch>
136+
<Match when={item.type === "issue"}>
137+
<span class="badge badge-outline badge-sm badge-info">Issue</span>
138+
</Match>
139+
<Match when={item.type === "pullRequest"}>
140+
<span class="badge badge-outline badge-sm badge-success">PR</span>
141+
</Match>
142+
</Switch>
139143
</ItemRow>
140144
)}
141145
</Show>
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
import { createMemo, createSignal, onCleanup } from "solid-js";
2+
import { Tooltip as KobalteTooltip } from "@kobalte/core/tooltip";
3+
import type { JSX } from "solid-js";
4+
5+
// SECURITY: tooltip content must be JSX children, never raw HTML
6+
7+
const TOOLTIP_CONTENT_CLASS = "z-50 max-w-xs rounded bg-neutral px-2 py-1 text-xs text-neutral-content shadow-lg";
8+
9+
interface TooltipProps {
10+
content: string;
11+
placement?: "top" | "bottom" | "left" | "right";
12+
focusable?: boolean;
13+
class?: string;
14+
children: JSX.Element;
15+
}
16+
17+
export function Tooltip(props: TooltipProps) {
18+
const [isHovered, setIsHovered] = createSignal(false);
19+
const [isFocused, setIsFocused] = createSignal(false);
20+
const open = createMemo(() => isHovered() || isFocused());
21+
22+
// openDelay is ignored in controlled mode; implement the delay manually
23+
let hoverTimer: ReturnType<typeof setTimeout> | undefined;
24+
let closeTimer: ReturnType<typeof setTimeout> | undefined;
25+
onCleanup(() => {
26+
clearTimeout(hoverTimer);
27+
clearTimeout(closeTimer);
28+
});
29+
30+
return (
31+
<KobalteTooltip
32+
open={open()}
33+
onOpenChange={(isOpen) => {
34+
if (!isOpen) {
35+
clearTimeout(hoverTimer);
36+
clearTimeout(closeTimer);
37+
setIsHovered(false);
38+
setIsFocused(false);
39+
}
40+
}}
41+
placement={props.placement ?? "top"}
42+
gutter={4}
43+
>
44+
<KobalteTooltip.Trigger
45+
as="span"
46+
class={`inline-flex items-center${props.class ? ` ${props.class}` : ""}`}
47+
tabindex={props.focusable ? "0" : undefined}
48+
onPointerEnter={() => {
49+
clearTimeout(hoverTimer);
50+
clearTimeout(closeTimer);
51+
hoverTimer = setTimeout(() => setIsHovered(true), 300);
52+
}}
53+
onPointerLeave={() => {
54+
clearTimeout(hoverTimer);
55+
closeTimer = setTimeout(() => setIsHovered(false), 100);
56+
}}
57+
onFocusIn={() => setIsFocused(true)}
58+
onFocusOut={() => setIsFocused(false)}
59+
>
60+
{props.children}
61+
</KobalteTooltip.Trigger>
62+
<KobalteTooltip.Portal>
63+
<KobalteTooltip.Content class={TOOLTIP_CONTENT_CLASS}>
64+
<KobalteTooltip.Arrow />
65+
{props.content}
66+
</KobalteTooltip.Content>
67+
</KobalteTooltip.Portal>
68+
</KobalteTooltip>
69+
);
70+
}

src/app/stores/view.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ const TRACKED_ITEMS_CAP = 200;
99

1010
export const TrackedItemSchema = z.object({
1111
id: z.number(),
12+
number: z.number(),
1213
type: z.enum(["issue", "pullRequest"]),
1314
repoFullName: z.string(),
1415
title: z.string(),

0 commit comments

Comments
 (0)