mirror of
https://github.com/alkimake/paperclip.git
synced 2026-06-14 01:50:39 +09:00
fix(ui): keep issue breadcrumb context out of the URL
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
parent
2ac1c62ab1
commit
962a882799
6 changed files with 185 additions and 49 deletions
|
|
@ -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();
|
||||
|
|
|
|||
|
|
@ -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 (
|
||||
<Link
|
||||
to={createIssueDetailPath(issuePathId, issueLinkState)}
|
||||
to={createIssueDetailPath(issuePathId)}
|
||||
state={issueLinkState}
|
||||
data-inbox-issue-link
|
||||
onClickCapture={() => 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",
|
||||
|
|
|
|||
|
|
@ -3,50 +3,80 @@ import {
|
|||
armIssueDetailInboxQuickArchive,
|
||||
createIssueDetailLocationState,
|
||||
createIssueDetailPath,
|
||||
hasLegacyIssueDetailQuery,
|
||||
readIssueDetailLocationState,
|
||||
readIssueDetailBreadcrumb,
|
||||
rememberIssueDetailLocationState,
|
||||
shouldArmIssueDetailInboxQuickArchive,
|
||||
} from "./issueDetailBreadcrumb";
|
||||
|
||||
const sessionStorageMock = (() => {
|
||||
const store = new Map<string, string>();
|
||||
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");
|
||||
|
||||
|
|
|
|||
|
|
@ -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<IssueDetailLocationState> | 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<IssueDetailLocationState>;
|
||||
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<IssueDetailLocationState>, 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 {
|
||||
|
|
|
|||
|
|
@ -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") {
|
||||
|
|
|
|||
|
|
@ -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() {
|
|||
<span key={ancestor.id} className="flex items-center gap-1">
|
||||
{i > 0 && <ChevronRight className="h-3 w-3 shrink-0" />}
|
||||
<Link
|
||||
to={createIssueDetailPath(ancestor.identifier ?? ancestor.id, location.state, location.search)}
|
||||
state={location.state}
|
||||
to={createIssueDetailPath(ancestor.identifier ?? ancestor.id)}
|
||||
state={resolvedIssueDetailState ?? location.state}
|
||||
onClickCapture={() =>
|
||||
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) => (
|
||||
<Link
|
||||
key={child.id}
|
||||
to={createIssueDetailPath(child.identifier ?? child.id, location.state, location.search)}
|
||||
state={location.state}
|
||||
to={createIssueDetailPath(child.identifier ?? child.id)}
|
||||
state={resolvedIssueDetailState ?? location.state}
|
||||
onClickCapture={() =>
|
||||
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"
|
||||
>
|
||||
<div className="flex items-center gap-2 min-w-0">
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue