From ee9d48a3cdf9c66eec36d7ed118ca53597e58a37 Mon Sep 17 00:00:00 2001
From: Dotta <bippadotta@protonmail.com>
Date: Tue, 9 Jun 2026 16:59:02 +0000
Subject: [PATCH] fix(ui): align sidebar takeover review feedback

---
 tests/e2e/sidebar-takeover.spec.ts        | 17 ++++++++++----
 ui/src/components/SidebarNavItem.tsx      |  6 ++++-
 ui/src/components/SidebarSection.test.tsx | 28 +++++++++++++++++++++++
 ui/src/components/SidebarSection.tsx      |  4 +++-
 ui/src/context/SidebarContext.tsx         |  2 ++
 5 files changed, 50 insertions(+), 7 deletions(-)

diff --git a/tests/e2e/sidebar-takeover.spec.ts b/tests/e2e/sidebar-takeover.spec.ts
index 9223721775f..e59639353df 100644
--- a/tests/e2e/sidebar-takeover.spec.ts
+++ b/tests/e2e/sidebar-takeover.spec.ts
@@ -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();
   });
 
diff --git a/ui/src/components/SidebarNavItem.tsx b/ui/src/components/SidebarNavItem.tsx
index 4dc2a7bc0f3..5aaf41037e4 100644
--- a/ui/src/components/SidebarNavItem.tsx
+++ b/ui/src/components/SidebarNavItem.tsx
@@ -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.
diff --git a/ui/src/components/SidebarSection.test.tsx b/ui/src/components/SidebarSection.test.tsx
index 51d2d2fe152..10832627044 100644
--- a/ui/src/components/SidebarSection.test.tsx
+++ b/ui/src/components/SidebarSection.test.tsx
@@ -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);
diff --git a/ui/src/components/SidebarSection.tsx b/ui/src/components/SidebarSection.tsx
index a6a78611104..d277755f230 100644
--- a/ui/src/components/SidebarSection.tsx
+++ b/ui/src/components/SidebarSection.tsx
@@ -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
diff --git a/ui/src/context/SidebarContext.tsx b/ui/src/context/SidebarContext.tsx
index a69fe239283..e9e5d3bcd1a 100644
--- a/ui/src/context/SidebarContext.tsx
+++ b/ui/src/context/SidebarContext.tsx
@@ -176,7 +176,9 @@ export function SidebarProvider({ children }: { children: ReactNode }) {
       peeking,
       setPeeking,
       forceCollapsed,
+      setForceCollapsed,
       routeRequestsCollapsed,
+      setRouteRequestsCollapsed,
     ],
   );