From 962a8827995510c63b31d04b62583c3f2d340a41 Mon Sep 17 00:00:00 2001 From: dotta Date: Mon, 6 Apr 2026 06:32:25 -0500 Subject: [PATCH] fix(ui): keep issue breadcrumb context out of the URL Co-Authored-By: Paperclip --- ui/src/components/IssueRow.test.tsx | 4 +- ui/src/components/IssueRow.tsx | 5 +- ui/src/lib/issueDetailBreadcrumb.test.ts | 60 +++++++++--- ui/src/lib/issueDetailBreadcrumb.ts | 113 +++++++++++++++++++---- ui/src/pages/Inbox.tsx | 4 +- ui/src/pages/IssueDetail.tsx | 48 ++++++++-- 6 files changed, 185 insertions(+), 49 deletions(-) diff --git a/ui/src/components/IssueRow.test.tsx b/ui/src/components/IssueRow.test.tsx index d5f67e9d..4ba0cee2 100644 --- a/ui/src/components/IssueRow.test.tsx +++ b/ui/src/components/IssueRow.test.tsx @@ -128,9 +128,7 @@ describe("IssueRow", () => { const link = container.querySelector("[data-inbox-issue-link]") as HTMLAnchorElement | null; expect(link).not.toBeNull(); - expect(link?.getAttribute("to") ?? link?.getAttribute("href")).toContain( - "/issues/PAP-1?from=inbox&fromHref=%2FPAP%2Finbox%2Fmine", - ); + expect(link?.getAttribute("to") ?? link?.getAttribute("href")).toBe("/issues/PAP-1"); act(() => { root.unmount(); diff --git a/ui/src/components/IssueRow.tsx b/ui/src/components/IssueRow.tsx index 5711a69c..09df3f03 100644 --- a/ui/src/components/IssueRow.tsx +++ b/ui/src/components/IssueRow.tsx @@ -2,7 +2,7 @@ import type { ReactNode } from "react"; import type { Issue } from "@paperclipai/shared"; import { Link } from "@/lib/router"; import { X } from "lucide-react"; -import { createIssueDetailPath } from "../lib/issueDetailBreadcrumb"; +import { createIssueDetailPath, rememberIssueDetailLocationState } from "../lib/issueDetailBreadcrumb"; import { cn } from "../lib/utils"; import { StatusIcon } from "./StatusIcon"; @@ -51,9 +51,10 @@ export function IssueRow({ return ( rememberIssueDetailLocationState(issuePathId, issueLinkState)} className={cn( "group flex items-start gap-2 border-b border-border py-2.5 pl-2 pr-3 text-sm no-underline text-inherit transition-colors last:border-b-0 sm:items-center sm:py-2 sm:pl-1", selected ? "hover:bg-transparent" : "hover:bg-accent/50", diff --git a/ui/src/lib/issueDetailBreadcrumb.test.ts b/ui/src/lib/issueDetailBreadcrumb.test.ts index cebf65bf..84e776c4 100644 --- a/ui/src/lib/issueDetailBreadcrumb.test.ts +++ b/ui/src/lib/issueDetailBreadcrumb.test.ts @@ -3,50 +3,80 @@ import { armIssueDetailInboxQuickArchive, createIssueDetailLocationState, createIssueDetailPath, + hasLegacyIssueDetailQuery, + readIssueDetailLocationState, readIssueDetailBreadcrumb, + rememberIssueDetailLocationState, shouldArmIssueDetailInboxQuickArchive, } from "./issueDetailBreadcrumb"; +const sessionStorageMock = (() => { + const store = new Map(); + return { + getItem: (key: string) => store.get(key) ?? null, + setItem: (key: string, value: string) => { + store.set(key, value); + }, + clear: () => { + store.clear(); + }, + }; +})(); + +Object.defineProperty(globalThis, "window", { + configurable: true, + value: { sessionStorage: sessionStorageMock }, +}); + describe("issueDetailBreadcrumb", () => { + it("returns clean issue detail paths", () => { + expect(createIssueDetailPath("PAP-465")).toBe("/issues/PAP-465"); + }); + it("prefers the full breadcrumb from route state", () => { const state = createIssueDetailLocationState("Inbox", "/inbox/mine", "inbox"); - expect(readIssueDetailBreadcrumb(state, "?from=issues")).toEqual({ + expect(readIssueDetailBreadcrumb("PAP-465", state, "?from=issues")).toEqual({ label: "Inbox", href: "/inbox/mine", }); }); it("falls back to the source query param when route state is unavailable", () => { - expect(readIssueDetailBreadcrumb(null, "?from=inbox")).toEqual({ + expect(readIssueDetailBreadcrumb("PAP-465", null, "?from=inbox")).toEqual({ label: "Inbox", href: "/inbox", }); }); - it("adds the source query param when building an issue detail path", () => { - const state = createIssueDetailLocationState("Inbox", "/inbox/mine", "inbox"); - - expect(createIssueDetailPath("PAP-465", state)).toBe( - "/issues/PAP-465?from=inbox&fromHref=%2Finbox%2Fmine", - ); - }); - - it("reuses the current source query param when state has been dropped", () => { - expect(createIssueDetailPath("PAP-465", null, "?from=issues&fromHref=%2Fissues%3Fq%3Dabc")).toBe( - "/issues/PAP-465?from=issues&fromHref=%2Fissues%3Fq%3Dabc", - ); + it("can detect legacy query-based breadcrumb links", () => { + expect(hasLegacyIssueDetailQuery("?from=inbox&fromHref=%2Finbox%2Fmine")).toBe(true); + expect(hasLegacyIssueDetailQuery("?q=test")).toBe(false); }); it("restores the exact breadcrumb href from the query fallback", () => { expect( - readIssueDetailBreadcrumb(null, "?from=inbox&fromHref=%2FPAP%2Finbox%2Funread"), + readIssueDetailBreadcrumb("PAP-465", null, "?from=inbox&fromHref=%2FPAP%2Finbox%2Funread"), ).toEqual({ label: "Inbox", href: "/PAP/inbox/unread", }); }); + it("reads hidden breadcrumb context from session storage when route state is unavailable", () => { + const state = createIssueDetailLocationState("Inbox", "/inbox/mine", "inbox"); + sessionStorageMock.clear(); + rememberIssueDetailLocationState("PAP-465", state); + + expect( + readIssueDetailLocationState("PAP-465", null), + ).toEqual({ + issueDetailBreadcrumb: { label: "Inbox", href: "/inbox/mine" }, + issueDetailSource: "inbox", + issueDetailInboxQuickArchiveArmed: false, + }); + }); + it("can arm quick archive only for explicit inbox keyboard entry state", () => { const state = createIssueDetailLocationState("Inbox", "/inbox/mine", "inbox"); diff --git a/ui/src/lib/issueDetailBreadcrumb.ts b/ui/src/lib/issueDetailBreadcrumb.ts index a53864e4..9670f561 100644 --- a/ui/src/lib/issueDetailBreadcrumb.ts +++ b/ui/src/lib/issueDetailBreadcrumb.ts @@ -13,6 +13,7 @@ type IssueDetailLocationState = { const ISSUE_DETAIL_SOURCE_QUERY_PARAM = "from"; const ISSUE_DETAIL_BREADCRUMB_HREF_QUERY_PARAM = "fromHref"; +const ISSUE_DETAIL_STORAGE_KEY_PREFIX = "paperclip:issue-detail-breadcrumb:"; function isIssueDetailBreadcrumb(value: unknown): value is IssueDetailBreadcrumb { if (typeof value !== "object" || value === null) return false; @@ -44,6 +45,17 @@ function readIssueDetailBreadcrumbHrefFromSearch(search?: string): string | null return href && href.startsWith("/") ? href : null; } +function inferIssueDetailSource( + state: Partial | null, + breadcrumb: IssueDetailBreadcrumb | null, +): IssueDetailSource | null { + if (isIssueDetailSource(state?.issueDetailSource)) return state.issueDetailSource; + if (!breadcrumb) return null; + if (breadcrumb.label === "Inbox" || breadcrumb.href.includes("/inbox")) return "inbox"; + if (breadcrumb.label === "Issues" || breadcrumb.href.includes("/issues")) return "issues"; + return null; +} + function breadcrumbForSource(source: IssueDetailSource): IssueDetailBreadcrumb { if (source === "inbox") return { label: "Inbox", href: "/inbox" }; return { label: "Issues", href: "/issues" }; @@ -71,34 +83,97 @@ export function armIssueDetailInboxQuickArchive(state: unknown): IssueDetailLoca }; } -export function createIssueDetailPath(issuePathId: string, state?: unknown, search?: string): string { - const source = readIssueDetailSource(state) ?? readIssueDetailSourceFromSearch(search); - const breadcrumb = - (typeof state === "object" && state !== null - ? (state as IssueDetailLocationState).issueDetailBreadcrumb - : null); - const breadcrumbHref = - (isIssueDetailBreadcrumb(breadcrumb) ? breadcrumb.href : null) ?? - readIssueDetailBreadcrumbHrefFromSearch(search); - if (!source) return `/issues/${issuePathId}`; - const params = new URLSearchParams(); - params.set(ISSUE_DETAIL_SOURCE_QUERY_PARAM, source); - if (breadcrumbHref) params.set(ISSUE_DETAIL_BREADCRUMB_HREF_QUERY_PARAM, breadcrumbHref); - return `/issues/${issuePathId}?${params.toString()}`; +function readStoredIssueDetailLocationState(issuePathId: string): IssueDetailLocationState | null { + if (typeof window === "undefined" || !window.sessionStorage) return null; + + const raw = window.sessionStorage.getItem(`${ISSUE_DETAIL_STORAGE_KEY_PREFIX}${issuePathId}`); + if (!raw) return null; + + try { + const parsed = JSON.parse(raw) as Partial; + const breadcrumb = isIssueDetailBreadcrumb(parsed.issueDetailBreadcrumb) + ? parsed.issueDetailBreadcrumb + : null; + const source = inferIssueDetailSource(parsed, breadcrumb); + if (!breadcrumb || !source) return null; + return { + issueDetailBreadcrumb: breadcrumb, + issueDetailSource: source, + issueDetailInboxQuickArchiveArmed: parsed.issueDetailInboxQuickArchiveArmed === true, + }; + } catch { + return null; + } } -export function readIssueDetailBreadcrumb(state: unknown, search?: string): IssueDetailBreadcrumb | null { +function normalizeIssueDetailLocationState( + state: unknown, + search?: string, +): IssueDetailLocationState | null { if (typeof state === "object" && state !== null) { const candidate = (state as IssueDetailLocationState).issueDetailBreadcrumb; - if (isIssueDetailBreadcrumb(candidate)) return candidate; + if (isIssueDetailBreadcrumb(candidate)) { + const source = inferIssueDetailSource(state as Partial, candidate); + if (!source) return null; + return { + issueDetailBreadcrumb: candidate, + issueDetailSource: source, + issueDetailInboxQuickArchiveArmed: + (state as IssueDetailLocationState).issueDetailInboxQuickArchiveArmed === true, + }; + } } const source = readIssueDetailSourceFromSearch(search); + const href = readIssueDetailBreadcrumbHrefFromSearch(search); if (!source) return null; - const fallback = breadcrumbForSource(source); - const href = readIssueDetailBreadcrumbHrefFromSearch(search); - return href ? { ...fallback, href } : fallback; + return { + issueDetailBreadcrumb: href ? { ...breadcrumbForSource(source), href } : breadcrumbForSource(source), + issueDetailSource: source, + issueDetailInboxQuickArchiveArmed: false, + }; +} + +export function rememberIssueDetailLocationState(issuePathId: string, state: unknown, search?: string): void { + if (typeof window === "undefined" || !window.sessionStorage) return; + + const normalized = normalizeIssueDetailLocationState(state, search); + if (!normalized) return; + + window.sessionStorage.setItem( + `${ISSUE_DETAIL_STORAGE_KEY_PREFIX}${issuePathId}`, + JSON.stringify(normalized), + ); +} + +export function createIssueDetailPath(issuePathId: string): string { + return `/issues/${issuePathId}`; +} + +export function hasLegacyIssueDetailQuery(search?: string): boolean { + if (!search) return false; + const params = new URLSearchParams(search); + return params.has(ISSUE_DETAIL_SOURCE_QUERY_PARAM) || params.has(ISSUE_DETAIL_BREADCRUMB_HREF_QUERY_PARAM); +} + +export function readIssueDetailLocationState( + issuePathId: string | null | undefined, + state: unknown, + search?: string, +): IssueDetailLocationState | null { + const normalized = normalizeIssueDetailLocationState(state, search); + if (normalized) return normalized; + if (!issuePathId) return null; + return readStoredIssueDetailLocationState(issuePathId); +} + +export function readIssueDetailBreadcrumb( + issuePathId: string | null | undefined, + state: unknown, + search?: string, +): IssueDetailBreadcrumb | null { + return readIssueDetailLocationState(issuePathId, state, search)?.issueDetailBreadcrumb ?? null; } export function shouldArmIssueDetailInboxQuickArchive(state: unknown): boolean { diff --git a/ui/src/pages/Inbox.tsx b/ui/src/pages/Inbox.tsx index 6bb5cb6c..052dd7c7 100644 --- a/ui/src/pages/Inbox.tsx +++ b/ui/src/pages/Inbox.tsx @@ -21,6 +21,7 @@ import { armIssueDetailInboxQuickArchive, createIssueDetailLocationState, createIssueDetailPath, + rememberIssueDetailLocationState, } from "../lib/issueDetailBreadcrumb"; import { hasBlockingShortcutDialog, isKeyboardShortcutTextInputTarget } from "../lib/keyboardShortcuts"; import { EmptyState } from "../components/EmptyState"; @@ -1521,7 +1522,8 @@ export function Inbox() { if (item.kind === "issue") { const pathId = item.issue.identifier ?? item.issue.id; const detailState = armIssueDetailInboxQuickArchive(issueLinkState); - act.navigate(createIssueDetailPath(pathId, detailState), { state: detailState }); + rememberIssueDetailLocationState(pathId, detailState); + act.navigate(createIssueDetailPath(pathId), { state: detailState }); } else if (item.kind === "approval") { act.navigate(`/approvals/${item.approval.id}`); } else if (item.kind === "failed_run") { diff --git a/ui/src/pages/IssueDetail.tsx b/ui/src/pages/IssueDetail.tsx index d2cf091f..966bba18 100644 --- a/ui/src/pages/IssueDetail.tsx +++ b/ui/src/pages/IssueDetail.tsx @@ -17,8 +17,11 @@ import { assigneeValueFromSelection, suggestedCommentAssigneeValue } from "../li import { extractIssueTimelineEvents } from "../lib/issue-timeline-events"; import { queryKeys } from "../lib/queryKeys"; import { + hasLegacyIssueDetailQuery, createIssueDetailPath, + readIssueDetailLocationState, readIssueDetailBreadcrumb, + rememberIssueDetailLocationState, shouldArmIssueDetailInboxQuickArchive, } from "../lib/issueDetailBreadcrumb"; import { hasBlockingShortcutDialog, resolveInboxQuickArchiveKeyAction } from "../lib/keyboardShortcuts"; @@ -375,9 +378,13 @@ export function IssueDetail() { ), [activeRun, liveRuns], ); + const resolvedIssueDetailState = useMemo( + () => readIssueDetailLocationState(issueId, location.state, location.search), + [issueId, location.state, location.search], + ); const sourceBreadcrumb = useMemo( - () => readIssueDetailBreadcrumb(location.state, location.search) ?? { label: "Issues", href: "/issues" }, - [location.state, location.search], + () => readIssueDetailBreadcrumb(issueId, location.state, location.search) ?? { label: "Issues", href: "/issues" }, + [issueId, location.state, location.search], ); // Filter out runs already shown by the live widget to avoid duplication @@ -967,13 +974,24 @@ export function IssueDetail() { // Redirect to identifier-based URL if navigated via UUID useEffect(() => { + const nextState = resolvedIssueDetailState ?? location.state; if (issue?.identifier && issueId !== issue.identifier) { - navigate(createIssueDetailPath(issue.identifier, location.state, location.search), { + rememberIssueDetailLocationState(issue.identifier, nextState, location.search); + navigate(createIssueDetailPath(issue.identifier), { replace: true, - state: location.state, + state: nextState, + }); + return; + } + + if (issueId && hasLegacyIssueDetailQuery(location.search)) { + rememberIssueDetailLocationState(issueId, nextState, location.search); + navigate(createIssueDetailPath(issueId), { + replace: true, + state: nextState, }); } - }, [issue, issueId, navigate, location.state, location.search]); + }, [issue, issueId, navigate, location.state, location.search, resolvedIssueDetailState]); useEffect(() => { if (!issue?.id) return; @@ -1155,8 +1173,14 @@ export function IssueDetail() { {i > 0 && } + rememberIssueDetailLocationState( + ancestor.identifier ?? ancestor.id, + resolvedIssueDetailState ?? location.state, + location.search, + )} className="hover:text-foreground transition-colors truncate max-w-[200px]" title={ancestor.title} > @@ -1575,8 +1599,14 @@ export function IssueDetail() { {childIssues.map((child) => ( + rememberIssueDetailLocationState( + child.identifier ?? child.id, + resolvedIssueDetailState ?? location.state, + location.search, + )} className="flex items-center justify-between px-3 py-2 text-sm hover:bg-accent/20 transition-colors" >