From 5e2243f081726b995f120a78f9b159596d7dcbae Mon Sep 17 00:00:00 2001 From: scotttong Date: Wed, 11 Nov 2026 14:01:22 -0800 Subject: [PATCH] fix(ui): align sidebar avatars and navigation icons (#20926) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The left navigation mixes agent avatars, task status icons, and page icons. > - Agent avatars used 20px boxes. Task and page icons used 16px boxes. > - The different widths moved agent labels 4px to the right of other labels. > - This pull request gives these icons the same 16px square slot. > - Shared label positions make pinned and recent items easier to scan. ## Linked Issues or Issue Description Refs #19685, which added mixed agent and page pins. No duplicate alignment PR was found. **What happened?** Agent avatars in pinned and recent navigation were wider than task and page icons. Their labels started 4px farther to the right. Inline icon layout also produced different box heights. **Expected behavior** Use the existing 16px task and page icon size for all leading navigation icons. Center each icon in a square slot. Start the labels at the same horizontal position. **Steps to reproduce** 1. Pin an agent conversation and a page such as Routines. 2. Open agent conversations and tasks so both appear in Recents. 3. Compare the icon boxes and label positions across the rows. **Paperclip version or commit** Base commit: `c5e6ab5ca813`. **Deployment mode** Local source checkout. This is a client layout issue. ## What Changed - Use 16px agent avatars for pinned and recent conversation rows. - Give `SidebarNavItem` a fixed 16px square icon slot with centered contents and left-aligned text. The existing token scale supplies the size. - Add light and dark Storybook examples with mixed icons and long labels. The browser assertions compare slot sizes, label positions, and glyph bounds. - Add a production `CoordinatorSidebar` regression test that compares pinned and recent avatar dimensions with a task status icon. ## Verification - `pnpm exec vitest run ui/src/components/SidebarNavItem.test.tsx ui/src/components/CoordinatorSidebar.test.tsx ui/src/components/SidebarAgents.test.tsx` passes: 59 tests across three files. - Regression proof: the new production-avatar test fails with the original 20px avatars (`expected 20 to be 16`) and passes with 16px avatars. This addresses the initial review's test coverage gap. - `pnpm check:token-gates` passes. - `pnpm --filter @paperclipai/ui typecheck` passes. - `pnpm --filter @paperclipai/ui build` passes on the final source tree. - `pnpm verify --base origin/master` returns exit 2 because the new Storybook file needs broad CI coverage. The focused local tests run separately. CI must supply the remaining coverage. - Browser inspection on macOS checked all 13 mixed rows in light and dark themes. Every icon slot measured 16 by 16 CSS pixels. Every label started at the same position. Before and after captures use production components with fixture data; they do not prove server persistence or permissions. - Review `sidebar-icon-alignment--dark` and `sidebar-icon-alignment--light` in Storybook to inspect the mixed rows and run their layout assertions. - [Greptile review](https://github.com/paperclipai/paperclip/pull/20926#issuecomment-6806138954): 5/5 on `e779f4707e7826d577b23a2991f3f36755e954be`. The initial 4/5 review requested production Recents coverage. The added regression test addresses that finding; the thread is resolved. - CI queue recovery: the superseded first run was canceled. The next run stalled before runner assignment; GitHub accepted force-cancel but left it queued and refused the rerun request. The final empty commit requests fresh CI without changing the source tree. Required CI is tracked in [run 43095809539](https://github.com/paperclipai/paperclip/actions/runs/43095809539). Its jobs are still queued; CI completion is not yet claimed. - External blocker: after more than 15 minutes, the fresh run still has zero jobs. Other repository PR runs are also queued. A repository Actions or runner administrator must restore dispatch before the required typecheck, test, build, and E2E gates can report results. The root cause of the dispatch delay is not established. ## Risks - Low risk. The shared icon slot also affects other navigation rows. Existing badge, rail, and expanded-state tests cover the shared component. - Agent avatars become smaller in pinned and recent navigation, as requested. No data or API contracts change. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected. This change is a scoped UI bug fix. ## Model Used - OpenAI GPT-6 via Codex. The host does not expose the exact served model snapshot or context window size. Used reasoning, code editing, shell execution, and browser inspection. ## 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 - [ ] 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 Co-authored-by: Paperclip --- ui/src/components/CoordinatorSidebar.test.tsx | 22 +++++ ui/src/components/CoordinatorSidebar.tsx | 4 +- ui/src/components/SidebarNavItem.tsx | 6 +- .../sidebar-icon-alignment.stories.tsx | 88 +++++++++++++++++++ 4 files changed, 115 insertions(+), 5 deletions(-) create mode 100644 ui/storybook/stories/navigation-preferences/sidebar-icon-alignment.stories.tsx diff --git a/ui/src/components/CoordinatorSidebar.test.tsx b/ui/src/components/CoordinatorSidebar.test.tsx index ab03a60606d..5ceda071e89 100644 --- a/ui/src/components/CoordinatorSidebar.test.tsx +++ b/ui/src/components/CoordinatorSidebar.test.tsx @@ -95,6 +95,28 @@ describe("Coordinator task creation", () => { }); describe("Coordinator pinned pages", () => { + it("matches pinned and recent agent avatar dimensions to task status icons", async () => { + fixture.agents = [{ id: "cto", companyId: "company", name: "CTO", urlKey: "cto", role: "cto", status: "idle" } as Agent]; + const chat = { id: "chat-cto", conversationAgentId: "cto", updatedAt: "2026-11-11T12:00:00Z", isUnreadForMe: false }; + fixture.chats = [chat]; + fixture.recentTasks = [{ id: "task", companyId: "company", identifier: "TASK-1", title: "Recent task", status: "todo", recordedAt: 1 }]; + localStorage.setItem("paperclip.coordinator.pinned-nav.v1:reviewer", JSON.stringify(["agent-chat:company:cto"])); + + await renderSidebar(); + + const avatars = [...container.querySelectorAll('a[href="/chats/cto"] [data-slot="agent-avatar"] img')]; + const taskIcon = container.querySelector('a[href="/issues/TASK-1"] [data-slot="sidebar-nav-icon"] svg'); + expect(avatars).toHaveLength(2); + expect(taskIcon).not.toBeNull(); + const width = Number(taskIcon!.getAttribute("width")); + const height = Number(taskIcon!.getAttribute("height")); + expect(width).toBeGreaterThan(0); + expect(height).toBeGreaterThan(0); + for (const avatar of avatars) { + expect(avatar.width).toBe(width); + expect(avatar.height).toBe(height); + } + }); it("builds hundreds of agent pins with input reads proportional to list size", () => { const agents = Array.from({ length: 500 }, (_, index) => ({ id: `agent-${index}`, companyId: "company", name: `Agent ${index}`, urlKey: `agent-${index}` } as Agent)); const pins = agents.map(agent => `agent-chat:company:${agent.id}`).reverse(); diff --git a/ui/src/components/CoordinatorSidebar.tsx b/ui/src/components/CoordinatorSidebar.tsx index d62a88759eb..f2b622121dd 100644 --- a/ui/src/components/CoordinatorSidebar.tsx +++ b/ui/src/components/CoordinatorSidebar.tsx @@ -50,7 +50,7 @@ export function buildPinnedNavigationItems(pins: readonly string[], agents: read const items = new Map(destinations.map(item => [item.to, { key: item.to, to: item.to, label: item.label, icon: }])); for (const agent of agents) { const key = agentChatPinKey(agent.companyId, agent.id); - if (keys.has(key)) items.set(key, { key, to: `/chats/${encodeURIComponent(agentRouteRef(agent))}`, label: agent.name, icon: }); + if (keys.has(key)) items.set(key, { key, to: `/chats/${encodeURIComponent(agentRouteRef(agent))}`, label: agent.name, icon: }); } return pins.flatMap(key => { const item = items.get(key); @@ -199,7 +199,7 @@ export function CoordinatorSidebar({ children }: { children?: ReactNode }) { {recents.map(item => item.kind === "task" ? } /> : item.kind === "chat" - ? } /> + ? } /> : )} } diff --git a/ui/src/components/SidebarNavItem.tsx b/ui/src/components/SidebarNavItem.tsx index 029c25331ba..6c77c4eedd2 100644 --- a/ui/src/components/SidebarNavItem.tsx +++ b/ui/src/components/SidebarNavItem.tsx @@ -39,7 +39,7 @@ interface SidebarNavItemProps { /** * Pre-rendered icon element for rows whose icon isn't a plain Lucide * component (e.g. the agent rows' `AgentIcon`). Takes precedence over `icon`; - * the caller owns its sizing/color classes. + * the caller supplies a 16px icon to fit the shared navigation slot. */ iconNode?: ReactNode; end?: boolean; @@ -132,7 +132,7 @@ export function SidebarNavItem({ // the sidebar edges, rounded-lg matches the card anchor, px-2 gives // the icon breathing room inside the pill. Rows with hover menus // (agents/projects) reserve extra right padding via className. - "flex items-center gap-2.5 mx-2 rounded-lg px-2 py-1.5 pointer-coarse:py-1 text-(length:--text-compact) font-medium transition-colors", + "flex items-center gap-2.5 mx-2 rounded-lg px-2 py-1.5 pointer-coarse:py-1 text-left text-(length:--text-compact) font-medium transition-colors", (active ?? isActive) ? "bg-sidebar-accent text-sidebar-accent-foreground" : "text-foreground/80 hover:bg-sidebar-accent hover:text-sidebar-accent-foreground", @@ -141,7 +141,7 @@ export function SidebarNavItem({ } > {showIconSlot ? ( - + {iconNode ?? (Icon ? : null)} {alert && ( ({ + ...agent, companyId: "company-storybook", urlKey: agent.id, + appearance: appearanceForPalette(palette), +})) as Agent[]; + +const recentItems = [ + { title: "Align sidebar icons and labels", status: "blocked" }, + { agent: agents[0] }, + { title: "Rebuild Needs You overview", status: "blocked" }, + { agent: agents[1] }, + { agent: agents[2] }, + { title: "Fix agent blocked after retry", status: "todo" }, + { title: "Count shadcn components", status: "done" }, + { title: "Diagnose PAP-740 recovery", status: "done" }, + { title: "Status update on recent tasks", status: "done" }, + { agent: agents[3] }, +]; + +function SidebarIconAlignment() { + const [pins, setPins] = useState(["agent-chat:company-storybook:ceo-v2", "agent-chat:company-storybook:glm-coder", "/routines"]); + return ; +} + +const meta = { + id: "sidebar-icon-alignment", + title: "Oversight and operation/Navigation, profile, and announcements/Sidebar icon alignment", + component: SidebarIconAlignment, + parameters: { + layout: "fullscreen", initialEntries: ["/PAP/chats/ceo-v2"], + paperclip: { + capabilities: ["navigation-preferences/preferences"], status: "current", implementation: "production", boundary: "fixtures", + notes: "Production pinned-item builder/list, nav rows, avatar and task glyphs in the Coordinator sidebar surface. Mixed icon types, active state, long labels, reorder and unpin use local fixtures; this does not prove API persistence or authorization.", + }, + }, + play: async ({ canvasElement }) => { + const navigation = within(canvasElement).getByRole("navigation", { name: "Mixed navigation icons" }); + const rows = within(navigation).getAllByRole("link"); + const reference = rows.find(row => row.textContent === "Routines")!.querySelector('[data-slot="sidebar-nav-icon"]')!; + const expectedBox = reference.getBoundingClientRect(); + const expectedLabelX = reference.nextElementSibling!.getBoundingClientRect().left; + for (const row of rows) { + const icon = row.querySelector('[data-slot="sidebar-nav-icon"]')!; + const box = icon.getBoundingClientRect(); + await expect(box.width).toBe(expectedBox.width); + await expect(box.height).toBe(expectedBox.width); + await expect(icon.nextElementSibling!.getBoundingClientRect().left).toBe(expectedLabelX); + const glyph = icon.firstElementChild!.getBoundingClientRect(); + await expect(glyph.width).toBeLessThanOrEqual(box.width); + await expect(glyph.height).toBeLessThanOrEqual(box.height); + } + }, +} satisfies Meta; +export default meta; +type Story = StoryObj; +export const Dark: Story = { globals: { theme: "dark" } }; +export const Light: Story = { globals: { theme: "light" } };