close the navigation audit's cross-cutting section

Records the pattern that came out of the refactor (path segment vs query param
vs stays-a-button), the grep that re-checks it, and the fact that BackButton —
which this section named as a standard building block — was dead and is gone.

Also folds in the two selections the audit never listed: the email folder and
the Soulseek room/peer rails. Neither was in the findings table; both were found
by sweeping for useGlobal<//useState after the listed rows were closed, which is
worth recording as the reason the table alone was not enough.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
2026-08-07 13:05:46 +00:00
co-authored by Claude Opus 5
parent 5a2ef6a0a0
commit 62b5b1db6f
+24 -8
View File
@@ -1,7 +1,10 @@
# Frontend Navigation Audit — the "opaque click" anti-pattern
> **Status, 2026-08-07. Phases 14 are closed except the two chat items.** Every finding H1H5 and M1M10
> is done, void, or decided-and-recorded, and so is every Phase 4 line. What remains, and only this:
> **Status, 2026-08-07. Phases 14 and the cross-cutting section are closed except the two chat items.**
> Every finding H1H5 and M1M10 is done, void, or decided-and-recorded, and so is every Phase 4 line and
> every cross-cutting line. Two selections that the audit never listed were found afterwards by sweeping
> for `useGlobal<` / `useState` rather than by reading the table — the email folder and the Soulseek
> room/peer rails — and are done too. What remains, and only this:
> **H4** (chat detail reading `sessionId` from `useParams` instead of the `chat:selected-session` channel)
> and the **"New Chat"** button that depends on it. Both are inside the chat nucleus, which has its own
> owner — they are not stalled here, they are somebody else's to land.
@@ -163,8 +166,9 @@ This is the primary surface to convert to URL-driven selection.
| `chat:selected-session` | open chat session | `/chat/:sessionId` | `ChatDetailPanel.tsx:136`, `SessionList.tsx:16` (H4) |
| `chat:active-cwd` | chat working dir | query param on `/chat` | `ChatDetailPanel.tsx:102`, `SessionList.tsx:14` |
| `SELECTED_DASHBOARD_KEY` (`useGlobal`) | selected dashboard | `/dashboards/:id` | `DashboardListApp.tsx:36`, `DashboardPreview.tsx:287` (H2) |
| `SELECTED_PROJECT` (`useGlobal`) | selected project | `/projects/:id` | `ProjectListApp.tsx:45`, `ProjectPreview.tsx:386` (H3) |
| local `useState` selections | search / room / conversation / peer / open file / preview slug | respective nested routes | `SearchView`, `SoulseekRooms`, `SoulseekChat`, `SoulseekUsers`, `CodeEditor/useEditorState`, `PreviewProvider` |
| ~~`SELECTED_PROJECT` (`useGlobal`)~~ | ~~selected project~~ | — | **void.** Projects was deleted 2026-07-30; H3 was a finding about code that no longer exists |
| ~~`EMAIL_FOLDER` (`useGlobal`)~~ | mailbox folder | `/email?folder=` | **done**`EmailList.tsx`. Not in the original map; found by sweeping for `useGlobal<` after the listed rows were closed, which is the only reason it is here |
| ~~local `useState` selections~~ | search / room / conversation / peer / open file / preview slug | respective nested routes | **done**`SearchView` (`?search=`), `SoulseekRooms` (`?room=`), `SoulseekChat` (`?peer=`), `SoulseekUsers` (`?user=`), `CodeEditor/useEditorState` (`?file=`). "Preview slug" named `PreviewProvider`, which is not in the repo — the finding was wrong |
**Excluded — event-bus / refresh signals, NOT selection:** `files:refresh-signal`,
`SLSKD_REFRESH_CHANNEL`, `MUSIC_RESYNC_CHANNEL`.
@@ -265,11 +269,23 @@ publishers means changing the chat panel, which is another agent's, so it is wri
- [x] Decided/skipped: Jobs step deep-link (a feature, not a fix — owner's call), Preview slug (**void**: no such app), FileBrowser widget (stays local, on M4's rule). Reasoning for each in the LOW section. (Browser tabs: **done** — see the LOW section. Monitor scope: **done** as M10, it was not a view toggle. Music favorites: **decided** — stays a channel, reasoning in the LOW section.)
### Cross-cutting for the refactor itself
- [ ] Standardise a URL-as-source-of-truth pattern for panel selection (replace the `usePanelChannel`/`useGlobal`
- [x] Standardise a URL-as-source-of-truth pattern for panel selection (replace the `usePanelChannel`/`useGlobal`
selection channels in the map above with `useParams`/`useSearchParams`, keeping channels only for
genuine signals/refresh buses).
- [ ] Adopt `<NavLink>` (real react-router) for all nav chrome so active state stops being JS-derived.
- [ ] Keep `NavLink.tsx` (query-string wrapper) and `BackButton.tsx` as the standard building blocks.
genuine signals/refresh buses). Done except `chat:selected-session` (H4), which is the chat agent's.
The pattern that came out of it, in order of preference: a **path segment** when the thing is what
the page is about (`/plans/:name`, `/jobs/:id`); a **query param** when it is a lens onto a page that
is about something else (`?folder=`, `?room=`, `?file=`) or when a master list needs a live preview
(`?selected=`). A **control that writes and then navigates stays a `<button>`** — a link cannot
express the "and then" — and only pure navigation becomes a `<Link>`.
Sweep to re-check this later: `grep -rn "usePanelChannel\|useGlobal<" src/workspaces/officerdev/src src/apps/officer-web`.
Everything it returns today is a registry, a refresh bus, a form draft, a connection state or a
preference — no selections.
- [x] Adopt `<NavLink>` (real react-router) for all nav chrome so active state stops being JS-derived.
Done — `39125b5`. Note `end={item.to === '/'}`: without it NavLink treats `/` as an ancestor of
every route, and with it on the rest a detail route would lose its tile.
- [x] Keep `NavLink.tsx` (query-string wrapper) as a standard building block. **`BackButton.tsx` is not one
— it had zero importers, was unchanged since `9ab0940`, and has been deleted.** The Jobs back control
is a plain `<Link to="/jobs">`, which is what the other two job panes already did.
---