diff --git a/tests/storybook-visual/relationship-badges.spec.ts b/tests/storybook-visual/relationship-badges.spec.ts new file mode 100644 index 0000000000..b5958068af --- /dev/null +++ b/tests/storybook-visual/relationship-badges.spec.ts @@ -0,0 +1,68 @@ +import { expect, test } from "@playwright/test"; + +const storyIds = [ + "product-issue-management--issue-properties-relationship-badges", + "product-issue-management--issue-properties-relationship-badges-inline", +]; + +for (const storyId of storyIds) { + test(`${storyId} stays still until an operator acts`, async ({ page }) => { + await page.goto(`/iframe.html?id=${storyId}&viewMode=story&globals=theme:dark`); + const remove = page.getByRole("button", { name: "Remove PAP-18313 as blocker", exact: true }); + await expect(remove).toBeAttached(); + + // Sample successive painted frames, not just the settled end state: an + // autoplay remove/reset cycle would otherwise leave an identical screenshot. + const frames = await page.evaluate(async () => { + const samples = []; + for (let frame = 0; frame < 60; frame += 1) { + await new Promise((resolve) => requestAnimationFrame(() => resolve())); + const button = document.querySelector('button[aria-label="Remove PAP-18313 as blocker"]'); + const link = button?.previousElementSibling; + samples.push({ + route: document.querySelector('[data-testid="relationship-route"]')?.textContent, + opacity: button ? getComputedStyle(button).opacity : null, + badge: link?.getBoundingClientRect().toJSON() ?? null, + }); + } + return samples; + }); + expect(frames[0].route).toBe("/PAP/storybook"); + expect(frames[0].opacity).toBe("0"); + expect(frames[0].badge).not.toBeNull(); + for (const frame of frames) expect(frame).toEqual(frames[0]); + + const link = page.getByRole("link", { name: "Task PAP-18313: Review task relationships", exact: true }).first(); + const before = await link.boundingBox(); + await link.hover(); + await expect(remove).toHaveCSS("opacity", "1"); + expect(await link.boundingBox()).toEqual(before); + const textRight = await link.locator("span").evaluate((element) => element.getBoundingClientRect().right); + expect(textRight).toBeLessThan((await remove.boundingBox())!.x); + + await link.getByRole("img", { name: "Todo" }).click(); + await expect(page.getByTestId("relationship-route")).toHaveText("/PAP/issues/PAP-18313"); + await expect(remove).toBeAttached(); + await page.keyboard.press("Tab"); + await expect(remove).toBeFocused(); + await page.keyboard.press("Space"); + await expect(remove).not.toBeAttached(); + await expect(page.getByRole("button", { name: "Remove PAP-18314 as blocker" })).toBeAttached(); + await expect(page.getByTestId("relationship-route")).toHaveText("/PAP/issues/PAP-18313"); + }); +} + +test("switching relationship previews never runs removal or navigation", async ({ page }) => { + await page.goto(`/?path=/story/${storyIds[0]}`); + const preview = page.frameLocator("#storybook-preview-iframe"); + for (const name of [ + "IssueProperties - relationship badges inline", + "IssueProperties - relationship badges", + "IssueProperties - relationship badges inline", + ]) { + await page.getByRole("link", { name, exact: true }).click(); + await expect(preview.getByRole("button", { name: "Remove PAP-18313 as blocker" })).toBeAttached(); + await expect(preview.getByTestId("relationship-route")).toHaveText("/PAP/storybook"); + await expect(preview.getByRole("button", { name: "Remove PAP-18314 as blocker" })).toBeAttached(); + } +}); diff --git a/ui/src/components/IssueProperties.test.tsx b/ui/src/components/IssueProperties.test.tsx index c012f1d19b..8f72726293 100644 --- a/ui/src/components/IssueProperties.test.tsx +++ b/ui/src/components/IssueProperties.test.tsx @@ -209,7 +209,7 @@ async function flush() { function findRowTrigger(container: HTMLElement, label: string): HTMLButtonElement | undefined { const labelSpan = container.querySelector(`[data-property-label="${label}"]`); const row = labelSpan?.closest('[data-property-row="true"]'); - return (row?.querySelector("button") as HTMLButtonElement | null) ?? undefined; + return ((row?.querySelector(`button[aria-label="Edit ${label.toLowerCase()}"]`) ?? row?.querySelector("button")) as HTMLButtonElement | null) ?? undefined; } async function waitForAssertion(assertion: () => void, attempts = 20) { @@ -590,7 +590,9 @@ describe("IssueProperties", () => { for (const label of ["Labels", "Blocked by", "Subtasks"]) { const trigger = findRowTrigger(container, label); - const chipStack = trigger?.querySelector("div"); + const chipStack = label === "Labels" + ? trigger?.querySelector("div") + : trigger?.closest('[data-property-value="true"]')?.querySelector(".flex-col"); expect(chipStack?.classList).toContain("flex-col"); expect(chipStack?.classList).toContain("items-start"); } @@ -1206,6 +1208,66 @@ describe("IssueProperties", () => { act(() => root.unmount()); }); + it.each([false, true])("keeps relationship badges mounted as queries settle (inline=%s)", async (inline) => { + let resolveProjects!: (projects: Project[]) => void; + mockProjectsApi.list.mockReturnValue(new Promise((resolve) => { resolveProjects = resolve; })); + const onUpdate = vi.fn(); + const issue = createIssue({ + blockedBy: [createIssue({ id: "blocker-1", identifier: "PAP-2", status: "in_progress" })], + }); + const props = { issue, childIssues: [], onUpdate, inline, sidePanelContentOnly: true }; + const { root, queryClient } = renderPropertiesWithQueryClient(container, props); + const link = container.querySelector('a[href="/issues/PAP-2"]'); + const remove = container.querySelector('[aria-label="Remove PAP-2 as blocker"]'); + expect(link).not.toBeNull(); + expect(remove).not.toBeNull(); + await flush(); + await act(async () => resolveProjects([])); + await flush(); + // A parent page can provide a fresh task object after a query refresh. + await act(async () => root.render( + + + , + )); + expect(container.querySelector('a[href="/issues/PAP-2"]')).toBe(link); + expect(container.querySelector('[aria-label="Remove PAP-2 as blocker"]')).toBe(remove); + expect(link?.textContent).toContain("in_progress"); + expect(onUpdate).not.toHaveBeenCalled(); + expect(container.querySelector('[aria-expanded="true"]')).toBeNull(); + act(() => root.unmount()); + }); + + it("links relationship status and IDs and removes only the selected blocker with its X", async () => { + const onUpdate = vi.fn(); + const blockers = [ + createIssue({ id: "issue-2", identifier: "PAP-2", title: "Existing blocker", status: "in_progress" }), + createIssue({ id: "issue-3", identifier: "PAP-3", title: "Keep blocker", status: "todo" }), + ]; + const root = renderProperties(container, { + issue: createIssue({ blockedBy: blockers }), + childIssues: [createIssue({ id: "child-1", identifier: "PAP-4", status: "done" })], + onUpdate, + inline: true, + }); + await flush(); + const row = container.querySelector('[data-property-label="Blocked by"]')!.closest('[data-property-row]')!; + const link = row.querySelector('a[href="/issues/PAP-2"]')!; + expect(link).not.toBeNull(); + expect(link.textContent).toContain("in_progress"); + expect(link.closest("button")).toBeNull(); + link.addEventListener("click", (event) => event.preventDefault()); + await act(async () => link.click()); + expect(onUpdate).not.toHaveBeenCalled(); + expect(container.querySelector('input[aria-label="Search tasks to add as blockers"]')).toBeNull(); + const remove = row.querySelector('button[aria-label="Remove PAP-2 as blocker"]')!; + expect(remove.closest("a")).toBeNull(); + await act(async () => remove.click()); + expect(onUpdate).toHaveBeenCalledExactlyOnceWith({ blockedByIssueIds: ["issue-3"] }); + expect(container.querySelector('a[href="/issues/PAP-4"]')?.textContent).toContain("done"); + act(() => root.unmount()); + }); + it("edits blockers from the blocked-by relationship flyout", async () => { const onUpdate = vi.fn(); mockIssuesApi.list.mockResolvedValue([ @@ -1233,7 +1295,7 @@ describe("IssueProperties", () => { await flush(); const blockerTrigger = findRowTrigger(container, "Blocked by"); - expect(blockerTrigger?.textContent).toContain("PAP-2"); + expect(blockerTrigger?.closest('[data-property-row="true"]')?.textContent).toContain("PAP-2"); expect(container.textContent).not.toContain("Add blocker"); expect(container.querySelector('input[placeholder="Search tasks..."]')).toBeNull(); @@ -1395,7 +1457,7 @@ describe("IssueProperties", () => { await flush(); const blockerTrigger = findRowTrigger(container, "Blocked by"); - expect(blockerTrigger?.textContent).toContain("PAP-2"); + expect(blockerTrigger?.closest('[data-property-row="true"]')?.textContent).toContain("PAP-2"); await act(async () => { blockerTrigger!.dispatchEvent(new MouseEvent("click", { bubbles: true })); @@ -1439,16 +1501,16 @@ describe("IssueProperties", () => { await flush(); const blockedByTrigger = findRowTrigger(container, "Blocked by"); - expect(blockedByTrigger?.textContent).toContain("BLOCK-1"); - expect(blockedByTrigger?.textContent).toContain("BLOCK-2"); - expect(blockedByTrigger?.textContent).toContain("+5 more"); - expect(blockedByTrigger?.textContent).not.toContain("BLOCK-7"); + expect(blockedByTrigger?.closest('[data-property-row="true"]')?.textContent).toContain("BLOCK-1"); + expect(blockedByTrigger?.closest('[data-property-row="true"]')?.textContent).toContain("BLOCK-2"); + expect(blockedByTrigger?.closest('[data-property-row="true"]')?.textContent).toContain("+5 more"); + expect(blockedByTrigger?.closest('[data-property-row="true"]')?.textContent).not.toContain("BLOCK-7"); const subtasksTrigger = findRowTrigger(container, "Subtasks"); - expect(subtasksTrigger?.textContent).toContain("SUB-1"); - expect(subtasksTrigger?.textContent).toContain("SUB-2"); - expect(subtasksTrigger?.textContent).toContain("+5 more"); - expect(subtasksTrigger?.textContent).not.toContain("SUB-7"); + expect(subtasksTrigger?.closest('[data-property-row="true"]')?.textContent).toContain("SUB-1"); + expect(subtasksTrigger?.closest('[data-property-row="true"]')?.textContent).toContain("SUB-2"); + expect(subtasksTrigger?.closest('[data-property-row="true"]')?.textContent).toContain("+5 more"); + expect(subtasksTrigger?.closest('[data-property-row="true"]')?.textContent).not.toContain("SUB-7"); await act(async () => { blockedByTrigger!.dispatchEvent(new MouseEvent("click", { bubbles: true })); @@ -1632,8 +1694,8 @@ describe("IssueProperties", () => { }); await flush(); - expect(findRowTrigger(container, "Blocked by")?.textContent).toContain("BLOCK-1"); - expect(findRowTrigger(container, "Blocked by")?.textContent).toContain("+5 more"); + expect(findRowTrigger(container, "Blocked by")?.closest('[data-property-row="true"]')?.textContent).toContain("BLOCK-1"); + expect(findRowTrigger(container, "Blocked by")?.closest('[data-property-row="true"]')?.textContent).toContain("+5 more"); const nextBlockedBy = [{ id: "next-blocker", @@ -1659,9 +1721,9 @@ describe("IssueProperties", () => { }); await flush(); - expect(findRowTrigger(container, "Blocked by")?.textContent).toContain("NEXT-1"); - expect(findRowTrigger(container, "Blocked by")?.textContent).not.toContain("BLOCK-1"); - expect(findRowTrigger(container, "Blocked by")?.textContent).not.toContain("more"); + expect(findRowTrigger(container, "Blocked by")?.closest('[data-property-row="true"]')?.textContent).toContain("NEXT-1"); + expect(findRowTrigger(container, "Blocked by")?.closest('[data-property-row="true"]')?.textContent).not.toContain("BLOCK-1"); + expect(findRowTrigger(container, "Blocked by")?.closest('[data-property-row="true"]')?.textContent).not.toContain("more"); act(() => root.unmount()); }); @@ -2407,8 +2469,7 @@ describe("IssueProperties", () => { }); await flush(); - const selectedParentTrigger = Array.from(container.querySelectorAll("button")) - .find((button) => button.textContent?.includes("PAP-2 Candidate parent")); + const selectedParentTrigger = findRowTrigger(container, "Parent"); expect(selectedParentTrigger).not.toBeUndefined(); const parentLink = container.querySelector('a[href="/issues/PAP-2"]'); expect(parentLink).not.toBeNull(); diff --git a/ui/src/components/IssueReferencePill.tsx b/ui/src/components/IssueReferencePill.tsx index 1e23262212..e376db9444 100644 --- a/ui/src/components/IssueReferencePill.tsx +++ b/ui/src/components/IssueReferencePill.tsx @@ -1,7 +1,9 @@ +import { X } from "lucide-react"; import type { ReactNode } from "react"; import type { IssueRelationIssueSummary } from "@paperclipai/shared"; import { Link } from "@/lib/router"; import { cn } from "../lib/utils"; +import { badgeVariants } from "./ui/badge"; import { StatusIcon } from "./StatusIcon"; export function IssueReferencePill({ @@ -9,29 +11,65 @@ export function IssueReferencePill({ strikethrough, className, children, + onRemove, + variant = "mention", }: { issue: Pick & - Partial>; + { status?: string }; strikethrough?: boolean; + variant?: "mention" | "property"; className?: string; children?: ReactNode; + /** Reserves space for a separate hover/focus action without moving the task link. */ + onRemove?: (issueId: string) => void; }) { const issueLabel = issue.identifier ?? issue.title; const classNames = cn( - "paperclip-mention-chip paperclip-mention-chip--issue", - "inline-flex items-center gap-1 rounded-full border border-border px-2 py-0.5 text-xs no-underline", + variant === "property" || onRemove + ? cn(badgeVariants({ variant: "outline" }), "min-w-0 max-w-full shrink font-normal no-underline") + : "paperclip-mention-chip paperclip-mention-chip--issue inline-flex items-center gap-1 rounded-full border border-border px-2 py-0.5 text-xs no-underline", issue.identifier && "hover:bg-accent/50 focus-visible:outline-none focus-visible:ring-(length:--rad-3) focus-visible:ring-ring", + onRemove && "pr-6", strikethrough && "opacity-60 line-through decoration-muted-foreground", className, ); const content = ( <> {issue.status ? : null} - {children !== undefined ? children : {issue.identifier ?? issue.title}} + {children !== undefined ? children : {issue.identifier ?? issue.title}} ); - if (!issue.identifier) { + if (onRemove) { + return ( + + + {content} + + + + ); + } + + if (!issue.identifier && variant === "mention") { return ( 0 ? (
{blockedByRelations.slice(0, 2).map((relation) => ( - - {relation.identifier ?? relation.title} - + onUpdate({ blockedByIssueIds: blockedByIds.filter((candidate) => candidate !== id) })} + /> ))} {blockedByRelations.length > 2 ? ( - - +{blockedByRelations.length - 2} more + + ) : null}
@@ -2062,13 +2064,11 @@ export function IssueProperties({ const subtasksTrigger = childIssues.length > 0 ? (
{childIssues.slice(0, 2).map((child) => ( - - {child.identifier ?? child.title} - + ))} {childIssues.length > 2 ? ( - - +{childIssues.length - 2} more + + ) : null}
@@ -2106,25 +2106,19 @@ export function IssueProperties({ const parentIdentifier = issue.ancestors?.[0]?.identifier ?? currentParentIssue?.identifier; const parentTitle = issue.ancestors?.[0]?.title ?? currentParentIssue?.title ?? issue.parentId?.slice(0, 8); const parentTrigger = issue.parentId ? ( - - {parentIdentifier ? `${parentIdentifier} ` : ""} - {parentTitle} - + ) : ( None ); - const parentLink = issue.parentId ? ( - e.stopPropagation()} - > - - - ) : undefined; const parentSearchActive = normalizedParentSearch.length > 0; // When the user types, search on the server. The default list caps at 500 rows // and sorts priority-first, so a medium-priority or low-priority match past that @@ -2438,7 +2432,7 @@ export function IssueProperties({ triggerContent={parentTrigger} triggerClassName="min-w-0 max-w-full" popoverClassName="w-72" - extra={parentLink} + separateTrigger={!!issue.parentId} > {parentContent} @@ -2452,6 +2446,7 @@ export function IssueProperties({ setBlockedByOpen(open); if (!open) setBlockedBySearch(""); }} + separateTrigger={blockedByRelations.length > 0} triggerContent={blockedByTrigger} triggerClassName="min-w-0 max-w-full" popoverClassName="w-72" @@ -2517,7 +2512,7 @@ export function IssueProperties({ {blockingIssues.length > 0 ? (
{visibleBlockingIssues.map((relation) => ( - + ))} 0} triggerContent={subtasksTrigger} triggerClassName="min-w-0 max-w-full" popoverClassName="w-72" diff --git a/ui/src/components/issue-properties/property-picker.tsx b/ui/src/components/issue-properties/property-picker.tsx index 582bc36049..0bea3aeff8 100644 --- a/ui/src/components/issue-properties/property-picker.tsx +++ b/ui/src/components/issue-properties/property-picker.tsx @@ -17,6 +17,7 @@ export function PropertyPicker({ popoverAlign = "end", extra, stacked = false, + separateTrigger = false, children, }: { inline?: boolean; @@ -30,6 +31,8 @@ export function PropertyPicker({ extra?: ReactNode; /** Top-aligns the row and vertically stacks chip collections in the trigger. */ stacked?: boolean; + /** Keep navigable relationship badges outside the picker button. */ + separateTrigger?: boolean; children: ReactNode; }) { const { enabled: streamlinedUiEnabled } = useStreamlinedUiEnabled(); @@ -39,6 +42,43 @@ export function PropertyPicker({ triggerClassName, ); + if (separateTrigger) { + const trigger = ( + + ); + return ( +
+ +
+ {triggerContent} + {inline ? trigger : ( + + {trigger} + + {children} + + + )} +
+ {extra} + + {inline && open ? ( +
+ {children} +
+ ) : null} +
+ ); + } + if (inline) { return (
diff --git a/ui/src/pages/DesignGuide.tsx b/ui/src/pages/DesignGuide.tsx index 5608504ad0..33e412861c 100644 --- a/ui/src/pages/DesignGuide.tsx +++ b/ui/src/pages/DesignGuide.tsx @@ -802,6 +802,8 @@ export function DesignGuide() {

Used wherever a task is referenced — in markdown, the Related Work tab, and activity summaries. Pass status to show the target issue's state at a glance. + Use variant="property" for compact badges with direct navigation. + Pass onRemove for a separate blocker removal control with reserved space. Use strikethrough for "removed" contexts.

@@ -809,6 +811,7 @@ export function DesignGuide() { + window.alert("Blocker removed")} issue={{ id: "demo-blocker", identifier: "PAP-303", title: "Hover or focus to remove blocker", status: "in_review" }} />
diff --git a/ui/storybook/stories/issue-management.stories.tsx b/ui/storybook/stories/issue-management.stories.tsx index 06fc95976f..3e1a67abb0 100644 --- a/ui/storybook/stories/issue-management.stories.tsx +++ b/ui/storybook/stories/issue-management.stories.tsx @@ -21,6 +21,7 @@ import { IssueDocumentsSection } from "@/components/IssueDocumentsSection"; import { IssueFiltersPopover } from "@/components/IssueFiltersPopover"; import { IssueGroupHeader } from "@/components/IssueGroupHeader"; import { IssueLinkQuicklook, IssueQuicklookCard } from "@/components/IssueLinkQuicklook"; +import { useLocation } from "@/lib/router"; import { IssueProperties } from "@/components/IssueProperties"; import { IssueRunLedgerContent } from "@/components/IssueRunLedger"; import { IssuesList } from "@/components/IssuesList"; @@ -191,6 +192,7 @@ function hydrateStorybookQueries(queryClient: ReturnType) queryClient.setQueryData(queryKeys.auth.session, storybookAuthSession); queryClient.setQueryData(queryKeys.agents.list(companyId), storybookAgents); queryClient.setQueryData(queryKeys.projects.list(companyId), storybookProjects); + queryClient.setQueryData(queryKeys.projects.list(companyId, { includeArchived: true }), storybookProjects); queryClient.setQueryData(queryKeys.issues.list(companyId), storybookIssues); queryClient.setQueryData(queryKeys.issues.labels(companyId), storybookIssueLabels); queryClient.setQueryData(queryKeys.issues.documents(primaryIssue.id), storybookIssueDocuments); @@ -282,6 +284,7 @@ function LongValueStorybookData({ children }: { children: React.ReactNode }) { const [ready] = useState(() => { hydrateStorybookQueries(queryClient); queryClient.setQueryData(queryKeys.projects.list(companyId), [longProject, ...storybookProjects]); + queryClient.setQueryData(queryKeys.projects.list(companyId, { includeArchived: true }), [longProject, ...storybookProjects]); queryClient.setQueryData(queryKeys.issues.list(companyId), [ longValueIssue, longParentIssue, @@ -324,6 +327,62 @@ function IssuePropertiesLongValuePane({ inline = false }: { inline?: boolean }) ); } +const relationshipChildren: Issue[] = ["in_progress", "todo", "in_review", "done"].map((status, index) => ({ + ...storybookIssues[0]!, + id: `relationship-child-${index}`, + identifier: `PAP-${18312 + index}`, + title: ["Implement task badges", "Review task relationships", "Verify keyboard navigation", "Ship task properties"][index]!, + status: status as Issue["status"], + parentId: "relationship-demo", +})); +const relationshipIssue: Issue = { + ...longValueIssue, + id: "relationship-demo", + projectId: primaryIssue.projectId, + project: primaryIssue.project, + identifier: "PAP-18311", + labels: [], + labelIds: [], + blockedBy: [relationshipChildren[1]!, relationshipChildren[2]!], + blocks: [relationshipChildren[3]!], +}; + +function IssuePropertiesRelationshipBadgesPane({ inline = false }: { inline?: boolean }) { + const [issue, setIssue] = useState(relationshipIssue); + const location = useLocation(); + return ( + +
+
+
Properties
+
+ setIssue((current) => ({ + ...current, + ...patch, + blockedBy: patch.blockedByIssueIds + ? [...relationshipChildren, ...storybookIssues, longParentIssue, longValueIssue].filter((child) => (patch.blockedByIssueIds as string[]).includes(child.id)) + : current.blockedBy, + }))} + /> +
+
+
+

Task relationship badges

+

Click a status icon or task ID to navigate. Hover or focus a blocker to reveal its remove button. Only the X removes that blocker.

+

The arrow opens the relationship picker. Badge widths stay fixed on hover.

+

Current route: {location.pathname}

+ +
+
+ + ); +} + function IssuePropertiesModelOverridePane() { return ( @@ -852,3 +911,16 @@ export const IssuePropertiesMobileBlockerActions: Story = { render: () => , globals: { viewport: { value: "mobile1" } }, }; + +// Keep preview stories passive. Interaction coverage lives in +// tests/storybook-visual/relationship-badges.spec.ts so switching stories never +// automatically focuses, navigates, removes, or restores a visible badge. +export const IssuePropertiesRelationshipBadges: Story = { + name: "IssueProperties - relationship badges", + render: () => , +}; + +export const IssuePropertiesRelationshipBadgesInline: Story = { + name: "IssueProperties - relationship badges inline", + render: () => , +};