mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-13 07:12:00 +01:00
fix(ui): omit task IDs from parent breadcrumbs (#20804)
## Thinking Path > - Paperclip helps people manage AI agents and their work. > - Task headers show a path from the parent task to the current task. > - The parent link adds a task number before its title on desktop. > - That extra number makes the parent title change when users open the task. > - This pull request shows the parent title without its number at every width. > - Users can follow the parent link with a consistent task title. ## Linked Issues or Issue Description Related: #20366 added parent task navigation. #10933 added the current task number. **What existing behavior does this improve?** Parent links in task header breadcrumbs. **Current behavior** A desktop parent link includes its task number. A private parent chip also shows its number on desktop. **Proposed behavior** Show only the parent title at all widths. Show Private for a private parent. Keep the current task number, title limits, and responsive truncation. **Reason and benefit** The parent title stays the same when the user follows the link. ## What Changed - Remove the parent identifier from the task page breadcrumb data. - Remove the unused viewport dependency from the breadcrumb effect after review. - Show the Private label at all widths for a private parent. - Update component tests, page navigation tests, and Storybook assertions. - Document title-only parent links in DESIGN.md. ## Verification - Six updated page regressions fail before the fix and pass after it. - All 285 focused BreadcrumbBar and IssueDetail tests pass. - UI typecheck and token gates pass. - Catalog validation and all 30 map/catalog tests pass. - Storybook build passes. All ten Chromium stories finish render/play with no page errors. - Browser checks cover 1440, 768, 390 and 320 pixel widths, light and dark themes, private parents, and keyboard parent navigation. - Parent numbers are absent at every width. Both long titles truncate on phones. The current task number stays visible, and the page has no horizontal overflow. - Private labels stay the same during desktop to phone to desktop resizing. Seven screenshots are saved as task evidence. - These checks mount the production header with fixture data. Page tests use mocked APIs. No live backend authorization or pixel baseline comparison is claimed. - [CI run 43020142521](https://github.com/paperclipai/paperclip/actions/runs/43020142521) passed onc848073ee1. Greptile reviewed that commit at 5/5. - [The independent review](https://github.com/paperclipai/paperclip/pull/20804#issuecomment-6795977289) found one unused viewport dependency. Commitd32c7aa6dcremoves it. All 285 focused UI tests and UI typecheck pass after this change. - Greptile and the independent Paperclip review both gived32c7aa6dca clean 5/5. There are no inline review threads. - [CI run 43022686830](https://github.com/paperclipai/paperclip/actions/runs/43022686830) attempt 1 passed all jobs except Runner Rust and its summary gate. The Rust process-reaping test reported one unattributed orphan after its owned tool had stopped. The Rust source tree and test command match the preceding passing run. Same-user orphan scans can overlap with parallel tests; the log does not identify the orphan's source. - [CI attempt 2](https://github.com/paperclipai/paperclip/actions/runs/43022686830/attempts/2) passed ond32c7aa6dcafter the one failed-job retry. Runner Rust and the summary gate now pass. All 63 current contexts are complete: 59 success, one neutral Paperclip review, and three conditional skips. There are no pending or failed contexts. The saved responsive screenshots remain current. - Workspace preflight could not complete because Cargo is absent. The shared and plugin SDK builds passed. No Runner source changed. ## Risks - Low risk. The change affects parent header labels. - Parent routes and collection filters stay intact. Navigation tests cover route state, saved state, and legacy query links. - Private parents remain noninteractive. - No API, schema, stored title, or shared breadcrumb renderer changes. ## Model Used - OpenAI Codex, based on GPT-6, with reasoning, tool use, code editing, and browser verification. The exact served snapshot ID and context window size are not exposed. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
4c04c20c9e
commit
123ee31552
5 files changed
+28
-24
No files matched your search
@@ -847,8 +847,9 @@ Task breadcrumbs link the immediate parent before the current task. Each task
|
||||
title displays at most 48 Unicode code points, including an ellipsis; saved
|
||||
titles, hover text and accessible names retain the full title. The parent uses
|
||||
at most 40% of the breadcrumb row, and both titles truncate when space runs out.
|
||||
Mobile omits the collection link and parent identifier to preserve both titles
|
||||
and the current task identifier. Private parents show a noninteractive label.
|
||||
Parent breadcrumbs show only the task title, with no identifier, at every width.
|
||||
Mobile omits the collection link to preserve both titles and the current task
|
||||
identifier. Private parents show a noninteractive Private label without an identifier.
|
||||
|
||||
The local task detail also offers a **Thread preview** selector. DES-1 opens a
|
||||
complete synthetic conversation matching the staging transcript layout; other
|
||||
|
||||
@@ -82,7 +82,7 @@ function TaskBreadcrumbs({
|
||||
useEffect(() => {
|
||||
setBreadcrumbs([
|
||||
{ label: "Tasks", href: sourceHref },
|
||||
...(parentTitle ? [{ label: parentTitle, identifier: "PAP-120", href: "/issues/PAP-120" }] : []),
|
||||
...(parentTitle ? [{ label: parentTitle, href: "/issues/PAP-120" }] : []),
|
||||
{
|
||||
label: title,
|
||||
identifier,
|
||||
@@ -273,8 +273,9 @@ describe("BreadcrumbBar", () => {
|
||||
<BreadcrumbProvider><TaskBreadcrumbs taskDetailLayout parentTitle="Prepare the engineering team" /></BreadcrumbProvider>,
|
||||
));
|
||||
const parent = container.querySelector<HTMLAnchorElement>('a[href="/issues/PAP-120"]');
|
||||
expect(parent?.textContent).toContain("Prepare the engineering team");
|
||||
expect(parent?.getAttribute("aria-label")).toBe("Prepare the engineering team PAP-120");
|
||||
expect(parent?.textContent).toBe("Prepare the engineering team");
|
||||
expect(parent?.querySelector('[data-slot="task-title-identifier"]')).toBeNull();
|
||||
expect(parent?.getAttribute("aria-label")).toBe("Prepare the engineering team");
|
||||
expect(container.querySelector('[aria-current="page"]')?.textContent).toContain("PAP-16679");
|
||||
if (mobile) {
|
||||
expect(container.querySelector('a[href="/issues"]')).toBeNull();
|
||||
|
||||
@@ -2923,7 +2923,7 @@ describe("IssueDetail", () => {
|
||||
expect(container.textContent).toContain("Issue detail smoke");
|
||||
expect(mockSetBreadcrumbs).toHaveBeenLastCalledWith([
|
||||
{ label: "Tasks", href: "/issues" },
|
||||
expect.objectContaining({ label: "Parent task visible in Properties", identifier: "PAP-0", href: "/issues/PAP-0" }),
|
||||
expect.objectContaining({ label: "Parent task visible in Properties", href: "/issues/PAP-0" }),
|
||||
expect.objectContaining({ label: "Issue detail smoke" }),
|
||||
]);
|
||||
});
|
||||
@@ -2939,6 +2939,8 @@ describe("IssueDetail", () => {
|
||||
await act(async () => { renderIssueView(); });
|
||||
await flushReact();
|
||||
const parentCrumb = mockSetBreadcrumbs.mock.calls.at(-1)![0][1];
|
||||
expect(parentCrumb.identifier).toBeUndefined();
|
||||
expect(parentCrumb.label).toBe("Parent task");
|
||||
expect(parentCrumb.onClick).toBeTypeOf("function");
|
||||
parentCrumb.onClick();
|
||||
const expected = { label: source === "query" ? "Inbox" : "Mine", href: state.issueDetailBreadcrumb!.href };
|
||||
@@ -2951,7 +2953,9 @@ describe("IssueDetail", () => {
|
||||
mockIssuesApi.get.mockResolvedValue(createIssue({ id: "parent-1", identifier: "PAP-0", title: "Parent task" }));
|
||||
await act(async () => { renderIssueView(); });
|
||||
await waitForAssertion(() => {
|
||||
expect(mockSetBreadcrumbs.mock.calls.at(-1)![0][0]).toEqual(expected);
|
||||
const breadcrumbs = mockSetBreadcrumbs.mock.calls.at(-1)![0];
|
||||
expect(breadcrumbs[0]).toEqual(expected);
|
||||
expect(breadcrumbs.at(-1)?.label).toBe(parentCrumb.label);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -2964,19 +2968,20 @@ describe("IssueDetail", () => {
|
||||
await flushReact();
|
||||
expect(mockSetBreadcrumbs).toHaveBeenLastCalledWith([
|
||||
{ label: "Tasks", href: "/issues" },
|
||||
expect.objectContaining({ label: "Private task", identifier: "PAP-0", href: undefined }),
|
||||
expect.objectContaining({ label: "Private task", href: undefined }),
|
||||
expect.objectContaining({ label: "Issue detail smoke" }),
|
||||
]);
|
||||
const parentCrumb = mockSetBreadcrumbs.mock.calls.at(-1)![0][1];
|
||||
expect(parentCrumb.identifier).toBeUndefined();
|
||||
expect(parentCrumb.onClick).toBeUndefined();
|
||||
await act(async () => { renderIssueView(parentCrumb.control); });
|
||||
const chip = container.querySelector('[data-testid="locked-issue-chip"]');
|
||||
expect(chip).not.toBeNull();
|
||||
expect(chip?.textContent).toBe(isMobile ? "Private" : "PAP-0");
|
||||
expect(chip?.textContent).toBe("Private");
|
||||
expect(container.querySelector('a, [role="link"]')).toBeNull();
|
||||
});
|
||||
|
||||
it("updates the private parent label when an open task changes viewport", async () => {
|
||||
it("keeps the private parent title free of IDs when an open task changes viewport", async () => {
|
||||
mockIssuesApi.get.mockResolvedValue(createIssue({
|
||||
ancestors: [{ id: "private-parent", identifier: "PAP-0", locked: true }] as unknown as Issue["ancestors"],
|
||||
}));
|
||||
@@ -3000,7 +3005,7 @@ describe("IssueDetail", () => {
|
||||
await act(async () => {
|
||||
renderIssueView(<BreadcrumbProvider><RenderBreadcrumbControl /></BreadcrumbProvider>);
|
||||
});
|
||||
for (const [index, expected] of ["PAP-0", "Private", "PAP-0"].entries()) {
|
||||
for (const [index, expected] of ["Private", "Private", "Private"].entries()) {
|
||||
await act(async () => { updateBreadcrumbs(captured[index]); });
|
||||
expect(container.querySelector('[data-testid="locked-issue-chip"]')?.textContent).toBe(expected);
|
||||
expect(container.querySelector('a, [role="link"]')).toBeNull();
|
||||
|
||||
@@ -5994,15 +5994,14 @@ export function TaskDetailSurface({ conversation, tasksTab }: { tasksTab?: TaskS
|
||||
sourceBreadcrumb,
|
||||
...(breadcrumbParent ? [{
|
||||
label: isLockedIssueStub(breadcrumbParent) ? "Private task" : breadcrumbParent.title,
|
||||
identifier: breadcrumbParent.identifier ?? undefined,
|
||||
href: isLockedIssueStub(breadcrumbParent)
|
||||
? undefined
|
||||
: createIssueDetailPath(breadcrumbParent.identifier ?? breadcrumbParent.id),
|
||||
onClick: isLockedIssueStub(breadcrumbParent) ? undefined : rememberBreadcrumbParent,
|
||||
control: isLockedIssueStub(breadcrumbParent)
|
||||
? <LockedIssueChip identifier={isMobile ? null : breadcrumbParent.identifier} />
|
||||
? <LockedIssueChip identifier={null} />
|
||||
: undefined,
|
||||
controlKey: isLockedIssueStub(breadcrumbParent) ? `private-parent:${isMobile}` : undefined,
|
||||
controlKey: isLockedIssueStub(breadcrumbParent) ? "private-parent" : undefined,
|
||||
}] : []),
|
||||
{
|
||||
// The status glyph (leading) already conveys in-progress/live state;
|
||||
@@ -6019,7 +6018,6 @@ export function TaskDetailSurface({ conversation, tasksTab }: { tasksTab?: TaskS
|
||||
breadcrumbTitle,
|
||||
breadcrumbIdentifier,
|
||||
breadcrumbParent,
|
||||
isMobile,
|
||||
rememberBreadcrumbParent,
|
||||
hasLiveRuns,
|
||||
setBreadcrumbs,
|
||||
|
||||
@@ -5,7 +5,6 @@ import { BreadcrumbBar } from "@/components/BreadcrumbBar";
|
||||
import { LockedIssueChip } from "@/components/LockedIssueChip";
|
||||
import { StatusIcon } from "@/components/StatusIcon";
|
||||
import { useBreadcrumbs } from "@/context/BreadcrumbContext";
|
||||
import { useSidebar } from "@/context/SidebarContext";
|
||||
import { useLocation } from "@/lib/router";
|
||||
import { PluginLauncherProvider } from "@/plugins/launchers";
|
||||
|
||||
@@ -23,21 +22,19 @@ function TaskBreadcrumbPreview({
|
||||
source?: "Tasks" | "Mine";
|
||||
}) {
|
||||
const { setBreadcrumbs } = useBreadcrumbs();
|
||||
const { isMobile } = useSidebar();
|
||||
const location = useLocation();
|
||||
useEffect(() => {
|
||||
setBreadcrumbs([
|
||||
{ label: source, href: source === "Mine" ? "/inbox" : "/issues" },
|
||||
...(showParent ? [{
|
||||
label: privateParent ? "Private task" : parentTitle,
|
||||
identifier: "PAP-120",
|
||||
href: privateParent ? undefined : "/issues/PAP-120",
|
||||
control: privateParent ? <LockedIssueChip identifier={isMobile ? null : "PAP-120"} /> : undefined,
|
||||
controlKey: privateParent ? `private-parent:${isMobile}` : undefined,
|
||||
control: privateParent ? <LockedIssueChip identifier={null} /> : undefined,
|
||||
controlKey: privateParent ? "private-parent" : undefined,
|
||||
}] : []),
|
||||
{ label: title, identifier: "PAP-121", leading: <StatusIcon status="in_progress" className="size-3" />, leadingKey: "in_progress" },
|
||||
]);
|
||||
}, [parentTitle, title, showParent, privateParent, source, isMobile, setBreadcrumbs]);
|
||||
}, [parentTitle, title, showParent, privateParent, source, setBreadcrumbs]);
|
||||
|
||||
return (
|
||||
<div className="min-h-dvh w-full bg-background text-foreground">
|
||||
@@ -67,7 +64,7 @@ const meta = {
|
||||
status: "current",
|
||||
implementation: "production",
|
||||
boundary: "fixtures",
|
||||
notes: "Mounts the shipped BreadcrumbBar with fixture breadcrumb context. Covers linked immediate parent, long titles, private parent, standalone task, Mine source, light/dark themes and responsive widths. Page data loading and parent authorization are outside this isolated header story.",
|
||||
notes: "Mounts the shipped BreadcrumbBar with fixture breadcrumb context. Covers title-only linked immediate parent, long titles, private parent, standalone task, Mine source, light/dark themes and responsive widths. Page data loading and parent authorization are outside this isolated header story.",
|
||||
},
|
||||
},
|
||||
args: { parentTitle: "Prepare the product launch", title: "Review the release checklist" },
|
||||
@@ -79,7 +76,8 @@ type Story = StoryObj<typeof meta>;
|
||||
export const Subtask: Story = {
|
||||
play: async ({ canvasElement }) => {
|
||||
const nav = within(await within(canvasElement).findByRole("navigation", { name: "breadcrumb" }));
|
||||
await expect(nav.getByRole("link", { name: "Prepare the product launch PAP-120" })).toHaveAttribute("href", "/PAP/issues/PAP-120");
|
||||
await expect(nav.getByRole("link", { name: "Prepare the product launch" })).toHaveAttribute("href", "/PAP/issues/PAP-120");
|
||||
await expect(nav.getByRole("link", { name: "Prepare the product launch" })).toHaveTextContent(/^Prepare the product launch$/);
|
||||
await expect(nav.getByRole("link", { name: "Review the release checklist PAP-121" })).toHaveAttribute("aria-current", "page");
|
||||
},
|
||||
};
|
||||
@@ -108,7 +106,8 @@ export const PrivateParent: Story = {
|
||||
const nav = await within(canvasElement).findByRole("navigation", { name: "breadcrumb" });
|
||||
await expect(within(nav).queryByRole("link", { name: /Prepare the product launch/ })).not.toBeInTheDocument();
|
||||
await expect(within(nav).queryByRole("link", { name: /private/i })).not.toBeInTheDocument();
|
||||
await expect(within(nav).getByTestId("locked-issue-chip")).toBeVisible();
|
||||
await expect(within(nav).getByTestId("locked-issue-chip")).toHaveTextContent(/^Private$/);
|
||||
await expect(nav).not.toHaveTextContent("PAP-120");
|
||||
await expect(nav.querySelectorAll('[aria-current="page"]')).toHaveLength(1);
|
||||
},
|
||||
};
|
||||
|
||||
Reference in new issue
Block a user