Merge pull request 'fix: Sidebar - make headings collapsible' (#97) from claude/issue-96 into master

Reviewed-on: #97

App/components/layout/sidebar.tsx

@@ -1,5 +1,6 @@
 "use client";
 
+import { useEffect, useState } from "react";
 import { usePathname } from "next/navigation";
 import Link from "next/link";
 import { INVENTORY_ENABLED, CREWING_ENABLED } from "@/lib/feature-flags";
@@ -33,6 +34,7 @@ import {
   UserCog,
   Gauge,
   BadgeCheck,
+  ChevronRight,
 } from "lucide-react";
 import type { Role } from "@prisma/client";
 
@@ -117,6 +119,16 @@ const ADMIN_ITEMS: NavItem[] = [
   { href: "/admin/companies",          label: "Companies",         icon: Briefcase  },
 ];
 
+interface Section {
+  id: string;
+  label: string;
+  items: NavItem[];
+}
+
+function isItemActive(href: string, pathname: string) {
+  return pathname === href || pathname.startsWith(href + "/");
+}
+
 export function Sidebar({ userRole }: { userRole: Role }) {
   const pathname = usePathname();
   const isAdmin  = userRole === "ADMIN";
@@ -125,6 +137,31 @@ export function Sidebar({ userRole }: { userRole: Role }) {
   const visiblePurchasing = PURCHASING_ITEMS.filter((i) => !i.roles || i.roles.includes(userRole));
   const visibleCrewing    = CREWING_ITEMS.filter((i) => !i.roles || i.roles.includes(userRole));
   const visibleMgrAdmin   = MANAGER_ADMIN_ITEMS.filter((i) => !i.roles || i.roles.includes(userRole));
+  const adminItems        = isAdmin ? [...MANAGER_ADMIN_ITEMS, ...ADMIN_ITEMS] : visibleMgrAdmin;
+
+  // Headed, collapsible sections (the main links above sit outside any section).
+  const sections: Section[] = [
+    { id: "purchasing",     label: "Purchasing",     items: visiblePurchasing },
+    { id: "crewing",        label: "Crewing",        items: visibleCrewing },
+    { id: "administration", label: "Administration", items: adminItems },
+  ].filter((s) => s.items.length > 0);
+
+  // The section (if any) that holds the currently active route.
+  const activeSectionId =
+    sections.find((s) => s.items.some((i) => isItemActive(i.href, pathname)))?.id ?? null;
+
+  // Single-open accordion, collapsed by default. Auto-expand the section that
+  // contains the active route so the user is never stranded on a hidden link.
+  const [openSection, setOpenSection] = useState<string | null>(activeSectionId);
+
+  // On navigation, open the section holding the new active route (which, being a
+  // single-open accordion, collapses any other open heading).
+  useEffect(() => {
+    if (activeSectionId) setOpenSection(activeSectionId);
+  }, [activeSectionId]);
+
+  const toggleSection = (id: string) =>
+    setOpenSection((current) => (current === id ? null : id));
 
   return (
     <aside className="flex h-screen w-60 shrink-0 flex-col border-r border-neutral-200 bg-white">
@@ -140,59 +177,61 @@ export function Sidebar({ userRole }: { userRole: Role }) {
           <NavLink key={item.href} item={item} pathname={pathname} />
         ))}
 
-        {visiblePurchasing.length > 0 && (
-          <>
-            <SectionHeader label="Purchasing" />
-            {visiblePurchasing.map((item) => (
-              <NavLink key={item.href} item={item} pathname={pathname} />
-            ))}
-          </>
-        )}
-
-        {/* Crewing — only renders once the flag is on and items exist (later phases) */}
-        {visibleCrewing.length > 0 && (
-          <>
-            <SectionHeader label="Crewing" />
-            {visibleCrewing.map((item) => (
-              <NavLink key={item.href} item={item} pathname={pathname} />
-            ))}
-          </>
-        )}
-
-        {/* Vendors under Administration for MANAGER / ACCOUNTS */}
-        {!isAdmin && visibleMgrAdmin.length > 0 && (
-          <>
-            <SectionHeader label="Administration" />
-            {visibleMgrAdmin.map((item) => (
-              <NavLink key={item.href} item={item} pathname={pathname} />
-            ))}
-          </>
-        )}
-
-        {/* Full Administration section for ADMIN */}
-        {isAdmin && (
-          <>
-            <SectionHeader label="Administration" />
-            {[...MANAGER_ADMIN_ITEMS, ...ADMIN_ITEMS].map((item) => (
-              <NavLink key={item.href} item={item} pathname={pathname} />
-            ))}
-          </>
-        )}
+        {sections.map((section) => {
+          const isOpen = openSection === section.id;
+          const regionId = `nav-section-${section.id}`;
+          return (
+            <div key={section.id}>
+              <SectionHeader
+                label={section.label}
+                isOpen={isOpen}
+                regionId={regionId}
+                onToggle={() => toggleSection(section.id)}
+              />
+              {isOpen && (
+                <div id={regionId} className="space-y-0.5">
+                  {section.items.map((item) => (
+                    <NavLink key={item.href} item={item} pathname={pathname} />
+                  ))}
+                </div>
+              )}
+            </div>
+          );
+        })}
       </nav>
     </aside>
   );
 }
 
-function SectionHeader({ label }: { label: string }) {
+function SectionHeader({
+  label,
+  isOpen,
+  regionId,
+  onToggle,
+}: {
+  label: string;
+  isOpen: boolean;
+  regionId: string;
+  onToggle: () => void;
+}) {
   return (
-    <div className="pt-4 pb-1 px-3">
-      <p className="text-xs font-semibold text-neutral-400 uppercase tracking-wider">{label}</p>
-    </div>
+    <button
+      type="button"
+      onClick={onToggle}
+      aria-expanded={isOpen}
+      aria-controls={regionId}
+      className="flex w-full items-center justify-between pt-4 pb-1 px-3 text-xs font-semibold text-neutral-400 uppercase tracking-wider hover:text-neutral-600"
+    >
+      <span>{label}</span>
+      <ChevronRight
+        className={cn("h-3.5 w-3.5 shrink-0 transition-transform", isOpen && "rotate-90")}
+      />
+    </button>
   );
 }
 
 function NavLink({ item, pathname }: { item: NavItem; pathname: string }) {
-  const isActive = pathname === item.href || pathname.startsWith(item.href + "/");
+  const isActive = isItemActive(item.href, pathname);
   const Icon = item.icon;
   return (
     <Link

App/tests/unit/sidebar.test.tsx

@@ -0,0 +1,102 @@
+import { describe, it, expect, vi, beforeEach } from "vitest";
+import { render, screen, fireEvent, within } from "@testing-library/react";
+
+// usePathname is mockable per-test so we can exercise the auto-expand behaviour.
+let mockPathname = "/dashboard";
+vi.mock("next/navigation", () => ({ usePathname: () => mockPathname }));
+
+import { Sidebar } from "@/components/layout/sidebar";
+
+beforeEach(() => {
+  mockPathname = "/dashboard";
+});
+
+function headerButton(label: string) {
+  return screen.getByRole("button", { name: new RegExp(`^${label}`, "i") });
+}
+
+describe("Sidebar collapsible sections", () => {
+  it("renders section headings as toggle buttons, collapsed by default", () => {
+    // ADMIN sees a Purchasing-less layout? No — render a MANAGER who has
+    // Purchasing + Administration headed sections.
+    render(<Sidebar userRole="MANAGER" />);
+
+    const purchasing = headerButton("Purchasing");
+    const administration = headerButton("Administration");
+
+    expect(purchasing).toHaveAttribute("aria-expanded", "false");
+    expect(administration).toHaveAttribute("aria-expanded", "false");
+
+    // Collapsed → section links are not in the DOM.
+    expect(screen.queryByRole("link", { name: /Cost Centres/i })).not.toBeInTheDocument();
+  });
+
+  it("expands a section and reveals its links when its header is clicked", () => {
+    render(<Sidebar userRole="MANAGER" />);
+
+    const purchasing = headerButton("Purchasing");
+    fireEvent.click(purchasing);
+
+    expect(purchasing).toHaveAttribute("aria-expanded", "true");
+    expect(screen.getByRole("link", { name: /Cost Centres/i })).toBeInTheDocument();
+  });
+
+  it("collapses other sections when one is opened (single-open accordion)", () => {
+    render(<Sidebar userRole="MANAGER" />);
+
+    const purchasing = headerButton("Purchasing");
+    const administration = headerButton("Administration");
+
+    fireEvent.click(purchasing);
+    expect(purchasing).toHaveAttribute("aria-expanded", "true");
+
+    fireEvent.click(administration);
+    expect(administration).toHaveAttribute("aria-expanded", "true");
+    // Opening Administration collapses Purchasing.
+    expect(purchasing).toHaveAttribute("aria-expanded", "false");
+  });
+
+  it("toggles a section closed when its header is clicked again", () => {
+    render(<Sidebar userRole="MANAGER" />);
+
+    const purchasing = headerButton("Purchasing");
+    fireEvent.click(purchasing);
+    expect(purchasing).toHaveAttribute("aria-expanded", "true");
+
+    fireEvent.click(purchasing);
+    expect(purchasing).toHaveAttribute("aria-expanded", "false");
+  });
+
+  it("auto-expands the section containing the active route on mount", () => {
+    mockPathname = "/admin/vessels"; // Cost Centres lives under Administration (manager mgmt → Purchasing)
+    render(<Sidebar userRole="MANAGER" />);
+
+    // /admin/vessels is in the Purchasing management block for a MANAGER.
+    const purchasing = headerButton("Purchasing");
+    expect(purchasing).toHaveAttribute("aria-expanded", "true");
+    expect(screen.getByRole("link", { name: /Cost Centres/i })).toBeInTheDocument();
+  });
+
+  it("keeps the PPMS brand outside any collapsible section", () => {
+    render(<Sidebar userRole="MANAGER" />);
+    // Brand text is always visible regardless of section state.
+    expect(screen.getByText("PPMS")).toBeInTheDocument();
+  });
+
+  it("renders the always-visible main links outside the sections", () => {
+    render(<Sidebar userRole="MANAGER" />);
+    expect(screen.getByRole("link", { name: /Dashboard/i })).toBeInTheDocument();
+    expect(screen.getByRole("link", { name: /My Profile/i })).toBeInTheDocument();
+  });
+
+  it("scopes revealed links to the opened section", () => {
+    render(<Sidebar userRole="MANAGER" />);
+    const administration = headerButton("Administration");
+    fireEvent.click(administration);
+
+    // Vendors appears under Administration for a manager.
+    const adminVendors = screen.getByRole("link", { name: /Vendors/i });
+    expect(adminVendors).toBeInTheDocument();
+    expect(within(adminVendors).queryByText("Vendors")).toBeTruthy();
+  });
+});