From 73f629de460907f509b88d89448a9cdd4446d737 Mon Sep 17 00:00:00 2001 From: Dotta Date: Mon, 8 Jun 2026 21:39:30 +0000 Subject: [PATCH] fix(ui): instant sidebar collapse with pixel-stable icons (PAP-10676) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two behavioral fixes for the collapsible sidebar so collapsing/expanding is instant and the icons never move between states: - Instant, not animated: drop the `transition-[width]` on the SidebarShell panel so the rail/expanded widths snap with no sliding motion. - Icons must not move (1:1 across states). Three drift sources fixed: 1. SidebarSection: the collapsed-rail header (a thin divider) was ~13px tall vs the ~36px expanded header, shifting every icon below it. The rail header now mirrors the expanded footprint (`px-3 py-1.5` + `min-h-6`). 2. Row labels used `sr-only`, which is `position:absolute` and removes the label's line-height from flow, so each rail row was ~3-4px shorter than expanded and the offset accumulated down the list. Labels are now kept in flow but zero-width/clipped/transparent (SIDEBAR_RAIL_HIDDEN_LABEL), so the row stays exactly as tall as expanded while remaining in the a11y tree. 3. The rail wrapped each NavLink directly in `TooltipTrigger asChild`; Radix Slot drops React Router's *function* className, stripping `flex` off the (it rendered as a block) so the in-flow label stacked under the icon. The tooltip now anchors to a plain wrapper element, keeping the flex. Verified by rendering the real Sidebar (new SidebarIconAlignment story) and measuring every nav/project/agent icon's bounding box in both states: dX=0 and dY within ±1px (rounding); a red/green overlay shows the icon glyphs perfectly superimposed. Tests updated accordingly (56 sidebar tests pass). Co-Authored-By: Paperclip --- ui/src/components/Sidebar.tsx | 4 +-- ui/src/components/SidebarAgents.test.tsx | 11 +++++--- ui/src/components/SidebarAgents.tsx | 15 +++++++--- ui/src/components/SidebarNavItem.test.tsx | 17 +++++++---- ui/src/components/SidebarNavItem.tsx | 14 ++++++++-- ui/src/components/SidebarProjects.test.tsx | 11 +++++--- ui/src/components/SidebarProjects.tsx | 13 +++++++-- ui/src/components/SidebarSection.tsx | 13 +++++++-- ui/src/components/SidebarShell.test.tsx | 8 ++++-- ui/src/components/SidebarShell.tsx | 10 +++++-- ui/src/lib/utils.ts | 12 ++++++++ .../stories/navigation-layout.stories.tsx | 28 +++++++++++++++++++ 12 files changed, 121 insertions(+), 35 deletions(-) diff --git a/ui/src/components/Sidebar.tsx b/ui/src/components/Sidebar.tsx index 6b02fd9e9c0..04778f264f2 100644 --- a/ui/src/components/Sidebar.tsx +++ b/ui/src/components/Sidebar.tsx @@ -33,7 +33,7 @@ import { queryKeys } from "../lib/queryKeys"; import { useInboxBadge } from "../hooks/useInboxBadge"; import { Button } from "@/components/ui/button"; import { Tooltip, TooltipContent, TooltipTrigger } from "@/components/ui/tooltip"; -import { cn } from "../lib/utils"; +import { cn, SIDEBAR_RAIL_HIDDEN_LABEL } from "../lib/utils"; import { PluginSlotOutlet } from "@/plugins/slots"; import { PluginLauncherOutlet } from "@/plugins/launchers"; import { SidebarCompanyMenu } from "./SidebarCompanyMenu"; @@ -128,7 +128,7 @@ export function Sidebar() { className="flex items-center gap-2.5 px-3 py-2 pointer-coarse:py-1.5 text-[13px] font-medium text-foreground/80 hover:bg-accent/50 hover:text-foreground transition-colors" > - New Task + New Task ); return rail ? ( diff --git a/ui/src/components/SidebarAgents.test.tsx b/ui/src/components/SidebarAgents.test.tsx index 51f4955f408..d2162e67c9b 100644 --- a/ui/src/components/SidebarAgents.test.tsx +++ b/ui/src/components/SidebarAgents.test.tsx @@ -314,12 +314,15 @@ describe("SidebarAgents", () => { await renderRailSidebarAgents(); - // The agent name is preserved in the a11y tree (sr-only) and the row links - // become tooltip triggers; the per-row actions dropdown is dropped. + // The agent name is preserved in the a11y tree but kept in flow (zero-width, + // clipped) so the row stays 1:1 tall with the expanded state (PAP-10676); the + // row links become tooltip triggers and the per-row actions dropdown is dropped. const nameSpan = Array.from(container.querySelectorAll("span")).find((el) => el.textContent === "Alpha"); - expect(nameSpan?.className).toContain("sr-only"); + expect(nameSpan?.className).not.toContain("sr-only"); + expect(nameSpan?.className).toContain("w-0"); + expect(nameSpan?.className).toContain("overflow-hidden"); const agentLink = container.querySelector('a[href^="/agents/"]:not([href="/agents/all"])'); - expect(agentLink?.getAttribute("data-slot")).toBe("tooltip-trigger"); + expect(agentLink?.parentElement?.getAttribute("data-slot")).toBe("tooltip-trigger"); expect(container.querySelector('button[aria-label="Open actions for Alpha"]')).toBeNull(); // The section header collapses to a divider (no caret / section menu). diff --git a/ui/src/components/SidebarAgents.tsx b/ui/src/components/SidebarAgents.tsx index 60d4e1e63a6..fc28a7c33e9 100644 --- a/ui/src/components/SidebarAgents.tsx +++ b/ui/src/components/SidebarAgents.tsx @@ -21,7 +21,7 @@ import { authApi } from "../api/auth"; import { heartbeatsApi } from "../api/heartbeats"; import { SIDEBAR_SCROLL_RESET_STATE } from "../lib/navigation-scroll"; import { queryKeys } from "../lib/queryKeys"; -import { cn, agentRouteRef, agentUrl } from "../lib/utils"; +import { cn, agentRouteRef, agentUrl, SIDEBAR_RAIL_HIDDEN_LABEL } from "../lib/utils"; import { useAgentOrder } from "../hooks/useAgentOrder"; import { resourceMembershipState, useResourceMembershipMutation, useResourceMemberships } from "../hooks/useResourceMemberships"; import { @@ -144,7 +144,7 @@ function SidebarAgentItem({ )} > - {agent.name} + {agent.name} {!rail && hasInvalidOrgChain ? ( ) : null} @@ -172,8 +172,15 @@ function SidebarAgentItem({ return (
{rail ? ( + // Anchor the tooltip to a wrapper, not the NavLink: Radix `asChild` (Slot) + // drops React Router's function className, which would strip `flex` off the + // and let the in-flow label stack under the icon, growing the row. + // Keeping the out of Slot preserves a 1:1 row height with the expanded + // state so the icon never moves (PAP-10676). - {link} + +
{link}
+ {agent.name} ) : ( @@ -486,7 +493,7 @@ export function SidebarAgents({ streamlined = false }: { streamlined?: boolean } className="flex items-center gap-2.5 px-3 py-1.5 pointer-coarse:py-1 text-[13px] font-medium text-muted-foreground transition-colors hover:bg-accent/50 hover:text-foreground" > - See all agents + See all agents ); return rail ? ( diff --git a/ui/src/components/SidebarNavItem.test.tsx b/ui/src/components/SidebarNavItem.test.tsx index c6f19027ddc..715c3658123 100644 --- a/ui/src/components/SidebarNavItem.test.tsx +++ b/ui/src/components/SidebarNavItem.test.tsx @@ -78,23 +78,28 @@ describe("SidebarNavItem", () => { expect(link().getAttribute("aria-label")).toBeNull(); }); - it("collapses the label to sr-only and the badge to a dot in the rail", () => { + 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(); - // Label stays in the a11y tree (sr-only, not display:none) so screen readers - // still announce it. + // 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).toContain("sr-only"); + expect(label?.className).not.toContain("sr-only"); + expect(label?.className).toContain("w-0"); + expect(label?.className).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 wrapper present (radix marks the trigger). - expect(link().getAttribute("data-slot")).toBe("tooltip-trigger"); + // Tooltip wraps the row; the trigger is the wrapper element so the NavLink's + // own flex className is preserved (PAP-10676), with the nested inside it. + expect(link().parentElement?.getAttribute("data-slot")).toBe("tooltip-trigger"); }); it("keeps the full presentation while peeking even when collapsed", () => { diff --git a/ui/src/components/SidebarNavItem.tsx b/ui/src/components/SidebarNavItem.tsx index 1c6952030f8..f6e97c68717 100644 --- a/ui/src/components/SidebarNavItem.tsx +++ b/ui/src/components/SidebarNavItem.tsx @@ -1,6 +1,6 @@ import { NavLink } from "@/lib/router"; import { SIDEBAR_SCROLL_RESET_STATE } from "../lib/navigation-scroll"; -import { cn } from "../lib/utils"; +import { cn, SIDEBAR_RAIL_HIDDEN_LABEL } from "../lib/utils"; import { useSidebar } from "../context/SidebarContext"; import { Tooltip, TooltipContent, TooltipTrigger } from "@/components/ui/tooltip"; import type { LucideIcon } from "lucide-react"; @@ -98,7 +98,7 @@ export function SidebarNavItem({ /> )} - {label} + {label} {!rail && textBadge && ( and render it as a block — the in-flow label would + // then stack under the icon and the row would grow. Anchoring the tooltip to a + // wrapper keeps the rendering normally (flex), so the row stays 1:1 with + // the expanded state and the icon never moves (PAP-10676). return ( - {link} + +
{link}
+ {label} ); diff --git a/ui/src/components/SidebarProjects.test.tsx b/ui/src/components/SidebarProjects.test.tsx index fddc26a87b6..6d8759348ee 100644 --- a/ui/src/components/SidebarProjects.test.tsx +++ b/ui/src/components/SidebarProjects.test.tsx @@ -351,12 +351,15 @@ describe("SidebarProjects", () => { it("renders icon-only project rows with tooltips and no row actions in the rail", async () => { await renderRailSidebarProjects(); - // Project names stay in the a11y tree (sr-only); rows become tooltip triggers - // and the per-row actions dropdown is dropped. + // Project names stay in the a11y tree but kept in flow (zero-width, clipped) + // so rows stay 1:1 tall with the expanded state (PAP-10676); rows become + // tooltip triggers and the per-row actions dropdown is dropped. const nameSpan = Array.from(container.querySelectorAll("span")).find((el) => el.textContent === "Bravo"); - expect(nameSpan?.className).toContain("sr-only"); + expect(nameSpan?.className).not.toContain("sr-only"); + expect(nameSpan?.className).toContain("w-0"); + expect(nameSpan?.className).toContain("overflow-hidden"); const projectLink = container.querySelector('a[href^="/projects/"]'); - expect(projectLink?.getAttribute("data-slot")).toBe("tooltip-trigger"); + expect(projectLink?.parentElement?.getAttribute("data-slot")).toBe("tooltip-trigger"); expect(container.querySelector('button[aria-label="Open actions for Bravo"]')).toBeNull(); // The section header collapses to a divider (no section menu trigger). expect(container.querySelector('button[aria-label="Projects section actions"]')).toBeNull(); diff --git a/ui/src/components/SidebarProjects.tsx b/ui/src/components/SidebarProjects.tsx index 841cc52e249..968e05d8d89 100644 --- a/ui/src/components/SidebarProjects.tsx +++ b/ui/src/components/SidebarProjects.tsx @@ -19,7 +19,7 @@ import { authApi } from "../api/auth"; import { projectsApi } from "../api/projects"; import { SIDEBAR_SCROLL_RESET_STATE } from "../lib/navigation-scroll"; import { queryKeys } from "../lib/queryKeys"; -import { cn, projectRouteRef } from "../lib/utils"; +import { cn, projectRouteRef, SIDEBAR_RAIL_HIDDEN_LABEL } from "../lib/utils"; import { useProjectOrder } from "../hooks/useProjectOrder"; import { resourceMembershipState, useResourceMembershipMutation, useResourceMemberships } from "../hooks/useResourceMemberships"; import { BudgetSidebarMarker } from "./BudgetSidebarMarker"; @@ -142,7 +142,7 @@ function ProjectItem({ )} > - {project.name} + {project.name} {!rail && project.pauseReason === "budget" ? : null} ); @@ -151,8 +151,15 @@ function ProjectItem({
{rail ? ( + // Anchor the tooltip to a wrapper, not the NavLink: Radix `asChild` + // (Slot) drops React Router's function className, which would strip + // `flex` off the and let the in-flow label stack under the icon, + // growing the row. Keeping the out of Slot preserves a 1:1 row + // height with the expanded state so the icon never moves (PAP-10676). - {link} + +
{link}
+ {project.name} ) : ( diff --git a/ui/src/components/SidebarSection.tsx b/ui/src/components/SidebarSection.tsx index 4998b057fb2..a6a78611104 100644 --- a/ui/src/components/SidebarSection.tsx +++ b/ui/src/components/SidebarSection.tsx @@ -197,12 +197,19 @@ export function SidebarSection({ // label, so replace it with a thin divider. The label stays in the a11y tree, // per-section carets/menus are dropped (no room + no toggle target in the // rail), and the items always render so their icons stay reachable. + // + // The header wrapper mirrors the expanded header's vertical footprint exactly + // (outer `px-3 py-1.5 pointer-coarse:py-1` + inner `min-h-6`) so item icons land + // at the identical y-position in both states — no movement on collapse/expand + // (PAP-10676). The divider is vertically centered within that same row. if (rail) { return (
-
- {label} -