From 8cef07a673debfd8e6a2d45288c939f8933ed40c Mon Sep 17 00:00:00 2001
From: Timothy <timothy.look@gmail.com>
Date: Sat, 18 Jul 2026 16:43:14 +0200
Subject: [PATCH] fix(396): mark active route in a default-collapsed group; fix
 mobile brand bar
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Cold-review findings:
- #1 (CONFIRMED): deep-linking/reloading into a route inside a default-collapsed
  labeled group (e.g. /app/settings, /app/libraries) left the expanded sidebar
  with no active indicator, violating "active route marked in both states". Now
  the group that contains the active route is force-shown (and its header shows
  expanded) WITHOUT persisting — navigating away reverts to the stored preference.
  Regression test added.
- #2 (mobile): a persisted-collapsed state hid the brand logo+wordmark while the
  toggle is also hidden at <=980px, leaving an empty header. Restore the brand in
  the mobile query (collapse is desktop-only).

Verified live (Playwright): /app/settings deep-link shows Settings active +
aria-current, System auto-revealed, Media stays collapsed, nothing persisted.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 web/src/App.test.tsx     | 16 ++++++++++++++++
 web/src/app/AppShell.tsx | 11 +++++++++--
 web/src/shell.css        | 13 +++++++++++++
 3 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/web/src/App.test.tsx b/web/src/App.test.tsx
index ef94623c4..e0baae7bf 100644
--- a/web/src/App.test.tsx
+++ b/web/src/App.test.tsx
@@ -720,6 +720,22 @@ describe('ChicoryTV SPA scaffold', () => {
       expect(container.querySelector('.ctv-app-shell')).toHaveClass('ctv-app-shell-collapsed');
       expect(screen.getByRole('button', { name: 'Expand sidebar' })).toBeInTheDocument();
     });
+
+    it('reveals + marks the active route even when its group is default-collapsed, without persisting', () => {
+      // Deep-link into a route inside the default-collapsed System group (expanded sidebar).
+      window.history.replaceState(null, '', '/app/settings');
+
+      render(<App />);
+
+      // The active item is rendered and marked, and its group header shows expanded…
+      const settingsLink = screen.getByRole('link', { name: 'Settings' });
+      expect(settingsLink).toHaveAttribute('aria-current', 'page');
+      expect(screen.getByRole('button', { name: 'System', expanded: true })).toBeInTheDocument();
+      // …but a sibling default-collapsed group (Media) stays collapsed…
+      expect(screen.queryByRole('link', { name: 'Libraries' })).not.toBeInTheDocument();
+      // …and the reveal did NOT write a stored preference (it reverts on navigating away).
+      expect(window.localStorage.getItem('ctv-sidebar-groups')).toBeNull();
+    });
   });
 });
 
diff --git a/web/src/app/AppShell.tsx b/web/src/app/AppShell.tsx
index 64097cf8b..bc596d35d 100644
--- a/web/src/app/AppShell.tsx
+++ b/web/src/app/AppShell.tsx
@@ -177,7 +177,14 @@ function Sidebar({
           // Accordions only apply in the expanded sidebar; the rail always shows every item
           // (separated by a hairline divider), so group-collapse is ignored there.
           const groupCollapsed = isGroupCollapsed(groupKey);
-          const showItems = collapsed || !groupCollapsed;
+          // Always reveal the group that contains the active route so its active item is visible +
+          // highlighted in the expanded sidebar, even when the group's stored state is collapsed
+          // (e.g. a fresh deep-link to /app/settings). This does NOT persist — navigating away
+          // reverts to the stored preference (acceptance: "active route marked in both states").
+          const containsActive =
+            activeRoute != null && ids.includes(activeRoute.id);
+          const expanded = containsActive || !groupCollapsed;
+          const showItems = collapsed || expanded;
 
           return (
             <Fragment key={groupKey}>
@@ -187,7 +194,7 @@ function Sidebar({
                 <button
                   type="button"
                   className="ctv-nav-group-header"
-                  aria-expanded={!groupCollapsed}
+                  aria-expanded={expanded}
                   onClick={() => onToggleGroup(groupKey)}
                 >
                   <span>{label}</span>
diff --git a/web/src/shell.css b/web/src/shell.css
index b6552e8f9..9a4698741 100644
--- a/web/src/shell.css
+++ b/web/src/shell.css
@@ -3816,6 +3816,19 @@ body {
     display: none;
   }
 
+  /* The collapse feature is desktop-only (the nav is hidden here). If the sidebar was collapsed on
+     desktop, don't let the rail's brand-hiding leave an empty header on mobile — restore the brand
+     (#396). */
+  .ctv-app-shell-collapsed .ctv-brand {
+    padding: 0 14px;
+    justify-content: flex-start;
+  }
+
+  .ctv-app-shell-collapsed .ctv-brand img,
+  .ctv-app-shell-collapsed .ctv-brand-wordmark {
+    display: revert;
+  }
+
   .ctv-topbar {
     flex-wrap: wrap;
     height: auto;