mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-27 19:35:13 +01:00
fix(ui): align sidebar takeover review feedback
This commit is contained in:
1 parent
d5627dc442
commit
ee9d48a3cd
5 files changed
+50
-7
No files matched your search
@@ -119,9 +119,10 @@ test.describe("Sidebar takeover (collapse + secondary pane)", () => {
|
||||
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).
|
||||
test("settings force-collapse overrides an expanded pin without mutating it", async ({ page }) => {
|
||||
// User has pinned the sidebar expanded ("0"). Company settings is a hard
|
||||
// secondary-sidebar takeover route, so forceCollapsed wins while the route is
|
||||
// active (force > pin > route request > default) but must not mutate the pin.
|
||||
await page.addInitScript(
|
||||
({ key }) => {
|
||||
window.localStorage.setItem(key, "0");
|
||||
@@ -134,8 +135,14 @@ test.describe("Sidebar takeover (collapse + secondary pane)", () => {
|
||||
// Secondary pane still shows on the takeover route.
|
||||
await expect(page.locator("[data-secondary-sidebar]")).toBeVisible();
|
||||
|
||||
// The app sidebar stays expanded despite the route request: the
|
||||
// expanded-only header control is visible.
|
||||
// The app sidebar is hard-collapsed despite the stored expanded pin.
|
||||
await expect(page.getByLabel(APP_SIDEBAR_EXPANDED_MARKER)).toHaveCount(0);
|
||||
|
||||
await page.goto(`/${prefix}/dashboard`);
|
||||
|
||||
// Leaving the takeover route clears the force and restores the user's
|
||||
// persisted expanded pin.
|
||||
await expect(page.locator("[data-secondary-sidebar]")).toHaveCount(0);
|
||||
await expect(page.getByLabel(APP_SIDEBAR_EXPANDED_MARKER)).toBeVisible();
|
||||
});
|
||||
|
||||
|
||||
@@ -27,6 +27,10 @@ export function SidebarNavExpandedProvider({ children }: { children: ReactNode }
|
||||
);
|
||||
}
|
||||
|
||||
export function useSidebarNavExpanded() {
|
||||
return useContext(SidebarNavExpandedContext);
|
||||
}
|
||||
|
||||
interface SidebarNavItemProps {
|
||||
to: string;
|
||||
label: string;
|
||||
@@ -63,7 +67,7 @@ export function SidebarNavItem({
|
||||
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);
|
||||
const forceExpanded = useSidebarNavExpanded();
|
||||
// The icon-only rail presentation only applies when pinned collapsed and not
|
||||
// peeking; a peek/expanded panel — or an expanded contextual pane — restores
|
||||
// the full label + badge.
|
||||
|
||||
@@ -5,6 +5,7 @@ import type { ReactNode } from "react";
|
||||
import { createRoot } from "react-dom/client";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { SidebarSection } from "./SidebarSection";
|
||||
import { SidebarNavExpandedProvider } from "./SidebarNavItem";
|
||||
import { Plus } from "lucide-react";
|
||||
|
||||
const sidebarState = vi.hoisted(() => ({
|
||||
@@ -17,6 +18,9 @@ vi.mock("@/lib/router", () => ({
|
||||
Link: ({ children, to, ...props }: { children: ReactNode; to: string }) => (
|
||||
<a href={to} {...props}>{children}</a>
|
||||
),
|
||||
NavLink: ({ children, to, ...props }: { children: ReactNode; to: string }) => (
|
||||
<a href={to} {...props}>{children}</a>
|
||||
),
|
||||
}));
|
||||
|
||||
vi.mock("../context/SidebarContext", () => ({
|
||||
@@ -159,6 +163,30 @@ describe("SidebarSection", () => {
|
||||
expect(staticLabelControl?.getAttribute("class")).not.toContain("focus-visible:bg-accent/50");
|
||||
});
|
||||
|
||||
it("keeps section labels visible inside an expanded contextual pane", async () => {
|
||||
sidebarState.collapsed = true;
|
||||
const currentRoot = createRoot(container);
|
||||
root = currentRoot;
|
||||
|
||||
await act(async () => {
|
||||
currentRoot.render(
|
||||
<SidebarNavExpandedProvider>
|
||||
<SidebarSection label="Settings">
|
||||
<a href="/settings">General</a>
|
||||
</SidebarSection>
|
||||
</SidebarNavExpandedProvider>,
|
||||
);
|
||||
});
|
||||
await flushReact();
|
||||
|
||||
const settingsLabel = Array.from(container.querySelectorAll("span"))
|
||||
.find((element) => element.textContent === "Settings");
|
||||
|
||||
expect(settingsLabel).toBeTruthy();
|
||||
expect(settingsLabel?.getAttribute("class")).toContain("uppercase");
|
||||
expect(container.querySelector(".bg-border\\/60")).toBeNull();
|
||||
});
|
||||
|
||||
it("keeps the header action outside the label menu hit area", async () => {
|
||||
const onAction = vi.fn();
|
||||
const currentRoot = createRoot(container);
|
||||
|
||||
@@ -18,6 +18,7 @@ import {
|
||||
DropdownMenuTrigger,
|
||||
} from "@/components/ui/dropdown-menu";
|
||||
import { cn } from "@/lib/utils";
|
||||
import { useSidebarNavExpanded } from "./SidebarNavItem";
|
||||
|
||||
type SidebarSectionIcon = ComponentType<{ className?: string }>;
|
||||
|
||||
@@ -190,7 +191,8 @@ export function SidebarSection({
|
||||
headerAction,
|
||||
}: SidebarSectionProps) {
|
||||
const { collapsed, peeking } = useSidebar();
|
||||
const rail = collapsed && !peeking;
|
||||
const forceExpanded = useSidebarNavExpanded();
|
||||
const rail = collapsed && !peeking && !forceExpanded;
|
||||
const content = <div className="flex flex-col gap-0.5 mt-0.5">{children}</div>;
|
||||
|
||||
// Collapsed rail: the section header would only show a clipped sliver of its
|
||||
|
||||
@@ -176,7 +176,9 @@ export function SidebarProvider({ children }: { children: ReactNode }) {
|
||||
peeking,
|
||||
setPeeking,
|
||||
forceCollapsed,
|
||||
setForceCollapsed,
|
||||
routeRequestsCollapsed,
|
||||
setRouteRequestsCollapsed,
|
||||
],
|
||||
);
|
||||
|
||||
|
||||
Reference in new issue
Block a user