mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-25 09:41:04 +01:00
fix(ui): full-label settings nav in the collapsed-takeover secondary pane (PAP-10700)
The takeover model collapses the app <Sidebar/> to its 64px rail and renders the contextual nav in a fixed 240px SecondarySidebar. Its SidebarNavItem children read the *global* useSidebar().collapsed (true during takeover), so they rendered icon-only with their label clipped to w-0/text-transparent — leaving the settings nav unreadable unless the user pinned the sidebar expanded. Add a SidebarNavExpandedProvider context that forces the full-label presentation regardless of the global rail collapse, and have SecondarySidebar wrap its children in it (the pane is always wide enough for labels). Covered by a SidebarNavItem unit test and an e2e assertion that the secondary-pane labels render at full width in the default collapsed state. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
1 parent
05951f20ae
commit
9ca8cfd15d
4 files changed
+77
-4
No files matched your search
@@ -97,6 +97,28 @@ test.describe("Sidebar takeover (collapse + secondary pane)", () => {
|
||||
await expect(page.getByLabel(APP_SIDEBAR_EXPANDED_MARKER)).toHaveCount(0);
|
||||
});
|
||||
|
||||
test("renders the secondary pane nav labels at full width despite the app rail collapse", async ({ page }) => {
|
||||
// Regression (PAP-10700): the secondary pane is 240px wide, but its
|
||||
// SidebarNavItem children read the *global* collapsed state and used to
|
||||
// render icon-only (label `w-0 text-transparent`), making the settings nav
|
||||
// unreadable in the default takeover state. The pane must force full labels.
|
||||
await page.goto(`/${prefix}/company/settings`);
|
||||
|
||||
const secondary = page.locator("[data-secondary-sidebar]");
|
||||
await expect(secondary).toBeVisible();
|
||||
|
||||
// App sidebar is collapsed to its rail (default unpinned takeover state)...
|
||||
await expect(page.getByLabel(APP_SIDEBAR_EXPANDED_MARKER)).toHaveCount(0);
|
||||
|
||||
// ...yet a settings nav label renders at its full text width, not clipped to
|
||||
// zero. "Environments" is unique to the company-settings nav.
|
||||
const envLabel = secondary.getByText("Environments", { exact: true });
|
||||
await expect(envLabel).toBeVisible();
|
||||
const labelBox = await envLabel.boundingBox();
|
||||
expect(labelBox).not.toBeNull();
|
||||
expect(labelBox!.width).toBeGreaterThan(20);
|
||||
});
|
||||
|
||||
test("an explicit expanded pin overrides the route-driven collapse", async ({ page }) => {
|
||||
// User has pinned the sidebar expanded ("0"). The settings route still
|
||||
// requests a collapse, but the pin must win (pin > route request > default).
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { type ReactNode } from "react";
|
||||
import { cn } from "@/lib/utils";
|
||||
import { SidebarNavExpandedProvider } from "./SidebarNavItem";
|
||||
|
||||
/**
|
||||
* Fixed-width contextual pane that sits between the collapsed app rail and the
|
||||
@@ -12,6 +13,10 @@ import { cn } from "@/lib/utils";
|
||||
* It is a dumb container — fixed `w-60`, full-height, non-shrinking, with a
|
||||
* right border and its own vertical scroll — so callers just drop the
|
||||
* contextual nav (or a plugin slot mount) inside as `children`.
|
||||
*
|
||||
* Because the pane is always 240px wide it forces the full-label presentation
|
||||
* on any `SidebarNavItem` inside it, so the contextual nav stays readable even
|
||||
* while the app sidebar is collapsed to its rail (PAP-10700).
|
||||
*/
|
||||
export function SecondarySidebar({
|
||||
children,
|
||||
@@ -28,7 +33,7 @@ export function SecondarySidebar({
|
||||
className,
|
||||
)}
|
||||
>
|
||||
{children}
|
||||
<SidebarNavExpandedProvider>{children}</SidebarNavExpandedProvider>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
@@ -5,7 +5,7 @@ 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 } from "./SidebarNavItem";
|
||||
import { SidebarNavItem, SidebarNavExpandedProvider } from "./SidebarNavItem";
|
||||
import { TooltipProvider } from "@/components/ui/tooltip";
|
||||
|
||||
const sidebarState = vi.hoisted(() => ({
|
||||
@@ -113,6 +113,26 @@ describe("SidebarNavItem", () => {
|
||||
expect(link().getAttribute("aria-label")).toBeNull();
|
||||
});
|
||||
|
||||
it("forces the full label inside an expanded contextual pane even when globally collapsed", () => {
|
||||
// The takeover model collapses the global sidebar (collapsed=true) while the
|
||||
// 240px SecondarySidebar still needs readable labels (PAP-10700). The
|
||||
// provider must override the global rail collapse.
|
||||
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(label?.className).not.toContain("w-0");
|
||||
expect(label?.className).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} />);
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import { createContext, useContext, type ReactNode } from "react";
|
||||
import { NavLink } from "@/lib/router";
|
||||
import { SIDEBAR_SCROLL_RESET_STATE } from "../lib/navigation-scroll";
|
||||
import { cn, SIDEBAR_RAIL_HIDDEN_LABEL } from "../lib/utils";
|
||||
@@ -5,6 +6,27 @@ import { useSidebar } from "../context/SidebarContext";
|
||||
import { Tooltip, TooltipContent, TooltipTrigger } from "@/components/ui/tooltip";
|
||||
import type { LucideIcon } from "lucide-react";
|
||||
|
||||
/**
|
||||
* Forces the full-label (non-rail) presentation for any `SidebarNavItem`
|
||||
* rendered beneath it, regardless of the global `useSidebar().collapsed` state.
|
||||
*
|
||||
* Takeover routes (PAP-10695) collapse the app `<Sidebar/>` to its 64px rail
|
||||
* and render the contextual nav in a fixed 240px `SecondarySidebar`. That pane
|
||||
* is always wide enough for labels, but its `SidebarNavItem` children still
|
||||
* read the *global* `collapsed=true` and would otherwise render icon-only —
|
||||
* leaving the settings nav unreadable (PAP-10700). Wrapping the pane in this
|
||||
* provider decouples its items from the global rail collapse.
|
||||
*/
|
||||
const SidebarNavExpandedContext = createContext(false);
|
||||
|
||||
export function SidebarNavExpandedProvider({ children }: { children: ReactNode }) {
|
||||
return (
|
||||
<SidebarNavExpandedContext.Provider value={true}>
|
||||
{children}
|
||||
</SidebarNavExpandedContext.Provider>
|
||||
);
|
||||
}
|
||||
|
||||
interface SidebarNavItemProps {
|
||||
to: string;
|
||||
label: string;
|
||||
@@ -39,9 +61,13 @@ export function SidebarNavItem({
|
||||
liveCount,
|
||||
}: SidebarNavItemProps) {
|
||||
const { isMobile, setSidebarOpen, collapsed, peeking } = useSidebar();
|
||||
// A fixed-width contextual pane (SecondarySidebar) forces full labels even
|
||||
// when the global app sidebar is collapsed to its rail (PAP-10700).
|
||||
const forceExpanded = useContext(SidebarNavExpandedContext);
|
||||
// The icon-only rail presentation only applies when pinned collapsed and not
|
||||
// peeking; a peek/expanded panel restores the full label + badge.
|
||||
const rail = collapsed && !peeking;
|
||||
// peeking; a peek/expanded panel — or an expanded contextual pane — restores
|
||||
// the full label + badge.
|
||||
const rail = collapsed && !peeking && !forceExpanded;
|
||||
|
||||
const hasBadge = badge != null && badge > 0;
|
||||
const hasLive = liveCount != null && liveCount > 0;
|
||||
|
||||
Reference in new issue
Block a user