Skip to content

Commit ee48645

Browse files
wgordon17wgordon17
committed
fix(ui): addresses PR review findings for tracked items
- removes ignore action from TrackedTab (untrack only) - normalizes IgnoredItem.id from string to number (z.coerce.number) - extracts TypeBadge from duplicated Switch/Match blocks - replaces Unicode arrows with Heroicons SVG chevrons - adds FLIP reorder animation (200ms ease-in-out slide) - adds hot poll prune for closed/merged tracked PRs - flattens SettingsPage toggle handler - adds config store reset in DashboardPage test beforeEach - adds tracked tab badge count integration test - adds skipped-then-real poll auto-prune test
1 parent 4f60e87 commit ee48645

17 files changed

+210
-112
lines changed

‎src/app/components/dashboard/ActionsTab.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ export default function ActionsTab(props: ActionsTabProps) {
156156

157157
function handleIgnore(run: WorkflowRun) {
158158
ignoreItem({
159-
id: String(run.id),
159+
id: run.id,
160160
type: "workflowRun",
161161
repo: run.repoFullName,
162162
title: run.name,
@@ -174,7 +174,7 @@ export default function ActionsTab(props: ActionsTabProps) {
174174
const eventFilter = viewState.tabFilters.actions.event;
175175

176176
return props.workflowRuns.filter((run) => {
177-
if (ignoredIds.has(String(run.id))) return false;
177+
if (ignoredIds.has(run.id)) return false;
178178
if (!viewState.showPrRuns && run.isPrRun) return false;
179179
if (org && !run.repoFullName.startsWith(`${org}/`)) return false;
180180
if (repo && run.repoFullName !== repo) return false;

‎src/app/components/dashboard/DashboardPage.tsx‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,22 @@ export default function DashboardPage() {
328328
run.completedAt = update.completedAt;
329329
}
330330
}));
331+
// Prune tracked PRs that became closed/merged via hot poll.
332+
// The auto-prune createEffect only fires when the pullRequests array
333+
// reference changes (full refresh). Hot poll mutates nested pr.state
334+
// in-place via produce(), leaving the array ref unchanged.
335+
if (config.enableTracking && viewState.trackedItems.length > 0 && prUpdates.size > 0) {
336+
const pruneKeys = new Set<string>();
337+
for (const [prId, update] of prUpdates) {
338+
const stateVal = update.state?.toUpperCase();
339+
if (stateVal === "CLOSED" || stateVal === "MERGED") {
340+
if (viewState.trackedItems.some(t => t.type === "pullRequest" && t.id === prId)) {
341+
pruneKeys.add(`pullRequest:${prId}`);
342+
}
343+
}
344+
}
345+
if (pruneKeys.size > 0) pruneClosedTrackedItems(pruneKeys);
346+
}
331347
},
332348
{
333349
onStart: (prDbIds, runIds) => {
@@ -389,20 +405,20 @@ export default function DashboardPage() {
389405

390406
return {
391407
issues: dashboardData.issues.filter((i) => {
392-
if (ignoredIssues.has(String(i.id))) return false;
408+
if (ignoredIssues.has(i.id)) return false;
393409
if (viewState.hideDepDashboard && i.title === "Dependency Dashboard") return false;
394410
if (repo && i.repoFullName !== repo) return false;
395411
if (org && !i.repoFullName.startsWith(org + "/")) return false;
396412
return true;
397413
}).length,
398414
pullRequests: dashboardData.pullRequests.filter((p) => {
399-
if (ignoredPRs.has(String(p.id))) return false;
415+
if (ignoredPRs.has(p.id)) return false;
400416
if (repo && p.repoFullName !== repo) return false;
401417
if (org && !p.repoFullName.startsWith(org + "/")) return false;
402418
return true;
403419
}).length,
404420
actions: dashboardData.workflowRuns.filter((w) => {
405-
if (ignoredRuns.has(String(w.id))) return false;
421+
if (ignoredRuns.has(w.id)) return false;
406422
if (!viewState.showPrRuns && w.isPrRun) return false;
407423
if (repo && w.repoFullName !== repo) return false;
408424
if (org && !w.repoFullName.startsWith(org + "/")) return false;

‎src/app/components/dashboard/IgnoreBadge.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { Tooltip } from "../shared/Tooltip";
44

55
interface IgnoreBadgeProps {
66
items: IgnoredItem[];
7-
onUnignore: (id: string) => void;
7+
onUnignore: (id: number) => void;
88
}
99

1010
function typeIcon(type: IgnoredItem["type"]): string {

‎src/app/components/dashboard/IssuesTab.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ export default function IssuesTab(props: IssuesTabProps) {
121121
const meta = new Map<number, { roles: ReturnType<typeof deriveInvolvementRoles> }>();
122122

123123
let items = props.issues.filter((issue) => {
124-
if (ignored.has(String(issue.id))) return false;
124+
if (ignored.has(issue.id)) return false;
125125
if (filter.repo && issue.repoFullName !== filter.repo) return false;
126126
if (filter.org && !issue.repoFullName.startsWith(filter.org + "/")) return false;
127127

@@ -235,7 +235,7 @@ export default function IssuesTab(props: IssuesTabProps) {
235235

236236
function handleIgnore(issue: Issue) {
237237
ignoreItem({
238-
id: String(issue.id),
238+
id: issue.id,
239239
type: "issue",
240240
repo: issue.repoFullName,
241241
title: issue.title,

‎src/app/components/dashboard/PersonalSummaryStrip.tsx‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ interface PersonalSummaryStripProps {
2222

2323
export default function PersonalSummaryStrip(props: PersonalSummaryStripProps) {
2424
const ignoredIds = createMemo(() => {
25-
const ids = new Set<string>();
25+
const ids = new Set<number>();
2626
for (const item of viewState.ignoredItems) ids.add(item.id);
2727
return ids;
2828
});
@@ -34,7 +34,7 @@ export default function PersonalSummaryStrip(props: PersonalSummaryStripProps) {
3434
const ignored = ignoredIds();
3535
let assignedIssues = 0;
3636
for (const i of props.issues) {
37-
if (ignored.has(String(i.id))) continue;
37+
if (ignored.has(i.id)) continue;
3838
if (viewState.hideDepDashboard && i.title === "Dependency Dashboard") continue;
3939
if (i.assigneeLogins.some((a) => a.toLowerCase() === login)) assignedIssues++;
4040
}
@@ -50,7 +50,7 @@ export default function PersonalSummaryStrip(props: PersonalSummaryStripProps) {
5050
let prsReadyToMerge = 0;
5151
let prsBlocked = 0;
5252
for (const pr of props.pullRequests) {
53-
if (ignored.has(String(pr.id))) continue;
53+
if (ignored.has(pr.id)) continue;
5454
const isAuthor = pr.userLogin.toLowerCase() === login;
5555
if (
5656
!isAuthor &&
@@ -81,7 +81,7 @@ export default function PersonalSummaryStrip(props: PersonalSummaryStripProps) {
8181

8282
const runningActions = createMemo(() => {
8383
const ignored = ignoredIds();
84-
return props.workflowRuns.filter((r) => !ignored.has(String(r.id)) && r.status === "in_progress").length;
84+
return props.workflowRuns.filter((r) => !ignored.has(r.id) && r.status === "in_progress").length;
8585
});
8686

8787
const summaryItems = createMemo(() => {

‎src/app/components/dashboard/PullRequestsTab.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ export default function PullRequestsTab(props: PullRequestsTabProps) {
190190
const meta = new Map<number, { roles: ReturnType<typeof deriveInvolvementRoles>; sizeCategory: ReturnType<typeof prSizeCategory> }>();
191191

192192
let items = props.pullRequests.filter((pr) => {
193-
if (ignored.has(String(pr.id))) return false;
193+
if (ignored.has(pr.id)) return false;
194194
if (filter.repo && pr.repoFullName !== filter.repo) return false;
195195
if (filter.org && !pr.repoFullName.startsWith(filter.org + "/")) return false;
196196

@@ -341,7 +341,7 @@ export default function PullRequestsTab(props: PullRequestsTabProps) {
341341

342342
function handleIgnore(pr: PullRequest) {
343343
ignoreItem({
344-
id: String(pr.id),
344+
id: pr.id,
345345
type: "pullRequest",
346346
repo: pr.repoFullName,
347347
title: pr.title,

‎src/app/components/dashboard/TrackedTab.tsx‎

Lines changed: 69 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,60 @@
11
import { For, Show, Switch, Match, createMemo } from "solid-js";
22
import { config } from "../../stores/config";
3-
import { viewState, ignoreItem, untrackItem, moveTrackedItem } from "../../stores/view";
3+
import { viewState, untrackItem, moveTrackedItem } from "../../stores/view";
4+
import type { TrackedItem } from "../../stores/view";
45
import type { Issue, PullRequest } from "../../services/api";
56
import ItemRow from "./ItemRow";
67
import { Tooltip } from "../shared/Tooltip";
78

9+
function TypeBadge(props: { type: TrackedItem["type"] }) {
10+
return (
11+
<Switch>
12+
<Match when={props.type === "issue"}>
13+
<span class="badge badge-outline badge-sm badge-info">Issue</span>
14+
</Match>
15+
<Match when={props.type === "pullRequest"}>
16+
<span class="badge badge-outline badge-sm badge-success">PR</span>
17+
</Match>
18+
</Switch>
19+
);
20+
}
21+
22+
// FLIP animation: record positions before move, animate slide after DOM updates
23+
const itemRefs = new Map<string, HTMLDivElement>();
24+
const prefersReducedMotion = () =>
25+
typeof window !== "undefined" && window.matchMedia("(prefers-reduced-motion: reduce)").matches;
26+
27+
function recordPositions(): Map<string, DOMRect> {
28+
const snapshot = new Map<string, DOMRect>();
29+
for (const [key, el] of itemRefs) {
30+
snapshot.set(key, el.getBoundingClientRect());
31+
}
32+
return snapshot;
33+
}
34+
35+
function animateMove(before: Map<string, DOMRect>) {
36+
if (prefersReducedMotion()) return;
37+
requestAnimationFrame(() => {
38+
for (const [key, el] of itemRefs) {
39+
const old = before.get(key);
40+
if (!old) continue;
41+
const now = el.getBoundingClientRect();
42+
const dy = old.top - now.top;
43+
if (Math.abs(dy) < 1) continue;
44+
el.animate(
45+
[{ transform: `translateY(${dy}px)` }, { transform: "translateY(0)" }],
46+
{ duration: 200, easing: "ease-in-out" }
47+
);
48+
}
49+
});
50+
}
51+
52+
function handleMove(id: number, type: "issue" | "pullRequest", direction: "up" | "down") {
53+
const before = recordPositions();
54+
moveTrackedItem(id, type, direction);
55+
animateMove(before);
56+
}
57+
858
export interface TrackedTabProps {
959
issues: Issue[];
1060
pullRequests: PullRequest[];
@@ -44,26 +94,36 @@ export default function TrackedTab(props: TrackedTabProps) {
4494

4595
const isFirst = () => index() === 0;
4696
const isLast = () => index() === viewState.trackedItems.length - 1;
97+
const itemKey = `${item.type}:${item.id}`;
4798

4899
return (
49-
<div class="flex items-center gap-1">
50-
{/* Arrow buttons */}
100+
<div
101+
class="flex items-center gap-1"
102+
ref={(el) => { itemRefs.set(itemKey, el); }}
103+
>
104+
{/* Reorder buttons */}
51105
<div class="flex flex-col shrink-0 pl-2">
52106
<button
53107
class="btn btn-ghost btn-xs"
54108
disabled={isFirst()}
55109
aria-label={`Move up: ${item.title}`}
56-
onClick={() => moveTrackedItem(item.id, item.type, "up")}
110+
onClick={() => handleMove(item.id, item.type, "up")}
57111
>
58-
▲
112+
{/* Heroicons 20px solid: chevron-up */}
113+
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 20 20" fill="currentColor" class="h-3.5 w-3.5">
114+
<path fill-rule="evenodd" d="M14.77 12.79a.75.75 0 01-1.06-.02L10 8.832 6.29 12.77a.75.75 0 11-1.08-1.04l4.25-4.5a.75.75 0 011.08 0l4.25 4.5a.75.75 0 01-.02 1.06z" clip-rule="evenodd" />
115+
</svg>
59116
</button>
60117
<button
61118
class="btn btn-ghost btn-xs"
62119
disabled={isLast()}
63120
aria-label={`Move down: ${item.title}`}
64-
onClick={() => moveTrackedItem(item.id, item.type, "down")}
121+
onClick={() => handleMove(item.id, item.type, "down")}
65122
>
66-
▼
123+
{/* Heroicons 20px solid: chevron-down */}
124+
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 20 20" fill="currentColor" class="h-3.5 w-3.5">
125+
<path fill-rule="evenodd" d="M5.23 7.21a.75.75 0 011.06.02L10 11.168l3.71-3.938a.75.75 0 111.08 1.04l-4.25 4.5a.75.75 0 01-1.08 0l-4.25-4.5a.75.75 0 01.02-1.06z" clip-rule="evenodd" />
126+
</svg>
67127
</button>
68128
</div>
69129

@@ -79,14 +139,7 @@ export default function TrackedTab(props: TrackedTabProps) {
79139
<span class="font-medium text-sm text-base-content truncate">
80140
{item.title}
81141
</span>
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>
142+
<TypeBadge type={item.type} />
90143
</div>
91144
<div class="text-xs text-base-content/60 mt-0.5">
92145
{item.repoFullName}{" "}
@@ -120,26 +173,9 @@ export default function TrackedTab(props: TrackedTabProps) {
120173
labels={live().labels}
121174
onTrack={() => untrackItem(item.id, item.type)}
122175
isTracked={true}
123-
onIgnore={() => {
124-
ignoreItem({
125-
id: String(item.id),
126-
type: item.type,
127-
repo: live().repoFullName,
128-
title: live().title,
129-
ignoredAt: Date.now(),
130-
});
131-
untrackItem(item.id, item.type);
132-
}}
133176
density={config.viewDensity}
134177
>
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>
178+
<TypeBadge type={item.type} />
143179
</ItemRow>
144180
)}
145181
</Show>

‎src/app/components/settings/SettingsPage.tsx‎

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -619,17 +619,12 @@ export default function SettingsPage() {
619619
checked={config.enableTracking}
620620
onChange={(e) => {
621621
const val = e.currentTarget.checked;
622-
if (!val) {
623-
if (config.defaultTab === "tracked") {
624-
saveWithFeedback({ enableTracking: val, defaultTab: "issues" });
625-
} else {
626-
saveWithFeedback({ enableTracking: val });
627-
}
628-
if (viewState.lastActiveTab === "tracked") {
629-
updateViewState({ lastActiveTab: "issues" });
630-
}
631-
} else {
632-
saveWithFeedback({ enableTracking: val });
622+
saveWithFeedback({
623+
enableTracking: val,
624+
...(!val && config.defaultTab === "tracked" ? { defaultTab: "issues" as const } : {}),
625+
});
626+
if (!val && viewState.lastActiveTab === "tracked") {
627+
updateViewState({ lastActiveTab: "issues" });
633628
}
634629
}}
635630
class="toggle toggle-primary"

‎src/app/stores/view.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ export const ViewStateSchema = z.object({
5858
ignoredItems: z
5959
.array(
6060
z.object({
61-
id: z.string(),
61+
id: z.coerce.number(),
6262
type: z.enum(["issue", "pullRequest", "workflowRun"]),
6363
repo: z.string(),
6464
title: z.string(),
@@ -168,7 +168,7 @@ export function ignoreItem(item: IgnoredItem): void {
168168
);
169169
}
170170

171-
export function unignoreItem(id: string): void {
171+
export function unignoreItem(id: number): void {
172172
setViewState(
173173
produce((draft) => {
174174
draft.ignoredItems = draft.ignoredItems.filter((i) => i.id !== id);

‎tests/components/ActionsTab.test.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ describe("ActionsTab", () => {
162162
it("filters out ignored workflow runs", () => {
163163
const run = makeWorkflowRun({ id: 42, name: "Ignored Run", repoFullName: "owner/repo" });
164164
viewStore.ignoreItem({
165-
id: "42",
165+
id: 42,
166166
type: "workflowRun",
167167
repo: "owner/repo",
168168
title: "Ignored Run",

0 commit comments

Comments
 (0)