todo: files routing — what landed, and the half that did not
The URL model is done; rows are still opaque-click divs, which was the original finding and is now easier to fix than it was. Recorded honestly rather than leaving an entry that reads as untouched work. Also recorded the pattern behind the four bugs this produced: the list of overlay params has an owner, the acts that clear it do not, and every site deciding for itself is what let the breadcrumb ship wrong an hour after the rule was written. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -285,36 +285,42 @@ error boundaries** anywhere in the repo; and `dashboards.id` is a **global** pri
|
||||
|
||||
## Files app — routing
|
||||
|
||||
- [ ] **The files app needs a routing pass (noted 2026-08-15, after `31d8d725`).** Do this after the
|
||||
file-browser test pass, alongside the code-editor work above. `docs/navigation-audit.md` is the
|
||||
authority and names the anti-pattern; this is where it still lives.
|
||||
**The URL model is done (2026-08-15).** The folder is the pathname —
|
||||
`/files/archive/Tests/platform/docs` — and `?view=` is a NAME within it, not a second copy of the path.
|
||||
Deep links and refresh work, which they never had. Landed across `559560de`, `98ba604a`, `7e31564c`,
|
||||
`0dc88b51`, `7359af2f`, with `files-route.ts` + 23 tests as the one owner of encoding and of which params
|
||||
belong to the overlay.
|
||||
|
||||
**What prompted it.** Closing a viewer pane sent you back to home. `onCloseViewer` was
|
||||
`setSearchParams({})` — a wholesale wipe that took `?path=` with it. Fixed, but the shape of the bug
|
||||
is the point: four handlers touched the same query string and only one of them wiped it, because no
|
||||
single thing owns that URL.
|
||||
Four bugs came out of that work and are fixed: closing a pane sent you home; `?view=` was stripped on
|
||||
every page load by a mount effect that had made deep links impossible since 2026-02-23; navigating kept a
|
||||
pane belonging to the folder you left; and the breadcrumb opted out of that last rule because it is a
|
||||
`<Link>` and never called the function enforcing it.
|
||||
|
||||
**Verified state, so the next pass does not have to re-derive it:**
|
||||
- [ ] **Rows are still not links.** This is the half that did not get done, and it is the original
|
||||
finding. Both list and grid render `<div onDoubleClick>` (`FileItem.tsx:701,748`), and search
|
||||
results render `<div onClick>` (`FileViewContainer.tsx:187`) — the "opaque click" that
|
||||
`docs/navigation-audit.md` names. No cmd-click into a new tab, no middle-click, not
|
||||
link-focusable, and the target lives in a closure rather than the DOM.
|
||||
|
||||
- **Rows are not links.** Both list and grid render a `<div>` with `onDoubleClick`
|
||||
(`FileItem.tsx:701,748`) — the "opaque click" the audit names. No cmd-click into a new tab, no
|
||||
middle-click, not link-focusable, and the target is in a closure rather than the DOM.
|
||||
- **Search results are the same** — `<div onClick={() => handleSearchResultClick(entry)}>`
|
||||
(`FileViewContainer.tsx:185`), and that handler both navigates the folder AND opens the viewer.
|
||||
- **The breadcrumb already got it right** and is the model to copy: it renders a real `<Link>` when
|
||||
given `searchForPath`, and falls back to a `<button>` only for a panel that does not own the
|
||||
address bar (`Breadcrumb.tsx:21-26`).
|
||||
- **A panel's folder is not addressable.** `useFileBrowserApp(basePath, root, urlPath)` keeps the
|
||||
path in the URL only when `urlPath` is true — the `/files` screen. Every panel instance uses local
|
||||
state, so it always opens at its base and its location cannot be linked or restored. Deliberate as
|
||||
written; worth re-deciding rather than inheriting.
|
||||
- **Two path-ish params coexist**: `?path=` is where you are, `?view=` is what is on top. The
|
||||
ephemeral set (`view`, `ephemeral`, `ephemeral2`, `chatContext`, …) is listed once in
|
||||
`useFileViewerPanels.EPHEMERAL_KEYS` and that list is the closest thing to an owner.
|
||||
It is now much easier than it was: `folderHref(path)` gives a folder's URL and `hrefForPath` gives
|
||||
a full `To` with the overlay already stripped, so a row becomes
|
||||
`<Link to={hrefForPath(childPath)}>` and a file row becomes a link that sets `?view=<name>`. The
|
||||
breadcrumb has done exactly this since before today and is the model.
|
||||
|
||||
**The shape of the fix**, if it holds up: rows become `<Link to={{ search: … }}>` like the
|
||||
breadcrumb, so open-in-new-tab works and the target is in the DOM; one helper owns "set the viewer
|
||||
params" and "clear the viewer params" so a fifth handler cannot get it wrong the way the fourth did.
|
||||
Watch for: a row is also a click target for SELECTION (single click selects, double opens,
|
||||
shift/cmd extend). An anchor changes what those gestures mean by default, so the selection
|
||||
handlers have to keep working and `preventDefault` where they win.
|
||||
|
||||
- [ ] **The acts that clear the overlay have no owner, only the list does.** `VIEWER_PARAMS` and
|
||||
`withoutViewerParams` live in `files-route.ts` and are shared. But every site still decides FOR
|
||||
ITSELF whether to call them, which is precisely how the breadcrumb shipped wrong an hour after the
|
||||
rule was written. Three bugs in a row came from this. A single `navigateToFolder()` that every
|
||||
caller must go through — rather than a helper they may remember — is the fix if a fourth appears.
|
||||
|
||||
- [ ] **A panel's folder is still not addressable.** `urlPath` is true only on the `/files` screen;
|
||||
panels keep the path in local state, so they always open at their base and cannot be linked or
|
||||
restored. Deliberate as written — a dashboard can hold two browsers and one URL cannot serve both —
|
||||
but worth re-deciding rather than inheriting.
|
||||
|
||||
## Known bugs
|
||||
|
||||
|
||||
Reference in New Issue
Block a user