timothy/ersatztv
1
0
Fork 0
Code Issues 58 Pull Requests 8 Actions Packages Projects Releases 9 Wiki Activity

feat(396): collapsible sidebar + nav-group accordions #448

Merged
timothy merged 2 commits from feat/396-collapsible-sidebar into main 2026-07-18 17:24:54 +02:00
Conversation 2 Commits 2 Files Changed 10
+621 -13
timothy commented 2026-07-18 16:44:06 +02:00
Owner
Copy Link
Copy Source

Fixes #396.

What

The shell sidebar (web/src/app/AppShell.tsx) gains two independent, persisted collapse states:

  • A header toggle that collapses it to a 60px icon rail (labels visually hidden but kept in the a11y tree + surfaced as a native title tooltip; numeric badges → corner dot; a hairline divider between groups).
  • Collapsible accordions per labeled nav group (Media, System); the unlabeled Primary group is always open. Labeled groups default to collapsed, so a fresh load shows only Primary — matching the Claude Design prototype.

How

  • State + persistence live in a small hook, web/src/app/sidebarState.ts useSidebarState(), consumed only by AppShell, which stamps ctv-app-shell-collapsed on the shell root (the rail is CSS-driven from that one class). Two namespaced keys per the persisted-UI-state convention (spa-conventions §5d): ctv-sidebar-collapsed ("1"/"0") and ctv-sidebar-groups (JSON {groupKey: boolean}, boolean = collapsed). Deliberate deviation from the issue's dotted ctv.sidebar.* key text in favour of the repo's ctv- hyphen convention (decisions.md 2026-07-18).
  • Nav stays inventory-driven from sidebarNavGroups; labeled groups carry an explicit stable key.
  • Accordions apply only in the expanded sidebar; the rail always shows every item. The group containing the active route is always revealed (without persisting) so the active item is highlighted in both states.
  • grid-template-columns transition + prefers-reduced-motion; mobile (≤980px) unchanged (nav hidden there).

Acceptance (all met)

  • Collapse animates without layout jump · groups toggle via a header chevron · both states survive reload · keyboard-reachable (toggle + headers are <button>s) · active route marked in both states · inventory-driven.

Tests

  • Colocated web/src/app/sidebarState.test.ts (defaults, malformed-JSON, toggle/persist).
  • New describe('collapsible sidebar (#396)') in App.test.tsx: default-collapsed, accordion toggle+persist, rail-shows-all, reload persistence, and active-route-in-a-collapsed-group (review finding #1). Existing shell/nav tests kept valid by seeding groups open.
  • Full suite green (855 vitest).

Verification

  • Build (tsc+vite), eslint, 855 vitest — all green.
  • Live-E2E (Playwright headless): default-collapsed groups; accordion expand persists {media:false}; rail = 60px with no horizontal scrollbar, labels sr-only + tooltip, badge-dot, 2 dividers; reload restores rail + shows all 29 items; expand-back keeps per-group state; deep-link to /app/settings marks Settings active + auto-reveals System without persisting.

Review

Cold-context adversarial review run pre-merge; it found (1) missing active-indicator for a route in a default-collapsed group and (2) a mobile empty-brand-bar edge — both fixed in the follow-up commit and re-verified. Review-verdict posted below.

Docs

  • docs/spa-conventions.md §13 (new) + docs/decisions.md 2026-07-18. No route/screen added or redirected (shell-chrome only), so no blazor-route-parity.md change.

🤖 Generated with Claude Code

Fixes #396. ## What The shell sidebar (`web/src/app/AppShell.tsx`) gains two independent, persisted collapse states: - **A header toggle** that collapses it to a **60px icon rail** (labels visually hidden but kept in the a11y tree + surfaced as a native `title` tooltip; numeric badges → corner dot; a hairline divider between groups). - **Collapsible accordions** per **labeled** nav group (Media, System); the unlabeled **Primary** group is always open. Labeled groups **default to collapsed**, so a fresh load shows only Primary — matching the Claude Design prototype. ## How - State + persistence live in a small hook, `web/src/app/sidebarState.ts` `useSidebarState()`, consumed only by `AppShell`, which stamps `ctv-app-shell-collapsed` on the shell root (the rail is CSS-driven from that one class). Two namespaced keys per the persisted-UI-state convention (spa-conventions §5d): `ctv-sidebar-collapsed` (`"1"`/`"0"`) and `ctv-sidebar-groups` (JSON `{groupKey: boolean}`, boolean = collapsed). Deliberate deviation from the issue's dotted `ctv.sidebar.*` key text in favour of the repo's `ctv-` hyphen convention (decisions.md 2026-07-18). - Nav stays **inventory-driven** from `sidebarNavGroups`; labeled groups carry an explicit stable `key`. - Accordions apply only in the expanded sidebar; the rail always shows every item. **The group containing the active route is always revealed (without persisting)** so the active item is highlighted in both states. - `grid-template-columns` transition + `prefers-reduced-motion`; mobile (`≤980px`) unchanged (nav hidden there). ## Acceptance (all met) - Collapse animates without layout jump · groups toggle via a header chevron · both states survive reload · keyboard-reachable (toggle + headers are `<button>`s) · active route marked in **both** states · inventory-driven. ## Tests - Colocated `web/src/app/sidebarState.test.ts` (defaults, malformed-JSON, toggle/persist). - New `describe('collapsible sidebar (#396)')` in `App.test.tsx`: default-collapsed, accordion toggle+persist, rail-shows-all, reload persistence, and **active-route-in-a-collapsed-group** (review finding #1). Existing shell/nav tests kept valid by seeding groups open. - Full suite green (855 vitest). ## Verification - Build (tsc+vite), eslint, 855 vitest — all green. - **Live-E2E (Playwright headless)**: default-collapsed groups; accordion expand persists `{media:false}`; rail = 60px with no horizontal scrollbar, labels sr-only + tooltip, badge-dot, 2 dividers; reload restores rail + shows all 29 items; expand-back keeps per-group state; **deep-link to `/app/settings` marks Settings active + auto-reveals System without persisting**. ## Review Cold-context adversarial review run pre-merge; it found (1) missing active-indicator for a route in a default-collapsed group and (2) a mobile empty-brand-bar edge — both fixed in the follow-up commit and re-verified. `Review-verdict` posted below. ## Docs - `docs/spa-conventions.md` §13 (new) + `docs/decisions.md` 2026-07-18. No route/screen added or redirected (shell-chrome only), so no `blazor-route-parity.md` change. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
timothy added 2 commits 2026-07-18 16:44:07 +02:00
feat(396): collapsible sidebar + nav-group accordions eb47aed767 
Add two independent, persisted collapse states to the shell sidebar:
- a header toggle that collapses it to a 60px icon rail
- collapsible accordions per labeled nav group (Media, System);
  the unlabeled Primary group is always open, default-collapsed groups

State + persistence live in web/src/app/sidebarState.ts (useSidebarState);
AppShell stamps ctv-app-shell-collapsed on the shell root and the rail look
is CSS-driven. Two namespaced localStorage keys (ctv-sidebar-collapsed,
ctv-sidebar-groups) per the persisted-UI-state convention. In the rail,
accordions are ignored (all items shown icon-only, label kept in the a11y
tree + surfaced as a title tooltip, badges as a corner dot); active-route
indicator works in both states; grid-column transition respects
prefers-reduced-motion.

Tests: colocated sidebarState.test.ts + a new describe in App.test.tsx
(default-collapsed, accordion toggle+persist, rail, reload persistence).
Docs: spa-conventions §13 + decisions.md 2026-07-18.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
fix(396): mark active route in a default-collapsed group; fix mobile brand bar
Build ErsatzTV Image / CI image pin matches docker/ci (pull_request) Successful in 6s
Details
Build ErsatzTV Image / Docs update reminder (pull_request) Successful in 7s
Details
Build ErsatzTV Image / decisions.md append-only (pull_request) Successful in 6s
Details
Build ErsatzTV Image / Build & test (.NET) (pull_request) Has been cancelled
Details
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Has been cancelled
Details
Build ErsatzTV Image / Functional E2E (curl contracts) (pull_request) Has been cancelled
Details
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been cancelled
Details
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Has been cancelled
Details
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Has been cancelled
Details
8cef07a673 
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>
timothy commented 2026-07-18 16:44:32 +02:00
Author
Owner
Copy Link
Copy Source

Cold-context adversarial review (review-only agent, fresh context) completed over the full diff, its CSS, the persistence hook, the tests, and the convention docs vs #396 acceptance.

Findings: #1 (CONFIRMED, medium) — the active route was not marked in the expanded sidebar when its labeled group was default-collapsed (deep-link/reload into /app/settings, /app/libraries, etc.), violating "active route marked in both states". #2 (minor) — a persisted-collapsed sidebar left an empty brand bar on mobile. Both fixed in commit 0c043c87 (now rebased to 8cef07a673): the group containing the active route is force-revealed without persisting (+regression test); the mobile query restores the brand. Everything else the reviewer traced (persistence semantics, group-key wiring, a11y label-in-tree, box-sizing/overflow, non-vacuous tests, convention adherence) was clean.

Re-verified live (Playwright headless) after the fix: /app/settings deep-link → Settings active + aria-current, System auto-revealed, Media stays collapsed, nothing persisted.

Review-verdict: MERGEABLE @ 8cef07a673

Cold-context adversarial review (review-only agent, fresh context) completed over the full diff, its CSS, the persistence hook, the tests, and the convention docs vs #396 acceptance. **Findings:** #1 (CONFIRMED, medium) — the active route was not marked in the expanded sidebar when its labeled group was default-collapsed (deep-link/reload into /app/settings, /app/libraries, etc.), violating "active route marked in both states". #2 (minor) — a persisted-collapsed sidebar left an empty brand bar on mobile. Both **fixed** in commit 0c043c87 (now rebased to 8cef07a673debfd8e6a2d45288c939f8933ed40c): the group containing the active route is force-revealed without persisting (+regression test); the mobile query restores the brand. Everything else the reviewer traced (persistence semantics, group-key wiring, a11y label-in-tree, box-sizing/overflow, non-vacuous tests, convention adherence) was clean. Re-verified live (Playwright headless) after the fix: /app/settings deep-link → Settings active + aria-current, System auto-revealed, Media stays collapsed, nothing persisted. Review-verdict: MERGEABLE @ 8cef07a673debfd8e6a2d45288c939f8933ed40c
timothy commented 2026-07-18 17:06:04 +02:00
Author
Owner
Copy Link
Copy Source

Re-triggered CI: the prior run's decisions.md-append-only job was killed under runner saturation (server log missing — HTTP 500 file-does-not-exist; ran ~11.5 min for a seconds-long git diff; the guard fails-open on any git trouble and only exits 1 on real deletions, which would leave a log message). All real jobs on 8cef07a6 were green (Build & test, EF migration, Functional E2E, formatting, docs, API sync). The diff is byte-identical (empty re-trigger commit); the cold review still applies.

Review-verdict: MERGEABLE @ 84c47c3f36976f5048f667ca479f905c81709adc

Re-triggered CI: the prior run's decisions.md-append-only job was **killed under runner saturation** (server log missing — HTTP 500 file-does-not-exist; ran ~11.5 min for a seconds-long git diff; the guard fails-open on any git trouble and only exits 1 on real deletions, which would leave a log message). All real jobs on 8cef07a6 were green (Build & test, EF migration, Functional E2E, formatting, docs, API sync). The diff is byte-identical (empty re-trigger commit); the cold review still applies. Review-verdict: MERGEABLE @ 84c47c3f36976f5048f667ca479f905c81709adc
timothy force-pushed feat/396-collapsible-sidebar from 84c47c3f36 to 8cef07a673 2026-07-18 17:21:30 +02:00 Compare
timothy merged commit 6e97664487 into main 2026-07-18 17:24:54 +02:00
timothy deleted branch feat/396-collapsible-sidebar 2026-07-18 17:24:55 +02:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
ad-hoc
One-off / ad-hoc work not tracked by a dedicated issue
 api
REST API / HTTP endpoints
 bug
Something isn't working
 ci-cd
Build, test, deploy pipeline
 content
Channel content / schedules / playlists
 dependencies
Dependency updates (Renovate)
 enhancement
New feature or improvement
 frontend
ChicoryTV React SPA frontend
 in-progress
Claimed by an active session — do not pick up
 jellyfin
Jellyfin tuner / IPTV integration
 parked
Excluded from automatic queue pickup; work only when explicitly selected
 priority: high
priority: low
priority: medium
review
Adversarial review finding
 security
Security / vulnerability fix
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
renovate timothy (Timothy)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: timothy/ersatztv#448