mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-20 06:02:18 +02:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The main navigation and task detail view are core operator surfaces. > - Several controls used different hover states, popover layouts, and spacing rules. > - Recent task actions also needed a compact menu and correct inbox archive behavior. > - These differences made the interface feel inconsistent and caused some content to look crowded or clipped. > - This pull request aligns these surfaces with the Paperclip design tokens and current interaction patterns. > - The benefit is a simpler and more consistent operator experience in light and dark modes. ## Linked Issues or Issue Description **What happened?** Profile and organization popovers used inconsistent layouts. Navigation controls used different hover and selected backgrounds. Task warnings and the composer could crowd nearby content. Archiving a recent task could also remove it from more than the inbox. **Expected behavior** Popover menus should use the same compact visual language. Navigation controls should share readable hover and selected tokens. Task detail content should keep consistent spacing. Archiving should hide a task from the inbox while keeping it in the task list. **Steps to reproduce** 1. Open the main sidebar in light or dark mode. 2. Open the profile and organization menus. 3. Hover navigation items, the organization trigger, the profile trigger, and the feedback flag. 4. Open a task with a warning banner and a long thread. 5. Use the recent task overflow menu and archive a task. **Paperclip version or commit** Reproduced on `master` before this branch. **Deployment mode** Local dev (`pnpm dev`). ## What Changed - Rebuilt the profile and organization popovers with compact token-based layouts. - Matched organization popover width and alignment to the profile popover. - Unified sidebar hover and selected states in light and dark modes. - Added a recent task overflow menu with rename, archive, and pause or restart actions. - Kept archived tasks in the task list while removing them from the inbox. - Improved warning banner and composer spacing in task detail views. - Added and updated focused UI tests for the changed behavior. ## Verification - `pnpm check:token-gates` passed. - `pnpm --filter @paperclipai/ui typecheck` passed. - The seven affected UI test files passed with 216 tests. - `pnpm --filter @paperclipai/ui build` passed. - GitHub CI passed the full build, typecheck and release registry, general test, serialized server, canary dry-run, and end-to-end matrices. - Greptile reviewed commit `6e296be85` at 5/5 with no outstanding actionable findings. ## Risks - Risk is limited to sidebar presentation, recent task actions, and task detail layout. - The recent task archive action now follows inbox-only archive semantics. - No database schema or public API contract changed. > I checked [`ROADMAP.md`](ROADMAP.md). This pull request does not duplicate planned core work. ## Model Used - OpenAI Codex, GPT-5.6. The model used high reasoning, tool use, and code execution. The context window size was 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: Scott Tong <scott@scottsmbpm5max.lan> Co-authored-by: Paperclip <noreply@paperclip.ing>
196 lines
7.7 KiB
TypeScript
196 lines
7.7 KiB
TypeScript
// @vitest-environment jsdom
|
|
|
|
import { act } from "react";
|
|
import type { ReactNode } from "react";
|
|
import { createRoot, type Root } from "react-dom/client";
|
|
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
|
import { Inbox } from "lucide-react";
|
|
import { SidebarNavItem, SidebarNavExpandedProvider } from "./SidebarNavItem";
|
|
import { TooltipProvider } from "@/components/ui/tooltip";
|
|
|
|
const sidebarState = vi.hoisted(() => ({
|
|
isMobile: false,
|
|
setSidebarOpen: () => {},
|
|
collapsed: false,
|
|
peeking: false,
|
|
}));
|
|
|
|
vi.mock("@/lib/router", () => ({
|
|
NavLink: ({ children, to, className, ...props }: {
|
|
children: ReactNode | ((state: { isActive: boolean }) => ReactNode);
|
|
to: string;
|
|
className?: string | ((state: { isActive: boolean }) => string);
|
|
}) => {
|
|
const resolvedClassName = typeof className === "function" ? className({ isActive: false }) : className;
|
|
const resolvedChildren = typeof children === "function" ? children({ isActive: false }) : children;
|
|
return (
|
|
<a href={to} className={resolvedClassName} {...props}>
|
|
{resolvedChildren}
|
|
</a>
|
|
);
|
|
},
|
|
}));
|
|
|
|
vi.mock("../context/SidebarContext", () => ({
|
|
useSidebar: () => sidebarState,
|
|
}));
|
|
|
|
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
|
(globalThis as any).IS_REACT_ACT_ENVIRONMENT = true;
|
|
|
|
describe("SidebarNavItem", () => {
|
|
let container: HTMLDivElement;
|
|
let root: Root;
|
|
|
|
beforeEach(() => {
|
|
sidebarState.collapsed = false;
|
|
sidebarState.peeking = false;
|
|
container = document.createElement("div");
|
|
document.body.appendChild(container);
|
|
root = createRoot(container);
|
|
});
|
|
|
|
afterEach(() => {
|
|
act(() => {
|
|
root.unmount();
|
|
});
|
|
container.remove();
|
|
vi.clearAllMocks();
|
|
});
|
|
|
|
function render(node: ReactNode) {
|
|
act(() => {
|
|
root.render(<TooltipProvider>{node}</TooltipProvider>);
|
|
});
|
|
}
|
|
|
|
function link() {
|
|
return container.querySelector("a") as HTMLAnchorElement;
|
|
}
|
|
|
|
function classTokens(element: Element | null | undefined) {
|
|
return element?.className.toString().split(/\s+/).filter(Boolean) ?? [];
|
|
}
|
|
|
|
it("shows the full label and numeric badge when expanded", () => {
|
|
render(<SidebarNavItem to="/inbox" label="Inbox" icon={Inbox} badge={28} badgeLabel="unread" />);
|
|
|
|
const label = Array.from(container.querySelectorAll("span")).find((el) => el.textContent === "Inbox");
|
|
expect(label?.className).not.toContain("sr-only");
|
|
// The numeric badge is rendered in full (not a dot) and no rail aria-label is set.
|
|
expect(container.textContent).toContain("28");
|
|
expect(link().getAttribute("aria-label")).toBeNull();
|
|
});
|
|
|
|
it("outlines icon alert badges with the streamlined sidebar surface", () => {
|
|
render(<SidebarNavItem to="/inbox" label="Inbox" icon={Inbox} alert />);
|
|
|
|
const alertBadge = container.querySelector('[data-slot="sidebar-icon-alert-badge"]');
|
|
expect(alertBadge).not.toBeNull();
|
|
expect(classTokens(alertBadge)).toContain("shadow-(--shadow-sidebar-icon-badge)");
|
|
expect(classTokens(alertBadge)).not.toContain("shadow-(--shadow-extract-12)");
|
|
});
|
|
|
|
it("omits the icon slot when an item has no icon", () => {
|
|
render(<SidebarNavItem to="/issues/one" label="Recent task" />);
|
|
|
|
expect(link().querySelector('[data-slot="sidebar-nav-icon"]')).toBeNull();
|
|
expect(link().firstElementChild?.textContent).toBe("Recent task");
|
|
});
|
|
|
|
it("uses the sidebar accent surface for the active item", () => {
|
|
render(<SidebarNavItem to="/issues" label="Tasks" icon={Inbox} active />);
|
|
|
|
expect(classTokens(link())).toContain("bg-sidebar-accent");
|
|
expect(classTokens(link())).toContain("text-sidebar-accent-foreground");
|
|
expect(classTokens(link())).not.toContain("bg-background");
|
|
});
|
|
|
|
it("uses the legible sidebar accent surface for hover", () => {
|
|
render(<SidebarNavItem to="/issues" label="Tasks" icon={Inbox} />);
|
|
|
|
expect(classTokens(link())).toContain("hover:bg-sidebar-accent");
|
|
expect(classTokens(link())).toContain("hover:text-sidebar-accent-foreground");
|
|
expect(classTokens(link())).not.toContain("hover:bg-background");
|
|
});
|
|
|
|
it("clips the label (kept in flow for 1:1 row height) and collapses the badge to a dot in the rail", () => {
|
|
sidebarState.collapsed = true;
|
|
render(<SidebarNavItem to="/inbox" label="Inbox" icon={Inbox} badge={28} badgeLabel="unread" />);
|
|
|
|
// The label stays in the DOM/a11y tree (not display:none) so screen readers
|
|
// still announce it. Unlike sr-only it is kept IN FLOW (zero-width, clipped,
|
|
// transparent) so it still contributes its line-height — that keeps the row
|
|
// exactly as tall as the expanded state, so the icon never shifts (PAP-10676).
|
|
const label = Array.from(container.querySelectorAll("span")).find((el) => el.textContent === "Inbox");
|
|
expect(label).toBeTruthy();
|
|
expect(label?.className).not.toContain("sr-only");
|
|
expect(classTokens(label)).toContain("w-0");
|
|
expect(classTokens(label)).toContain("overflow-hidden");
|
|
|
|
// The numeric count is no longer rendered as text; it is a dot with an
|
|
// accessible text equivalent on the link.
|
|
expect(container.textContent).not.toContain("28 ");
|
|
expect(link().getAttribute("aria-label")).toBe("Inbox, 28 unread");
|
|
|
|
// Tooltip wraps the row; the trigger is the wrapper element so the NavLink's
|
|
// own flex className is preserved (PAP-10676), with the <a> nested inside it.
|
|
expect(link().parentElement?.getAttribute("data-slot")).toBe("tooltip-trigger");
|
|
});
|
|
|
|
it("surfaces the trailing status label in the rail aria-label", () => {
|
|
sidebarState.collapsed = true;
|
|
render(
|
|
<SidebarNavItem
|
|
to="/agents/codexcoder"
|
|
label="CodexCoder"
|
|
icon={Inbox}
|
|
trailing={<span aria-label="Invalid reporting chain" />}
|
|
trailingLabel="Invalid reporting chain"
|
|
/>,
|
|
);
|
|
|
|
// The trailing warning is hidden in the rail, so its text equivalent must
|
|
// ride on the link's accessible name.
|
|
expect(link().getAttribute("aria-label")).toBe("CodexCoder, Invalid reporting chain");
|
|
});
|
|
|
|
it("keeps the full presentation while peeking even when collapsed", () => {
|
|
sidebarState.collapsed = true;
|
|
sidebarState.peeking = true;
|
|
render(<SidebarNavItem to="/inbox" label="Inbox" icon={Inbox} badge={28} badgeLabel="unread" />);
|
|
|
|
const label = Array.from(container.querySelectorAll("span")).find((el) => el.textContent === "Inbox");
|
|
expect(label?.className).not.toContain("sr-only");
|
|
expect(container.textContent).toContain("28");
|
|
expect(link().getAttribute("aria-label")).toBeNull();
|
|
});
|
|
|
|
it("forces the full label inside an expanded contextual pane even when globally collapsed", () => {
|
|
// The takeover preserves the user's saved global collapse preference while
|
|
// replacing its contents. The provider keeps the contextual navigation
|
|
// readable without changing that preference.
|
|
sidebarState.collapsed = true;
|
|
render(
|
|
<SidebarNavExpandedProvider>
|
|
<SidebarNavItem to="/inbox" label="Inbox" icon={Inbox} badge={28} badgeLabel="unread" />
|
|
</SidebarNavExpandedProvider>,
|
|
);
|
|
|
|
const label = Array.from(container.querySelectorAll("span")).find((el) => el.textContent === "Inbox");
|
|
expect(classTokens(label)).not.toContain("w-0");
|
|
expect(classTokens(label)).toContain("flex-1");
|
|
// Full numeric badge, no rail aria-label, no tooltip wrapper.
|
|
expect(container.textContent).toContain("28");
|
|
expect(link().getAttribute("aria-label")).toBeNull();
|
|
expect(link().parentElement?.getAttribute("data-slot")).not.toBe("tooltip-trigger");
|
|
});
|
|
|
|
it("surfaces the live count in the rail aria-label", () => {
|
|
sidebarState.collapsed = true;
|
|
render(<SidebarNavItem to="/dashboard" label="Dashboard" icon={Inbox} liveCount={3} />);
|
|
|
|
expect(link().getAttribute("aria-label")).toBe("Dashboard, 3 live");
|
|
});
|
|
});
|