diff --git a/doc/plugins/PLUGIN_AUTHORING_GUIDE.md b/doc/plugins/PLUGIN_AUTHORING_GUIDE.md index dca3ec62466..33d4ec2abcf 100644 --- a/doc/plugins/PLUGIN_AUTHORING_GUIDE.md +++ b/doc/plugins/PLUGIN_AUTHORING_GUIDE.md @@ -336,6 +336,20 @@ Mount surfaces currently wired in the host include: - `commentAnnotation` - `commentContextMenuItem` +### `routeSidebar` and the app sidebar + +A `routeSidebar` slot gives a plugin page route its own contextual navigation. +It **coexists** with the main app sidebar rather than replacing it: while your +route is active the host collapses the app `` to its 64px icon rail +(still hover/peek-able) and renders your sidebar in a second pane, yielding +`[ app rail ][ your sidebar ][ content ]`. + +Because the host drives this collapse, a plugin should **not** mount +`RequestCollapsedSidebar` or otherwise try to collapse the app sidebar itself — +doing so is redundant and fights the host. The host restores the user's previous +sidebar state when they navigate away, and an explicit user pin (expanded or +collapsed) always takes precedence over the route-driven collapse. + ## Shared host components Use shared components from `@paperclipai/plugin-sdk/ui` when the plugin needs a diff --git a/doc/plugins/PLUGIN_SPEC.md b/doc/plugins/PLUGIN_SPEC.md index d1bd103486c..a247221ac24 100644 --- a/doc/plugins/PLUGIN_SPEC.md +++ b/doc/plugins/PLUGIN_SPEC.md @@ -1131,6 +1131,23 @@ Plugins may add sidebar links to: - global plugin settings - company-context plugin pages +### 19.5.1 Route Sidebars (`routeSidebar`) + +A `routeSidebar` slot supplies a contextual sidebar for a plugin page route +(matched by `routePath`). It **coexists** with the main app sidebar rather than +replacing it: while the route is active the host collapses the app `` +to its 64px icon rail (still hover/peek-able) and renders the plugin's +`routeSidebar` in a secondary pane, producing the layout +`[ app rail ][ route sidebar ][ content ]`. The same model applies to the +host's own company-settings sidebar. + +The host owns the collapse. Plugins must not mount `RequestCollapsedSidebar` or +otherwise attempt to collapse the app sidebar from a `routeSidebar` — the host +applies the collapse while the route is mounted and restores the previous state +on navigation away. Collapse precedence is: explicit user pin (expanded or +collapsed) > route-requested collapse > default expanded, so a user's pin always +wins over the route-driven collapse. + ## 19.6 Shared Components In `@paperclipai/plugin-sdk/ui` The host SDK ships shared components that plugins can import to quickly build UIs that match the host's look and feel. These are convenience building blocks, not a requirement. diff --git a/packages/plugins/sdk/README.md b/packages/plugins/sdk/README.md index b9a02d5c149..ca6e2682ab0 100644 --- a/packages/plugins/sdk/README.md +++ b/packages/plugins/sdk/README.md @@ -238,7 +238,9 @@ Adds a navigation-style entry to the main company sidebar navigation area, rende #### `routeSidebar` -Replaces the normal company sidebar while the current route is a plugin page route with the same `routePath`. Use this for full-page plugin workspaces that need their own local navigation while keeping the company rail and account footer. Receives `PluginRouteSidebarProps` with `context.companyId` and `context.companyPrefix` set to the active company. Requires the `ui.sidebar.register` capability. +A contextual sidebar shown while the current route is a plugin page route with the same `routePath`. Use this for full-page plugin workspaces that need their own local navigation. It does **not** replace the app sidebar: the host collapses the main `` to its 64px icon rail (still hover/peek-able) and renders your `routeSidebar` in a secondary pane beside it, producing `[ app rail ][ your sidebar ][ content ]`. Receives `PluginRouteSidebarProps` with `context.companyId` and `context.companyPrefix` set to the active company. Requires the `ui.sidebar.register` capability. + +Do **not** mount `RequestCollapsedSidebar` (or otherwise try to collapse the app sidebar) from a `routeSidebar` plugin — the host drives the collapse automatically while your route is active and restores the user's preference when they navigate away. A user's explicit pin (expanded or collapsed) always wins over this route-driven collapse. #### `sidebarPanel` diff --git a/tests/e2e/sidebar-takeover.spec.ts b/tests/e2e/sidebar-takeover.spec.ts new file mode 100644 index 00000000000..b5a4457c7a9 --- /dev/null +++ b/tests/e2e/sidebar-takeover.spec.ts @@ -0,0 +1,132 @@ +import { test, expect, request as pwRequest, type APIRequestContext } from "@playwright/test"; + +/** + * E2E: Sidebar takeover model (PAP-10695). + * + * Takeover routes (company settings, plugin `routeSidebar`) no longer *replace* + * the main app sidebar. Instead the host collapses the app `` to its + * 64px rail (still peek-able) and renders the contextual sidebar in a second + * pane → `[ app rail ][ secondary ~240px ][ content ]`. + * + * These specs assert the rail + secondary pane coexist on a company settings + * route, and that an explicit user pin (expanded) wins over the route-driven + * collapse (pin precedence). + * + * The plugin `routeSidebar` half of this behavior shares the exact same Layout + * code path (one `secondarySidebar`/`hasSecondarySidebar` resolver drives both + * company-settings and plugin routes) and is covered by the unit tests in + * `ui/src/components/Layout.test.tsx`. A live plugin-route e2e requires the + * `plugin-llm-wiki` plugin to be installed in the throwaway e2e instance, which + * is out of scope for this default local_trusted run; visual QA of both panes + * is delegated to the QA child issue. + */ + +const PORT = Number(process.env.PAPERCLIP_E2E_PORT ?? 3199); +const BASE_URL = `http://127.0.0.1:${PORT}`; +const COMPANY_NAME = `E2E-SidebarTakeover-${Date.now()}`; +const COLLAPSED_STORAGE_KEY = "paperclip.sidebar.collapsed"; + +// The sidebar header's "Open search" control only renders when the app sidebar +// is expanded (pinned or peeking); in the collapsed rail it is hidden to fit +// the 64px width. Its presence/absence is therefore a stable proxy for the +// app sidebar's collapsed state (see Sidebar.tsx). +const APP_SIDEBAR_EXPANDED_MARKER = "Open search"; + +async function createCompany(board: APIRequestContext): Promise<{ id: string; prefix: string }> { + const healthRes = await board.get(`${BASE_URL}/api/health`); + expect(healthRes.ok()).toBe(true); + const health = await healthRes.json(); + expect(health.deploymentMode).toBe("local_trusted"); + + const companyRes = await board.post(`${BASE_URL}/api/companies`, { + data: { name: COMPANY_NAME }, + }); + if (!companyRes.ok()) { + throw new Error(`POST /api/companies → ${companyRes.status()}: ${await companyRes.text()}`); + } + const company = await companyRes.json(); + return { + id: company.id, + prefix: company.issuePrefix ?? company.prefix ?? company.urlKey ?? "E2E", + }; +} + +test.describe("Sidebar takeover (collapse + secondary pane)", () => { + let board: APIRequestContext; + let companyId: string; + let prefix: string; + + test.beforeAll(async () => { + board = await pwRequest.newContext({ baseURL: BASE_URL }); + const company = await createCompany(board); + companyId = company.id; + prefix = company.prefix; + }); + + test.afterAll(async () => { + await board.delete(`${BASE_URL}/api/companies/${companyId}`).catch(() => {}); + await board.dispose(); + }); + + test.beforeEach(async ({ page }) => { + // Start each test from a clean (unpinned) sidebar state so the route-driven + // collapse is the only thing acting on it. + await page.addInitScript((key) => { + window.localStorage.removeItem(key); + }, COLLAPSED_STORAGE_KEY); + }); + + test("collapses the app sidebar to its rail and shows the settings sidebar beside it", async ({ page }) => { + await page.goto(`/${prefix}/company/settings`); + + // The contextual (secondary) pane is present... + const secondary = page.locator("[data-secondary-sidebar]"); + await expect(secondary).toBeVisible(); + await expect(secondary).toHaveCount(1); + + // ...and it is ~240px wide (w-60), distinct from the 64px app rail. + const secondaryBox = await secondary.boundingBox(); + expect(secondaryBox).not.toBeNull(); + expect(secondaryBox!.width).toBeGreaterThan(180); + + // The app sidebar is NOT replaced — its company nav still renders... + await expect(page.getByRole("link", { name: "Dashboard" })).toBeVisible(); + + // ...but it is collapsed to its rail: the expanded-only "Open search" + // header control is hidden. + await expect(page.getByLabel(APP_SIDEBAR_EXPANDED_MARKER)).toHaveCount(0); + }); + + 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). + await page.addInitScript( + ({ key }) => { + window.localStorage.setItem(key, "0"); + }, + { key: COLLAPSED_STORAGE_KEY }, + ); + + await page.goto(`/${prefix}/company/settings`); + + // 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. + await expect(page.getByLabel(APP_SIDEBAR_EXPANDED_MARKER)).toBeVisible(); + }); + + test("leaving the takeover route removes the secondary pane and restores the sidebar", async ({ page }) => { + await page.goto(`/${prefix}/company/settings`); + await expect(page.locator("[data-secondary-sidebar]")).toBeVisible(); + await expect(page.getByLabel(APP_SIDEBAR_EXPANDED_MARKER)).toHaveCount(0); + + // Navigate to a plain (non-takeover) route. + await page.goto(`/${prefix}/dashboard`); + + // No secondary pane, and the app sidebar is no longer force-collapsed. + await expect(page.locator("[data-secondary-sidebar]")).toHaveCount(0); + await expect(page.getByLabel(APP_SIDEBAR_EXPANDED_MARKER)).toBeVisible(); + }); +}); diff --git a/ui/src/components/Layout.test.tsx b/ui/src/components/Layout.test.tsx index 638e46bbd92..43321ec4202 100644 --- a/ui/src/components/Layout.test.tsx +++ b/ui/src/components/Layout.test.tsx @@ -17,6 +17,7 @@ const mockInstanceSettingsApi = vi.hoisted(() => ({ const mockNavigate = vi.hoisted(() => vi.fn()); const mockSetSelectedCompanyId = vi.hoisted(() => vi.fn()); const mockSetSidebarOpen = vi.hoisted(() => vi.fn()); +const mockSetRouteRequestsCollapsed = vi.hoisted(() => vi.fn()); const mockCompanyState = vi.hoisted(() => ({ companies: [{ id: "company-1", issuePrefix: "PAP", name: "Paperclip" }], selectedCompany: { id: "company-1", issuePrefix: "PAP", name: "Paperclip" }, @@ -27,9 +28,12 @@ const mockPluginSlots = vi.hoisted(() => ({ })); const mockUsePluginSlots = vi.hoisted(() => vi.fn()); const mockPluginSlotContexts = vi.hoisted(() => [] as Array>); +const mockSetPeeking = vi.hoisted(() => vi.fn()); const mockSidebarState = vi.hoisted(() => ({ sidebarOpen: true, isMobile: false, + collapsed: false, + peeking: false, })); let currentPathname = "/PAP/dashboard"; @@ -168,10 +172,12 @@ vi.mock("../context/SidebarContext", () => ({ setSidebarOpen: mockSetSidebarOpen, toggleSidebar: vi.fn(), toggleCollapsed: vi.fn(), - collapsed: false, - peeking: false, - setPeeking: vi.fn(), + collapsed: mockSidebarState.collapsed, + peeking: mockSidebarState.peeking, + setPeeking: mockSetPeeking, isMobile: mockSidebarState.isMobile, + routeRequestsCollapsed: false, + setRouteRequestsCollapsed: mockSetRouteRequestsCollapsed, }), })); @@ -240,6 +246,9 @@ describe("Layout", () => { mockPluginSlotContexts.length = 0; mockSidebarState.sidebarOpen = true; mockSidebarState.isMobile = false; + mockSidebarState.collapsed = false; + mockSidebarState.peeking = false; + mockSetPeeking.mockClear(); }); afterEach(() => { @@ -278,7 +287,54 @@ describe("Layout", () => { }); }); - it("renders the company settings sidebar on company settings routes", async () => { + it("collapses atomically when the pointer is still over the sidebar (no re-peek) — PAP-10676", async () => { + const root = createRoot(container); + const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + + const renderLayout = async () => { + await act(async () => { + root.render( + + + , + ); + }); + await flushReact(); + }; + + // The SidebarShell overlay panel carries the peek mouse handlers. + const panel = () => + [...container.querySelectorAll("div")].find( + (el) => el.className.includes("inset-y-0") && el.className.includes("overflow-hidden"), + ); + const hover = (el: HTMLElement) => { + // React derives onMouseEnter from a mouseover crossing in from outside. + el.dispatchEvent(new MouseEvent("mouseover", { bubbles: true, relatedTarget: document.body })); + }; + + // Expanded, then hover the panel so the pointer is registered as inside. + await renderLayout(); + const expandedPanel = panel(); + expect(expandedPanel).toBeTruthy(); + await act(async () => { hover(expandedPanel!); }); + + // Collapse while the pointer is still over the panel. + mockSidebarState.collapsed = true; + await renderLayout(); + // The peek is cancelled atomically on collapse. + expect(mockSetPeeking).toHaveBeenCalledWith(false); + + // A lingering/spurious hover while collapsed must NOT re-open the peek. + mockSetPeeking.mockClear(); + const railPanel = panel(); + await act(async () => { hover(railPanel!); }); + await act(async () => { await new Promise((r) => setTimeout(r, 80)); }); + expect(mockSetPeeking).not.toHaveBeenCalledWith(true); + + await act(async () => { root.unmount(); }); + }); + + it("keeps the app sidebar and shows the settings sidebar in the secondary pane on settings routes", async () => { currentPathname = "/PAP/company/settings/access"; mockPluginSlots.slots = [ { @@ -319,11 +375,15 @@ describe("Layout", () => { await flushReact(); await flushReact(); + // Takeover model (PAP-10695): the app sidebar is kept (collapsed to its + // rail) AND the settings sidebar renders in the secondary pane. expect(container.textContent).toContain("Company settings sidebar"); + expect(container.textContent).toContain("Main company nav"); expect(container.textContent).not.toContain("Company rail"); expect(container.textContent).not.toContain("Instance sidebar"); - expect(container.textContent).not.toContain("Main company nav"); expect(container.textContent).not.toContain("Plugin route sidebar"); + // The route asks the host to collapse the app sidebar to its rail. + expect(mockSetRouteRequestsCollapsed).toHaveBeenCalledWith(true); await act(async () => { root.unmount(); @@ -384,9 +444,10 @@ describe("Layout", () => { await flushReact(); expect(container.textContent).toContain("Company settings sidebar"); + expect(container.textContent).toContain("Main company nav"); expect(container.textContent).not.toContain("Company rail"); - expect(container.textContent).not.toContain("Main company nav"); expect(container.textContent).not.toContain("Plugin route sidebar"); + expect(mockSetRouteRequestsCollapsed).toHaveBeenCalledWith(true); await act(async () => { root.unmount(); @@ -434,11 +495,14 @@ describe("Layout", () => { await flushReact(); await flushReact(); + // Takeover model (PAP-10695): the app sidebar coexists with the plugin's + // route sidebar, which renders in the secondary pane. expect(container.textContent).toContain("Plugin route sidebar: Wiki Sidebar"); expect(container.querySelector("[data-plugin-slot-class='h-full w-full']")).not.toBeNull(); - expect(container.textContent).not.toContain("Main company nav"); + expect(container.textContent).toContain("Main company nav"); expect(container.textContent).not.toContain("Company settings sidebar"); expect(container.textContent).not.toContain("Instance sidebar"); + expect(mockSetRouteRequestsCollapsed).toHaveBeenCalledWith(true); await act(async () => { root.unmount(); @@ -493,7 +557,7 @@ describe("Layout", () => { }), ); expect(container.textContent).toContain("Plugin route sidebar: Wiki Sidebar"); - expect(container.textContent).not.toContain("Main company nav"); + expect(container.textContent).toContain("Main company nav"); await act(async () => { root.unmount(); @@ -621,6 +685,9 @@ describe("Layout", () => { expect(container.textContent).toContain("Main company nav"); expect(container.textContent).not.toContain("Plugin route sidebar"); + // No secondary pane, so the route must not force the sidebar collapsed. + expect(mockSetRouteRequestsCollapsed).not.toHaveBeenCalledWith(true); + expect(mockSetRouteRequestsCollapsed).toHaveBeenCalledWith(false); await act(async () => { root.unmount(); diff --git a/ui/src/components/Layout.tsx b/ui/src/components/Layout.tsx index c4fd8dc5623..caa6c6bdfa3 100644 --- a/ui/src/components/Layout.tsx +++ b/ui/src/components/Layout.tsx @@ -18,6 +18,7 @@ import { WorktreeBanner } from "./WorktreeBanner"; import { DevRestartBanner } from "./DevRestartBanner"; import { StandaloneBrowserControls } from "./StandaloneBrowserControls"; import { SidebarShell } from "./SidebarShell"; +import { SecondarySidebar } from "./SecondarySidebar"; import { SidebarAccountMenu } from "./SidebarAccountMenu"; import { useDialogActions } from "../context/DialogContext"; import { GeneralSettingsProvider } from "../context/GeneralSettingsContext"; @@ -57,6 +58,7 @@ export function Layout() { peeking, setPeeking, isMobile, + setRouteRequestsCollapsed, } = useSidebar(); const { openNewIssue, openOnboarding } = useDialogActions(); const { togglePanelVisible } = usePanel(); @@ -111,16 +113,21 @@ export function Layout() { }), [routeSidebarCompanyId, routeSidebarCompanyPrefix], ); - const companySidebar = routeSidebarSlot ? ( + // Takeover routes (company settings, plugin `routeSidebar`) no longer replace + // the app ``. Instead the host collapses it to its rail and renders + // the contextual sidebar in a second pane (PAP-10695). One resolver drives + // both desktop (SecondarySidebar) and mobile (off-canvas drawer). + const secondarySidebar = isCompanySettingsRoute ? ( + + ) : routeSidebarSlot ? ( - ) : ( - - ); + ) : null; + const hasSecondarySidebar = secondarySidebar != null; const { data: health } = useQuery({ queryKey: queryKeys.health, queryFn: () => healthApi.get(), @@ -136,6 +143,15 @@ export function Layout() { queryFn: () => instanceSettingsApi.getGeneral(), }).data?.keyboardShortcuts === true; + // While a secondary sidebar is shown, ask the app sidebar to default to its + // collapsed rail (still peek-able). An explicit user pin always wins — that + // precedence lives in SidebarContext. Clearing on cleanup restores the + // default when navigating off the takeover route (PAP-10695). + useEffect(() => { + setRouteRequestsCollapsed(hasSecondarySidebar); + return () => setRouteRequestsCollapsed(false); + }, [hasSecondarySidebar, setRouteRequestsCollapsed]); + useEffect(() => { if (companiesLoading || onboardingTriggered.current) return; if (health?.deploymentMode === "authenticated") return; @@ -217,6 +233,11 @@ export function Layout() { // the user is still hovering — it should only close when they actually mouse off // (PAP-10676). const pointerInsidePanel = useRef(false); + // When the user explicitly collapses while the pointer is still over the panel, + // suppress re-peeking until the pointer actually leaves — otherwise the lingering + // hover immediately re-expands the rail and the collapse "doesn't take" until the + // mouse moves away (PAP-10676). Re-armed on the next genuine pointer-leave. + const suppressPeekRef = useRef(false); const clearPeekTimer = useCallback(() => { if (peekTimer.current !== null) { window.clearTimeout(peekTimer.current); @@ -235,14 +256,21 @@ export function Layout() { clearPeekTimer(); peekTimer.current = window.setTimeout(() => setPeeking(false), 120); }, [clearPeekTimer, setPeeking]); + // Tracked even while expanded so that, at the moment of collapse, we know + // whether the pointer is over the panel and should suppress the re-peek. const handlePanelPointerEnter = useCallback(() => { pointerInsidePanel.current = true; - openPeek(); - }, [openPeek]); + if (collapsed && !suppressPeekRef.current) openPeek(); + }, [collapsed, openPeek]); const handlePanelPointerLeave = useCallback(() => { pointerInsidePanel.current = false; + suppressPeekRef.current = false; // pointer left — re-arm peek for the next hover closePeek(); }, [closePeek]); + const handlePanelFocus = useCallback(() => { + if (suppressPeekRef.current) return; + openPeekImmediate(); + }, [openPeekImmediate]); // Close on focus leaving the panel only when the pointer isn't hovering it. // Clicking a rail/peek nav item moves focus to
on navigation; if the // mouse is still over the flyout we keep it open until the pointer leaves. @@ -254,6 +282,23 @@ export function Layout() { // Tidy up any pending peek timer on unmount. useEffect(() => clearPeekTimer, [clearPeekTimer]); + // An explicit collapse must be atomic: cancel any in-flight/active peek, and if + // the pointer is still over the panel suppress re-peeking until it leaves, so the + // rail doesn't immediately re-expand under the lingering hover (PAP-10676). + const wasCollapsed = useRef(collapsed); + useEffect(() => { + if (collapsed !== wasCollapsed.current) { + if (collapsed) { + clearPeekTimer(); + setPeeking(false); + suppressPeekRef.current = pointerInsidePanel.current; + } else { + suppressPeekRef.current = false; + } + wasCollapsed.current = collapsed; + } + }, [collapsed, clearPeekTimer, setPeeking]); + // Intentionally do NOT close the peek on navigation: clicking a nav item means // the pointer is still over the flyout, so it should stay open until the user // actually mouses off (handled by onPanelMouseLeave) or blurs out / hits Escape @@ -436,11 +481,7 @@ export function Layout() { >
- {isCompanySettingsRoute ? ( - - ) : ( - companySidebar - )} + {hasSecondarySidebar ? secondarySidebar : }
- {isCompanySettingsRoute ? ( - - ) : ( - companySidebar - )} +
)} + {!isMobile && hasSecondarySidebar ? ( + {secondarySidebar} + ) : null} +
`: + * the host collapses it to its 64px rail (still peek-able) and renders the + * contextual sidebar here, yielding `[ rail 64px ][ secondary ~240px ][ content ]`. + * + * 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`. + */ +export function SecondarySidebar({ + children, + className, +}: { + children: ReactNode; + className?: string; +}) { + return ( +
+ {children} +
+ ); +}