diff --git a/doc/plugins/PLUGIN_AUTHORING_GUIDE.md b/doc/plugins/PLUGIN_AUTHORING_GUIDE.md index 33d4ec2abcf..5f8380f7b19 100644 --- a/doc/plugins/PLUGIN_AUTHORING_GUIDE.md +++ b/doc/plugins/PLUGIN_AUTHORING_GUIDE.md @@ -346,9 +346,11 @@ route is active the host collapses the app `` to its 64px icon rail 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. +doing so is redundant and fights the host. While your route is active the app +rail is forced collapsed (its expand toggle is hidden), overriding any user pin +— a secondary sidebar always collapses the primary. This force never changes the +user's saved expanded/collapsed preference, so the host restores exactly what +the user chose as soon as they navigate away. ## Shared host components diff --git a/doc/plugins/PLUGIN_SPEC.md b/doc/plugins/PLUGIN_SPEC.md index a247221ac24..ccd72f5e6f0 100644 --- a/doc/plugins/PLUGIN_SPEC.md +++ b/doc/plugins/PLUGIN_SPEC.md @@ -1144,9 +1144,13 @@ 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. +on navigation away. The collapse is a **hard invariant**: while a secondary +sidebar is shown the app rail is forced collapsed and its expand/toggle +affordance is hidden, *overriding* any user pin. Crucially, this force is +ephemeral — it never mutates the user's persisted expanded/collapsed preference, +so navigating back to a normal route restores exactly what the user chose. +Precedence is therefore: secondary-sidebar force > explicit user pin > +route-requested collapse (`RequestCollapsedSidebar`) > default expanded. ## 19.6 Shared Components In `@paperclipai/plugin-sdk/ui` diff --git a/packages/plugins/sdk/README.md b/packages/plugins/sdk/README.md index ca6e2682ab0..020b0b8dedc 100644 --- a/packages/plugins/sdk/README.md +++ b/packages/plugins/sdk/README.md @@ -240,7 +240,7 @@ Adds a navigation-style entry to the main company sidebar navigation area, rende 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. +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. The collapse is a hard invariant: a secondary sidebar always forces the app rail collapsed (hiding its expand toggle), overriding any user pin, but it never mutates the user's saved expanded/collapsed preference — that is restored as soon as they leave your route. #### `sidebarPanel` diff --git a/ui/src/components/Layout.test.tsx b/ui/src/components/Layout.test.tsx index 57c79b7f12e..0a550ca9869 100644 --- a/ui/src/components/Layout.test.tsx +++ b/ui/src/components/Layout.test.tsx @@ -17,7 +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 mockSetForceCollapsed = vi.hoisted(() => vi.fn()); const mockCompanyState = vi.hoisted(() => ({ companies: [{ id: "company-1", issuePrefix: "PAP", name: "Paperclip" }], selectedCompany: { id: "company-1", issuePrefix: "PAP", name: "Paperclip" }, @@ -173,11 +173,14 @@ vi.mock("../context/SidebarContext", () => ({ toggleSidebar: vi.fn(), toggleCollapsed: vi.fn(), collapsed: mockSidebarState.collapsed, + collapseLocked: false, peeking: mockSidebarState.peeking, setPeeking: mockSetPeeking, isMobile: mockSidebarState.isMobile, + forceCollapsed: false, + setForceCollapsed: mockSetForceCollapsed, routeRequestsCollapsed: false, - setRouteRequestsCollapsed: mockSetRouteRequestsCollapsed, + setRouteRequestsCollapsed: vi.fn(), }), })); @@ -410,7 +413,7 @@ describe("Layout", () => { expect(container.textContent).not.toContain("Instance sidebar"); 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); + expect(mockSetForceCollapsed).toHaveBeenCalledWith(true); await act(async () => { root.unmount(); @@ -474,7 +477,7 @@ describe("Layout", () => { expect(container.textContent).toContain("Main company nav"); expect(container.textContent).not.toContain("Company rail"); expect(container.textContent).not.toContain("Plugin route sidebar"); - expect(mockSetRouteRequestsCollapsed).toHaveBeenCalledWith(true); + expect(mockSetForceCollapsed).toHaveBeenCalledWith(true); await act(async () => { root.unmount(); @@ -529,7 +532,7 @@ describe("Layout", () => { 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); + expect(mockSetForceCollapsed).toHaveBeenCalledWith(true); await act(async () => { root.unmount(); @@ -713,8 +716,8 @@ 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); + expect(mockSetForceCollapsed).not.toHaveBeenCalledWith(true); + expect(mockSetForceCollapsed).toHaveBeenCalledWith(false); await act(async () => { root.unmount(); diff --git a/ui/src/components/Layout.tsx b/ui/src/components/Layout.tsx index caa6c6bdfa3..c23ac306465 100644 --- a/ui/src/components/Layout.tsx +++ b/ui/src/components/Layout.tsx @@ -58,7 +58,7 @@ export function Layout() { peeking, setPeeking, isMobile, - setRouteRequestsCollapsed, + setForceCollapsed, } = useSidebar(); const { openNewIssue, openOnboarding } = useDialogActions(); const { togglePanelVisible } = usePanel(); @@ -143,14 +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). + // A secondary sidebar always collapses the app sidebar to its rail (still + // peek-able) — a hard invariant that overrides the user pin while the route + // is active, but does NOT mutate the persisted preference. Clearing the force + // on cleanup restores the user's expanded/collapsed choice when navigating + // off the takeover route (PAP-10694). useEffect(() => { - setRouteRequestsCollapsed(hasSecondarySidebar); - return () => setRouteRequestsCollapsed(false); - }, [hasSecondarySidebar, setRouteRequestsCollapsed]); + setForceCollapsed(hasSecondarySidebar); + return () => setForceCollapsed(false); + }, [hasSecondarySidebar, setForceCollapsed]); useEffect(() => { if (companiesLoading || onboardingTriggered.current) return; diff --git a/ui/src/components/Sidebar.test.tsx b/ui/src/components/Sidebar.test.tsx index 960adfcc31d..c711b02a7cd 100644 --- a/ui/src/components/Sidebar.test.tsx +++ b/ui/src/components/Sidebar.test.tsx @@ -52,6 +52,7 @@ const mockSidebar = vi.hoisted(() => ({ isMobile: false, setSidebarOpen: vi.fn(), collapsed: false, + collapseLocked: false, peeking: false, toggleCollapsed: vi.fn(), setCollapsed: vi.fn(), @@ -315,6 +316,22 @@ describe("Sidebar", () => { }); }); + it("hides the expand/collapse toggle while a secondary sidebar locks the rail", async () => { + // A secondary sidebar forces the rail; the user must not be able to expand + // the primary while it is shown (PAP-10694). + mockInstanceSettingsApi.getExperimental.mockResolvedValue({ enableIsolatedWorkspaces: false }); + mockSidebar.collapseLocked = true; + const root = await renderSidebar(); + + expect(container.querySelector('button[aria-label="Collapse sidebar"]')).toBeNull(); + expect(container.querySelector('button[aria-label="Expand sidebar"]')).toBeNull(); + + mockSidebar.collapseLocked = false; + flushSync(() => { + root.unmount(); + }); + }); + it("keeps the collapsed rail top bar to just the company logo (no clipped search/toggle)", async () => { // In the narrow rail the search/toggle controls don't fit beside the logo and // would overflow/clip, shoving the logo out of the icon column (PAP-10676), so diff --git a/ui/src/components/Sidebar.tsx b/ui/src/components/Sidebar.tsx index 73d08e59837..592888ad286 100644 --- a/ui/src/components/Sidebar.tsx +++ b/ui/src/components/Sidebar.tsx @@ -41,7 +41,7 @@ import { SidebarCompanyMenu } from "./SidebarCompanyMenu"; export function Sidebar() { const { openNewIssue } = useDialogActions(); const { selectedCompanyId, selectedCompany } = useCompany(); - const { isMobile, collapsed, peeking, toggleCollapsed, setCollapsed } = useSidebar(); + const { isMobile, collapsed, collapseLocked, peeking, toggleCollapsed, setCollapsed } = useSidebar(); const rail = collapsed && !peeking; const inboxBadge = useInboxBadge(selectedCompanyId); const { data: experimentalSettings } = useQuery({ @@ -94,8 +94,10 @@ export function Sidebar() { {/* Desktop-only collapse/expand affordance. While peeking (hover flyout over the collapsed rail) it becomes a Pin that promotes the peek to a pinned-expanded sidebar; otherwise it toggles the pinned rail. Mobile - uses the off-canvas drawer, so this control is hidden there. */} - {!isMobile ? ( + uses the off-canvas drawer, so this control is hidden there. It is + also hidden while a secondary sidebar forces the rail (collapseLocked): + the user cannot expand the primary while a secondary sidebar is shown. */} + {!isMobile && !collapseLocked ? ( peeking ? (