put the browsed folder in the url on /files

the file browser's currentPath was useState, so back and forward did nothing and a
folder could not be linked to. it is `?path=` now on /files, and the breadcrumbs are
real links.

opt-in, keyed on the parsed workspace identity rather than the base path: a dashboard
can hold two file browsers and one shared param would move both, while an unscoped
panel (cwd `~`) sits on dashboards too, so `basePath === '/'` would have caught the
wrong ones.

two things the audit line did not know. `?view=` is ephemeral — useFileViewerPanels
wipes it on mount — so `path` is this screen's first durable param. and four
setSearchParams({...}) calls replaced the whole query string, which would have made
opening any file silently reset the folder to home; they go through a setViewerParams
helper now that carries `path` across.

folder rows stay buttons. cmd/ctrl/shift-click is already multi-select in FileItem and
open is double-click, so anchor semantics collide with a gesture that exists. that is a
product decision, not a defect — written up for the owner rather than guessed at.
This commit is contained in:
2026-08-07 12:28:49 +00:00
parent ec4aaaae8a
commit 5daa598b63
5 changed files with 95 additions and 27 deletions
+5 -5
View File
@@ -42,9 +42,9 @@ was in this list; the Projects feature was removed end to end on 2026-07-30, so
and navigate-site 22 below still name it — they are the record of work that happened, not of code that exists.) and navigate-site 22 below still name it — they are the record of work that happened, not of code that exists.)
**Section routes** (a param that names a view rather than an entity): `/settings/:page/:section`, **Section routes** (a param that names a view rather than an entity): `/settings/:page/:section`,
`/soulseek/:section`, `/headscale/:section`, `/wallet/:section`, `/system-monitor/:scope`. `/soulseek/:section`, `/headscale/:section`, `/wallet/:section`, `/system-monitor/:scope`.
**Query-param screens:** `/music?path=`. **Query-param screens:** `/music?path=`, `/files?path=`.
**Genuinely flat, still:** `/` `/files` `/plans` `/terminal` `/desktop` `/code-editor` `/qr-transfer`. **Genuinely flat, still:** `/` `/plans` `/terminal` `/desktop` `/code-editor` `/qr-transfer`.
`/files` and `/code-editor` are M4 and M5 — the last two that ought not to be. `/code-editor` is M5 — the last one that ought not to be.
The five settings pages are route **pairs** now, not flat screens — `/settings/{profile,ai,system,integrations,user-management}` The five settings pages are route **pairs** now, not flat screens — `/settings/{profile,ai,system,integrations,user-management}`
plus a `:section` each (M6). There has never been a `/settings/apps`; that entry was wrong when this list plus a `:section` each (M6). There has never been a `/settings/apps`; that entry was wrong when this list
was written. was written.
@@ -80,7 +80,7 @@ are good building blocks. The **Workspace/Panel framework** contains **zero** ro
| ~~M1~~ | ~~`Screens/Dashboard/CapabilityPage.tsx:431`~~ | task / skill / process | `/tasks/:dirName`, `/skills/:dirName`, `/processes/:dirName` | **Done.** One component backed three screens, so one change covered all of them. The auto-select-`items[0]` effect is gone — the bare route is now the list with an empty detail pane. `editing`/`isNew` moved to `?edit=1` / `?new=1` because a `<Link>` row cannot imperatively reset them. | | ~~M1~~ | ~~`Screens/Dashboard/CapabilityPage.tsx:431`~~ | task / skill / process | `/tasks/:dirName`, `/skills/:dirName`, `/processes/:dirName` | **Done.** One component backed three screens, so one change covered all of them. The auto-select-`items[0]` effect is gone — the bare route is now the list with an empty detail pane. `editing`/`isNew` moved to `?edit=1` / `?new=1` because a `<Link>` row cannot imperatively reset them. |
| ~~M2~~ | ~~`Screens/Dashboard/TaskLogs/index.tsx:104`~~ | a task-log run | `/task-logs/:id` | **Done.** As predicted — the detail fetch already keyed off the id, so only its source changed. `showDetail` is gone; the mobile swap and both back arrows derive from the param. | | ~~M2~~ | ~~`Screens/Dashboard/TaskLogs/index.tsx:104`~~ | a task-log run | `/task-logs/:id` | **Done.** As predicted — the detail fetch already keyed off the id, so only its source changed. `showDetail` is gone; the mobile swap and both back arrows derive from the param. |
| ~~M3~~ | ~~`Screens/Dashboard/Activity/ActivityScreen.tsx:63,73`~~ | background task / detached job | `/activity/:id` | **Done.** One param for both row kinds; the screen looks the id up in the polled registry and derives `task=`/`path=` from the row. The SSE effect now depends on that derived *string*, so the 3s poll no longer risks re-opening the stream. An id that has left the registry says so instead of hanging on "waiting for output". | | ~~M3~~ | ~~`Screens/Dashboard/Activity/ActivityScreen.tsx:63,73`~~ | background task / detached job | `/activity/:id` | **Done.** One param for both row kinds; the screen looks the id up in the polled registry and derives `task=`/`path=` from the row. The SSE effect now depends on that derived *string*, so the 3s poll no longer risks re-opening the stream. An id that has left the registry says so instead of hanging on "waiting for output". |
| M4 | `FileBrowser/.../useFileBrowserApp.ts:269`, `FileItem.tsx:516`, `Breadcrumb.tsx:16`, search-hit `:249` | a folder | `/files?path=<dir>` | **files** already open via `?view=`; **folders** are pure `currentPath` state — no URL, no back/forward. Folder rows + crumbs → `<Link>` on a `?path=` param. | | ~~M4~~ | ~~`FileBrowser/.../useFileBrowserApp.ts:269`~~, `FileItem.tsx:516`, ~~`Breadcrumb.tsx:16`~~ | a folder | `/files?path=<dir>` | **Partly done — the rest is an owner decision, not a defect.** `currentPath` is `?path=` on `/files`, so back/forward and linking a folder work, and the crumbs are `<Link>`s. Two things the audit line did not know: `?view=` is *ephemeral* (wiped on mount by `useFileViewerPanels`), so `path` is the screen's first durable param, and four `setSearchParams({…})` calls replaced the whole query string — opening any file would have silently reset the folder. They go through a `setViewerParams` helper now that keeps `path`. Opt-in via the parsed `WorkspaceIdentity` (`screens/files`), because a dashboard can hold two browsers and one shared param would move both. **Folder *items* stay buttons:** ⌘/Ctrl/Shift-click is already bound to multi-select in `FileItem.tsx` and open is double-click, so anchor semantics collide with an existing gesture. |
| M5 | `CodeEditor/FileTree.tsx:59`, `EditorTabs.tsx:33` | open source file / active tab | `/code-editor?file=<path>` (+ `open=` for tabs) | tree file rows → `<Link>`; active file in a param. | | M5 | `CodeEditor/FileTree.tsx:59`, `EditorTabs.tsx:33` | open source file / active tab | `/code-editor?file=<path>` (+ `open=` for tabs) | tree file rows → `<Link>`; active file in a param. |
| ~~M6~~ | `Settings/SettingsPanel.tsx` | a settings sub-section | `/settings/:page/:section` | **Done.** `<NavLink>` + `useParams`, five `*_SELECTED` globals gone, one `SettingsRoute` guard per page. The "one change covers all settings pages" claim was *almost* right: Integrations builds its own sidebar and did not go through `createSettingsPanelComponents`, and it also held the Enterprise/Personal tab in a second global — derived from the section key now, which is what fixes deep-linking a Personal section. | | ~~M6~~ | `Settings/SettingsPanel.tsx` | a settings sub-section | `/settings/:page/:section` | **Done.** `<NavLink>` + `useParams`, five `*_SELECTED` globals gone, one `SettingsRoute` guard per page. The "one change covers all settings pages" claim was *almost* right: Integrations builds its own sidebar and did not go through `createSettingsPanelComponents`, and it also held the Enterprise/Personal tab in a second global — derived from the section key now, which is what fixes deep-linking a Personal section. |
| M7 | `workspaces/components/Combobox.tsx:53` | caller-supplied route | (existing) | shared widget: `href`-bearing options do `navigate(option.href)`; render them as `<Link>`. Every caller inherits the opaque click. | | M7 | `workspaces/components/Combobox.tsx:53` | caller-supplied route | (existing) | shared widget: `href`-bearing options do `navigate(option.href)`; render them as `<Link>`. Every caller inherits the opaque click. |
@@ -217,7 +217,7 @@ publishers means changing the chat panel, which is another agent's, so it is wri
- [x] **M1** Capabilities → `/tasks|skills|processes/:dirName`, rows → `<Link>`; `CapabilityPage` takes an explicit `basePath` (not reused from `endpoint`, which only happens to match). Selection is `useParams`, the mobile pane swap and back arrow are derived from it, delete navigates to the bare route, and the two per-item modes are `?edit=1` / `?new=1`. No `<Navigate>` guard: an unknown `dirName` gets the empty detail pane. **Needs runtime test.** - [x] **M1** Capabilities → `/tasks|skills|processes/:dirName`, rows → `<Link>`; `CapabilityPage` takes an explicit `basePath` (not reused from `endpoint`, which only happens to match). Selection is `useParams`, the mobile pane swap and back arrow are derived from it, delete navigates to the bare route, and the two per-item modes are `?edit=1` / `?new=1`. No `<Navigate>` guard: an unknown `dirName` gets the empty detail pane. **Needs runtime test.**
- [x] **M2** TaskLogs → `/task-logs/:id`; rows are `<Link>`s, `showDetail` deleted. **Needs runtime test.** - [x] **M2** TaskLogs → `/task-logs/:id`; rows are `<Link>`s, `showDetail` deleted. **Needs runtime test.**
- [x] **M3** Activity → `/activity/:id`; the `{label, query}` selection object is gone — the id is the URL and the stream query is derived from the registry row. `/activity` also had no `usePageTitle` rule (it read "Officer"); added. **Needs runtime test.** - [x] **M3** Activity → `/activity/:id`; the `{label, query}` selection object is gone — the id is the URL and the stream query is derived from the registry row. `/activity` also had no `usePageTitle` rule (it read "Officer"); added. **Needs runtime test.**
- [ ] **M4** FileBrowser folders → `/files?path=`; folder rows + breadcrumbs `<Link>` (`useFileBrowserApp.ts:269`, `FileItem.tsx`, `Breadcrumb.tsx`). - [x] **M4** FileBrowser folders → `/files?path=`; breadcrumbs are `<Link>`s. Folder *rows* deliberately still buttons — ⌘-click is multi-select, open is double-click; converting them needs an owner call on the gesture.
- [ ] **M5** CodeEditor active file → `/code-editor?file=` (`FileTree.tsx:59`, `EditorTabs.tsx:33`). - [ ] **M5** CodeEditor active file → `/code-editor?file=` (`FileTree.tsx:59`, `EditorTabs.tsx:33`).
- [ ] **M7** Combobox: render `href` options as `<Link>` (`Combobox.tsx:53`). - [ ] **M7** Combobox: render `href` options as `<Link>` (`Combobox.tsx:53`).
@@ -11,16 +11,23 @@ import { useFileBrowserApp } from './useFileBrowserApp';
type FileBrowserAppProps = { type FileBrowserAppProps = {
basePath?: string; basePath?: string;
rootOverride?: string; rootOverride?: string;
/** Put the current folder in `?path=`. Opt-in — only the /files screen is guaranteed one browser. */
urlPath?: boolean;
}; };
export const FileBrowserApp = ({ basePath = '/', rootOverride }: FileBrowserAppProps) => { export const FileBrowserApp = ({ basePath = '/', rootOverride, urlPath }: FileBrowserAppProps) => {
const fileBrowserManager = useFileBrowserApp(basePath, rootOverride); const fileBrowserManager = useFileBrowserApp(basePath, rootOverride, urlPath);
const { handleNavigate } = fileBrowserManager; const { handleNavigate } = fileBrowserManager;
return ( return (
<div className="flex flex-col h-full overflow-hidden"> <div className="flex flex-col h-full overflow-hidden">
<Toolbar fileBrowserManager={fileBrowserManager} /> <Toolbar fileBrowserManager={fileBrowserManager} />
<Breadcrumb path={fileBrowserManager.currentPath} onNavigate={handleNavigate} basePath={basePath} /> <Breadcrumb
path={fileBrowserManager.currentPath}
onNavigate={handleNavigate}
basePath={basePath}
searchForPath={fileBrowserManager.searchForPath}
/>
<UploadProgress fileBrowserManager={fileBrowserManager} /> <UploadProgress fileBrowserManager={fileBrowserManager} />
<FileViewContainer fileBrowserManager={fileBrowserManager} /> <FileViewContainer fileBrowserManager={fileBrowserManager} />
<TaskRunnerDialog fileBrowserManager={fileBrowserManager} /> <TaskRunnerDialog fileBrowserManager={fileBrowserManager} />
@@ -4,7 +4,14 @@ import { FileBrowserApp } from './FileBrowserApp';
const cwdToPath = (cwd: string) => (cwd === '~' ? '/' : cwd.slice(1)); const cwdToPath = (cwd: string) => (cwd === '~' ? '/' : cwd.slice(1));
export const FileBrowserPanelWrapper = () => { export const FileBrowserPanelWrapper = () => {
const { cwd } = useWorkspace(); const { cwd, workspace } = useWorkspace();
const basePath = cwdToPath(cwd); const basePath = cwdToPath(cwd);
return <FileBrowserApp basePath={basePath} rootOverride={basePath !== '/' ? 'home' : undefined} />;
// Only the /files screen puts its folder in the URL. A dashboard can hold two file browsers, and one
// shared `?path=` would move both — so the address bar belongs to the workspace that is guaranteed to
// host exactly one. The test is the framework's parsed identity, not the base path: an unscoped panel
// (cwd `~`) sits on dashboards too, so `basePath === '/'` would catch the wrong browsers.
const urlPath = workspace?.kind === 'screen' && workspace.id === 'files';
return <FileBrowserApp basePath={basePath} rootOverride={basePath !== '/' ? 'home' : undefined} urlPath={urlPath} />;
}; };
@@ -1,25 +1,43 @@
import { Link } from 'react-router';
import { ChevronRight, Home } from 'lucide-react'; import { ChevronRight, Home } from 'lucide-react';
type BreadcrumbProps = { type BreadcrumbProps = {
path: string; path: string;
onNavigate: (path: string) => void; onNavigate: (path: string) => void;
basePath?: string; basePath?: string;
/**
* Query string for a folder, when this browser owns the address bar. Given, the crumbs are real links
* and cmd-click opens the folder in a new tab; absent (a dashboard panel, the widget) they stay buttons,
* because that browser's location is not in the URL and there would be nothing for the new tab to read.
*/
searchForPath?: ((path: string) => string) | null;
}; };
export const Breadcrumb = ({ path, onNavigate, basePath = '/' }: BreadcrumbProps) => { export const Breadcrumb = ({ path, onNavigate, basePath = '/', searchForPath }: BreadcrumbProps) => {
const relativePath = basePath !== '/' && path.startsWith(basePath) ? path.slice(basePath.length) : path; const relativePath = basePath !== '/' && path.startsWith(basePath) ? path.slice(basePath.length) : path;
const segments = relativePath.split('/').filter(Boolean); const segments = relativePath.split('/').filter(Boolean);
const Crumb = ({ to, children, className }: { to: string; children: React.ReactNode; className: string }) =>
searchForPath ? (
<Link to={{ search: searchForPath(to) }} className={className}>
{children}
</Link>
) : (
<button onClick={() => onNavigate(to)} className={className}>
{children}
</button>
);
return ( return (
<div className="shrink-0 border-b border-duck-dark/10 px-4 py-2"> <div className="shrink-0 border-b border-duck-dark/10 px-4 py-2">
<nav className="flex items-center gap-1 text-sm flex-wrap"> <nav className="flex items-center gap-1 text-sm flex-wrap">
<button <Crumb
onClick={() => onNavigate(basePath)} to={basePath}
className="flex items-center gap-1 text-duck-forest hover:text-duck-teal transition-colors cursor-pointer font-medium" className="flex items-center gap-1 text-duck-forest hover:text-duck-teal transition-colors cursor-pointer font-medium"
> >
<Home className="h-4 w-4" /> <Home className="h-4 w-4" />
<span>{basePath === '/' ? 'home' : basePath.split('/').pop()}</span> <span>{basePath === '/' ? 'home' : basePath.split('/').pop()}</span>
</button> </Crumb>
{segments.map((segment, i) => { {segments.map((segment, i) => {
const relative = '/' + segments.slice(0, i + 1).join('/'); const relative = '/' + segments.slice(0, i + 1).join('/');
@@ -32,12 +50,12 @@ export const Breadcrumb = ({ path, onNavigate, basePath = '/' }: BreadcrumbProps
{isLast ? ( {isLast ? (
<span className="text-duck-dark font-semibold">{segment}</span> <span className="text-duck-dark font-semibold">{segment}</span>
) : ( ) : (
<button <Crumb
onClick={() => onNavigate(segmentPath)} to={segmentPath}
className="text-duck-forest hover:text-duck-teal transition-colors cursor-pointer font-medium" className="text-duck-forest hover:text-duck-teal transition-colors cursor-pointer font-medium"
> >
{segment} {segment}
</button> </Crumb>
)} )}
</span> </span>
); );
@@ -8,21 +8,55 @@ import { useUserState } from 'state/useUserState';
import { useAuth } from 'hooks/useAuth'; import { useAuth } from 'hooks/useAuth';
import { useFilesRefresh } from '../../../channels'; import { useFilesRefresh } from '../../../channels';
export const useFileBrowserApp = (basePath: string, rootOverride?: string) => { /**
* Where the browser is looking, when it is the one browser that owns the address bar. `urlPath` is opt-in
* because the same app mounts as a dashboard panel, and a dashboard can hold two of them — one shared
* param would move both. The /files screen holds exactly one, so there it is the source of truth.
*/
export const FILES_PATH_PARAM = 'path';
/** The base folder is the bare address, so it drops the param rather than spelling itself out. */
const withPath = (prev: URLSearchParams, path: string, basePath: string) => {
const next = new URLSearchParams(prev);
if (path === basePath) next.delete(FILES_PATH_PARAM);
else next.set(FILES_PATH_PARAM, path);
return next;
};
export const useFileBrowserApp = (basePath: string, rootOverride?: string, urlPath = false) => {
const { user } = useAuth(); const { user } = useAuth();
const navigate = useNavigate(); const navigate = useNavigate();
const [searchParams, setSearchParams] = useSearchParams(); const [searchParams, setSearchParams] = useSearchParams();
const homeRoot = 'home'; const homeRoot = 'home';
const [, setGlobalPath] = useUserState<string>('files/currentPath', '/'); const [, setGlobalPath] = useUserState<string>('files/currentPath', '/');
const [currentPath, setLocalPath] = useState(basePath); const [localPath, setLocalPath] = useState(basePath);
const scoped = basePath !== '/'; const scoped = basePath !== '/';
// Navigation is session-local — the browser always opens at home, never restoring the // Navigation is session-local for a panel — it always opens at its base and never restores the last
// last path. The main (unscoped) browser still mirrors its folder into files/currentPath // path. On /files it is the URL instead, so back/forward work and a folder can be linked to; a fresh
// so the Create Dashboard flow can default a new dashboard's cwd to it. // visit still opens at home, because a URL with no `path` means the base. Either way the main
// (unscoped) browser mirrors its folder into files/currentPath so the Create Dashboard flow can
// default a new dashboard's cwd to it.
const currentPath = urlPath ? (searchParams.get(FILES_PATH_PARAM) ?? basePath) : localPath;
const setCurrentPath = (path: string) => { const setCurrentPath = (path: string) => {
setLocalPath(path); if (urlPath) setSearchParams((prev) => withPath(prev, path, basePath));
else setLocalPath(path);
if (!scoped) setGlobalPath(path); if (!scoped) setGlobalPath(path);
}; };
/** Query string for a folder — the current params with `path` set, so an open viewer survives a crumb. */
const searchForPath = (path: string) => withPath(searchParams, path, basePath).toString();
/**
* The ephemeral viewer params replace each other wholesale: opening a file closes whatever overlay was
* open. `path` is not one of them — it is where you are, not what is on top of it — so it survives.
*/
const setViewerParams = (params: Record<string, string>) =>
setSearchParams((prev) => {
const next = new URLSearchParams(params);
const keep = prev.get(FILES_PATH_PARAM);
if (keep) next.set(FILES_PATH_PARAM, keep);
return next;
});
const [entries, setEntries] = useState<DirEntry[]>([]); const [entries, setEntries] = useState<DirEntry[]>([]);
const [rootDir, setRootDir] = useState(''); const [rootDir, setRootDir] = useState('');
const [loading, setLoading] = useState(true); const [loading, setLoading] = useState(true);
@@ -246,7 +280,7 @@ export const useFileBrowserApp = (basePath: string, rootOverride?: string) => {
} else { } else {
const parentPath = entry.path.substring(0, entry.path.lastIndexOf('/')) || '/'; const parentPath = entry.path.substring(0, entry.path.lastIndexOf('/')) || '/';
setCurrentPath(parentPath); setCurrentPath(parentPath);
setSearchParams({ view: entry.path }); setViewerParams({ view: entry.path });
} }
setSearchQuery(''); setSearchQuery('');
}; };
@@ -266,7 +300,7 @@ export const useFileBrowserApp = (basePath: string, rootOverride?: string) => {
setCurrentPath(next); setCurrentPath(next);
} else { } else {
const filePath = currentPath === '/' ? `/${entry.name}` : `${currentPath}/${entry.name}`; const filePath = currentPath === '/' ? `/${entry.name}` : `${currentPath}/${entry.name}`;
setSearchParams({ view: filePath }); setViewerParams({ view: filePath });
} }
}; };
@@ -430,7 +464,7 @@ export const useFileBrowserApp = (basePath: string, rootOverride?: string) => {
const { audioPath } = await files.tts(filePath, { saveNextTo: true }); const { audioPath } = await files.tts(filePath, { saveNextTo: true });
toast.dismiss(toastId); toast.dismiss(toastId);
refresh(); refresh();
setSearchParams({ view: filePath, ephemeral: audioPath }); setViewerParams({ view: filePath, ephemeral: audioPath });
} catch { } catch {
toast.error('Failed to generate speech audio', { id: toastId }); toast.error('Failed to generate speech audio', { id: toastId });
} }
@@ -438,7 +472,7 @@ export const useFileBrowserApp = (basePath: string, rootOverride?: string) => {
const handlePlay = (entry: DirEntry) => { const handlePlay = (entry: DirEntry) => {
const filePath = entryPath(entry.name); const filePath = entryPath(entry.name);
setSearchParams({ play: filePath }); setViewerParams({ play: filePath });
}; };
const handleExtract = async (entry: DirEntry) => { const handleExtract = async (entry: DirEntry) => {
@@ -621,6 +655,8 @@ export const useFileBrowserApp = (basePath: string, rootOverride?: string) => {
homeRoot, homeRoot,
currentPath, currentPath,
setCurrentPath, setCurrentPath,
// Null when this browser does not own the address bar — the breadcrumb then stays buttons.
searchForPath: urlPath ? searchForPath : null,
// Directory listing // Directory listing
visibleEntries, visibleEntries,
loading, loading,