mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-27 19:35:13 +01:00
fix(ui): instant sidebar collapse with pixel-stable icons (PAP-10676)
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
<a> (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 <a> 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 <noreply@paperclip.ing>
This commit is contained in:
1 parent
e666173395
commit
73f629de46
12 files changed
+121
-35
No files matched your search
@@ -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"
|
||||
>
|
||||
<SquarePen className="h-4 w-4 shrink-0" />
|
||||
<span className={cn("truncate", rail && "sr-only")}>New Task</span>
|
||||
<span className={rail ? SIDEBAR_RAIL_HIDDEN_LABEL : "truncate"}>New Task</span>
|
||||
</button>
|
||||
);
|
||||
return rail ? (
|
||||
|
||||
@@ -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).
|
||||
|
||||
@@ -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({
|
||||
)}
|
||||
>
|
||||
<AgentIcon icon={agent.icon} className="shrink-0 h-3.5 w-3.5 text-muted-foreground" />
|
||||
<span className={cn("flex-1 truncate", rail && "sr-only")}>{agent.name}</span>
|
||||
<span className={rail ? SIDEBAR_RAIL_HIDDEN_LABEL : "flex-1 truncate"}>{agent.name}</span>
|
||||
{!rail && hasInvalidOrgChain ? (
|
||||
<AlertTriangle className="h-3.5 w-3.5 shrink-0 text-amber-500" aria-label="Invalid reporting chain" />
|
||||
) : null}
|
||||
@@ -172,8 +172,15 @@ function SidebarAgentItem({
|
||||
return (
|
||||
<div className="group/agent relative flex items-center">
|
||||
{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
|
||||
// <a> and let the in-flow label stack under the icon, growing the row.
|
||||
// Keeping the <a> out of Slot preserves a 1:1 row height with the expanded
|
||||
// state so the icon never moves (PAP-10676).
|
||||
<Tooltip>
|
||||
<TooltipTrigger asChild>{link}</TooltipTrigger>
|
||||
<TooltipTrigger asChild>
|
||||
<div className="min-w-0 flex-1">{link}</div>
|
||||
</TooltipTrigger>
|
||||
<TooltipContent side="right">{agent.name}</TooltipContent>
|
||||
</Tooltip>
|
||||
) : (
|
||||
@@ -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"
|
||||
>
|
||||
<Users className="shrink-0 h-3.5 w-3.5" />
|
||||
<span className={cn(rail && "sr-only")}>See all agents</span>
|
||||
<span className={rail ? SIDEBAR_RAIL_HIDDEN_LABEL : undefined}>See all agents</span>
|
||||
</Link>
|
||||
);
|
||||
return rail ? (
|
||||
|
||||
@@ -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(<SidebarNavItem to="/inbox" label="Inbox" icon={Inbox} badge={28} badgeLabel="unread" />);
|
||||
|
||||
// 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 <a> nested inside it.
|
||||
expect(link().parentElement?.getAttribute("data-slot")).toBe("tooltip-trigger");
|
||||
});
|
||||
|
||||
it("keeps the full presentation while peeking even when collapsed", () => {
|
||||
|
||||
@@ -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({
|
||||
/>
|
||||
)}
|
||||
</span>
|
||||
<span className={cn("flex-1 truncate", rail && "sr-only")}>{label}</span>
|
||||
<span className={rail ? SIDEBAR_RAIL_HIDDEN_LABEL : "flex-1 truncate"}>{label}</span>
|
||||
{!rail && textBadge && (
|
||||
<span
|
||||
className={cn(
|
||||
@@ -137,9 +137,17 @@ export function SidebarNavItem({
|
||||
|
||||
if (!rail) return link;
|
||||
|
||||
// The tooltip wraps a plain block element rather than the NavLink directly:
|
||||
// Radix `asChild` (Slot) drops React Router's *function* className, which would
|
||||
// strip `flex` off the <a> 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 <a> rendering normally (flex), so the row stays 1:1 with
|
||||
// the expanded state and the icon never moves (PAP-10676).
|
||||
return (
|
||||
<Tooltip>
|
||||
<TooltipTrigger asChild>{link}</TooltipTrigger>
|
||||
<TooltipTrigger asChild>
|
||||
<div>{link}</div>
|
||||
</TooltipTrigger>
|
||||
<TooltipContent side="right">{label}</TooltipContent>
|
||||
</Tooltip>
|
||||
);
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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({
|
||||
)}
|
||||
>
|
||||
<ProjectTile color={project.color ?? null} icon={project.icon ?? null} size="xs" />
|
||||
<span className={cn("flex-1 truncate", rail && "sr-only")}>{project.name}</span>
|
||||
<span className={rail ? SIDEBAR_RAIL_HIDDEN_LABEL : "flex-1 truncate"}>{project.name}</span>
|
||||
{!rail && project.pauseReason === "budget" ? <BudgetSidebarMarker title="Project paused by budget" /> : null}
|
||||
</NavLink>
|
||||
);
|
||||
@@ -151,8 +151,15 @@ function ProjectItem({
|
||||
<div className="flex flex-col gap-0.5">
|
||||
<div className="group/project relative flex items-center">
|
||||
{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 <a> and let the in-flow label stack under the icon,
|
||||
// growing the row. Keeping the <a> out of Slot preserves a 1:1 row
|
||||
// height with the expanded state so the icon never moves (PAP-10676).
|
||||
<Tooltip>
|
||||
<TooltipTrigger asChild>{link}</TooltipTrigger>
|
||||
<TooltipTrigger asChild>
|
||||
<div className="min-w-0 flex-1">{link}</div>
|
||||
</TooltipTrigger>
|
||||
<TooltipContent side="right">{project.name}</TooltipContent>
|
||||
</Tooltip>
|
||||
) : (
|
||||
|
||||
@@ -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 (
|
||||
<div>
|
||||
<div className="px-3 py-1.5">
|
||||
<span className="sr-only">{label}</span>
|
||||
<div className="h-px bg-border/60" aria-hidden="true" />
|
||||
<div className="px-3 py-1.5 pointer-coarse:py-1">
|
||||
<div className="flex min-h-6 items-center">
|
||||
<span className="sr-only">{label}</span>
|
||||
<div className="h-px w-full bg-border/60" aria-hidden="true" />
|
||||
</div>
|
||||
</div>
|
||||
{content}
|
||||
</div>
|
||||
|
||||
@@ -167,7 +167,7 @@ describe("SidebarShell", () => {
|
||||
expect(panel().className).toContain("z-30");
|
||||
});
|
||||
|
||||
it("drops the width transition for reduced-motion users", () => {
|
||||
it("opens and closes instantly with no width transition (PAP-10676)", () => {
|
||||
act(() => {
|
||||
root.render(
|
||||
<SidebarShell open>
|
||||
@@ -176,7 +176,9 @@ describe("SidebarShell", () => {
|
||||
);
|
||||
});
|
||||
|
||||
expect(panel().className).toContain("transition-[width]");
|
||||
expect(panel().className).toContain("motion-reduce:transition-none");
|
||||
// Open/close must be instant: the panel never animates its width, so neither
|
||||
// the transition nor its reduced-motion fallback should be present.
|
||||
expect(panel().className).not.toContain("transition-[width]");
|
||||
expect(panel().className).not.toContain("motion-reduce:transition-none");
|
||||
});
|
||||
});
|
||||
@@ -79,8 +79,12 @@ type SidebarShellProps = {
|
||||
* is wider than the reserved spacer.
|
||||
*
|
||||
* The icon-alignment guarantee comes for free: collapsing is a pure width
|
||||
* animation with `overflow-hidden` clipping the labels; item padding/icon markup
|
||||
* change with `overflow-hidden` clipping the labels; item padding/icon markup
|
||||
* are never touched, so the left-aligned icon stays pixel-identical.
|
||||
*
|
||||
* Opening/closing is intentionally instant (no width transition): the pin toggle
|
||||
* and peek snap between rail and expanded widths so the content never appears to
|
||||
* slide. Resizing the drag handle is likewise direct.
|
||||
*/
|
||||
export function SidebarShell({
|
||||
children,
|
||||
@@ -193,8 +197,8 @@ export function SidebarShell({
|
||||
<div
|
||||
className={cn(
|
||||
"absolute inset-y-0 left-0 flex flex-col overflow-hidden",
|
||||
// Reuse the existing fast width animation; reduced-motion drops it.
|
||||
!isResizing && "transition-[width] duration-100 ease-out motion-reduce:transition-none",
|
||||
// Open/close is instant (PAP-10676): no width transition so the rail and
|
||||
// expanded states snap without any sliding motion.
|
||||
// Overlay styling only while the panel is wider than its reserved
|
||||
// spacer (i.e. peeking) so it floats above content without reflow.
|
||||
isOverlay
|
||||
|
||||
@@ -7,6 +7,18 @@ export function cn(...inputs: ClassValue[]) {
|
||||
return twMerge(clsx(inputs));
|
||||
}
|
||||
|
||||
/**
|
||||
* Classes for a sidebar row label when the sidebar is collapsed to the icon rail
|
||||
* (PAP-10676). Unlike `sr-only` (which is `position: absolute` and therefore
|
||||
* removes the label from flow), this keeps the label in flow so it still
|
||||
* contributes its line-height to the row. That guarantees a row is the *exact*
|
||||
* same height collapsed as expanded, so the icons never shift vertically between
|
||||
* states. The label is clipped to zero visible width and rendered transparent,
|
||||
* but stays in the DOM and the a11y tree as the link's accessible name.
|
||||
*/
|
||||
export const SIDEBAR_RAIL_HIDDEN_LABEL =
|
||||
"block w-0 min-w-0 overflow-hidden whitespace-nowrap text-transparent select-none";
|
||||
|
||||
export function asObject(value: unknown): Record<string, unknown> {
|
||||
return typeof value === "object" && value !== null && !Array.isArray(value)
|
||||
? value as Record<string, unknown>
|
||||
|
||||
@@ -16,6 +16,7 @@ import { KeyboardShortcutsCheatsheetContent } from "@/components/KeyboardShortcu
|
||||
import { MobileBottomNav } from "@/components/MobileBottomNav";
|
||||
import { PageTabBar } from "@/components/PageTabBar";
|
||||
import { Sidebar } from "@/components/Sidebar";
|
||||
import { PluginLauncherProvider } from "@/plugins/launchers";
|
||||
import { SidebarAccountMenu } from "@/components/SidebarAccountMenu";
|
||||
import { SidebarCompanyMenu } from "@/components/SidebarCompanyMenu";
|
||||
import { StatusBadge } from "@/components/StatusBadge";
|
||||
@@ -349,3 +350,30 @@ export default meta;
|
||||
type Story = StoryObj<typeof meta>;
|
||||
|
||||
export const BoardChromeMatrix: Story = {};
|
||||
|
||||
// PAP-10676 verification harness: renders the real Sidebar at a fixed width so a
|
||||
// screenshot of the expanded state and a screenshot of the pinned-collapsed rail
|
||||
// can be overlaid. The icon column must be pixel-identical between the two — the
|
||||
// only difference should be the labels (sr-only in the rail). Playwright toggles
|
||||
// collapse via the in-sidebar control, so this exercises the real context path.
|
||||
function SidebarIconAlignmentHarness() {
|
||||
return (
|
||||
<PluginLauncherProvider>
|
||||
<div className="paperclip-story">
|
||||
<RouteSetter to="/PAP/projects/board-ui/issues" />
|
||||
<div className="flex min-h-[760px] items-start justify-center bg-muted/30 p-8">
|
||||
<div
|
||||
data-testid="sidebar-align-frame"
|
||||
className="h-[700px] w-60 overflow-hidden border border-border bg-background"
|
||||
>
|
||||
<Sidebar />
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
</PluginLauncherProvider>
|
||||
);
|
||||
}
|
||||
|
||||
export const SidebarIconAlignment: Story = {
|
||||
render: () => <SidebarIconAlignmentHarness />,
|
||||
};
|
||||
Reference in new issue
Block a user