From 990ead93b95c4c82993f5ca645161dbab65cf163 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Fri, 7 Aug 2026 12:34:10 +0000 Subject: [PATCH] put the open file in the url on /code-editor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit audit m5. the active file is `?file=`, tree file rows and tabs are links, and a `?file=` naming something that is not open now opens it — which is the part that makes a pasted link actually work rather than just describe. the open-tab *set* stays local state and i want that on the record as a choice, not an omission. it is a working session, not an address: it grows without bound, every entry costs a read on load, and nobody has ever linked someone else to a tab bar. opt-in via a prop from the screen rather than the workspace identity the file browser uses, because /code-editor renders CodeEditorView directly inside a Widget instead of through the panel wrapper — there is no workspace to ask. a dashboard editor is unchanged. tree *folder* rows stay buttons, and unlike the file browser's folders this needs nobody's call: expanding a directory is disclosure, not navigation. two things fixed while in here. the tab close control was a role="button" span nested inside the tab's own button — invalid before, and a nested interactive inside an anchor after — so it is a sibling button with an aria-label now. and closeFile picked the next-active file inside a setFiles updater, which is the impurity react double-invokes in development to catch. a path that fails to read is remembered, so a broken link errors once instead of once per render, and the address is left alone rather than rewritten. --- docs/navigation-audit.md | 10 +-- .../Screens/Dashboard/CodeEditor/index.tsx | 4 +- .../src/apps/CodeEditor/CodeEditor.tsx | 51 +++++++++++- .../src/apps/CodeEditor/EditorTabs.tsx | 54 +++++++++---- .../src/apps/CodeEditor/FileTree.tsx | 72 +++++++++++------ .../src/apps/CodeEditor/useEditorState.ts | 80 +++++++++++++++---- 6 files changed, 202 insertions(+), 69 deletions(-) diff --git a/docs/navigation-audit.md b/docs/navigation-audit.md index f51050e3..b13876ec 100644 --- a/docs/navigation-audit.md +++ b/docs/navigation-audit.md @@ -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.) **Section routes** (a param that names a view rather than an entity): `/settings/:page/:section`, `/soulseek/:section`, `/headscale/:section`, `/wallet/:section`, `/system-monitor/:scope`. -**Query-param screens:** `/music?path=`, `/files?path=`. -**Genuinely flat, still:** `/` `/plans` `/terminal` `/desktop` `/code-editor` `/qr-transfer`. -`/code-editor` is M5 — the last one that ought not to be. +**Query-param screens:** `/music?path=`, `/files?path=`, `/code-editor?file=`. +**Genuinely flat, still:** `/` `/plans` `/terminal` `/desktop` `/qr-transfer`. Of these only `/plans` selects +something (M9); the other four have nothing addressable to put in an address. 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 was written. @@ -81,7 +81,7 @@ are good building blocks. The **Workspace/Panel framework** contains **zero** ro | ~~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". | | ~~M4~~ | ~~`FileBrowser/.../useFileBrowserApp.ts:269`~~, `FileItem.tsx:516`, ~~`Breadcrumb.tsx:16`~~ | a folder | `/files?path=` | **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 ``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=` (+ `open=` for tabs) | tree file rows → ``; active file in a param. | +| ~~M5~~ | ~~`CodeEditor/FileTree.tsx:59`, `EditorTabs.tsx:33`~~ | open source file / active tab | `/code-editor?file=` | **Done, minus `open=`.** The active file is `?file=`; tree *file* rows and tabs are ``s. **The tab set stays local** — it is a working session, not an address: it grows without bound, each entry costs a read on load, and nobody links someone else to a tab bar. A `?file=` naming a file that is not open now *opens* it, which is what makes a pasted link work; a path that fails to read is remembered so a bad link errors once instead of once per render, and the address is left alone rather than rewritten. **Tree folder rows stay buttons** — unlike the M4 case this needs no owner call, because expanding a directory is disclosure, not navigation. Two things fixed in passing: the tab close control was a `role="button"` span *nested inside* the tab (invalid then, a nested interactive inside an anchor now) and is a sibling ` + )} + + + ); })} diff --git a/src/workspaces/officerdev/src/apps/CodeEditor/FileTree.tsx b/src/workspaces/officerdev/src/apps/CodeEditor/FileTree.tsx index 64cd65da..4e1f95f3 100644 --- a/src/workspaces/officerdev/src/apps/CodeEditor/FileTree.tsx +++ b/src/workspaces/officerdev/src/apps/CodeEditor/FileTree.tsx @@ -1,12 +1,19 @@ import { useState } from 'react'; +import { Link } from 'react-router'; import { ChevronRight, ChevronDown, Folder } from 'lucide-react'; import { getIcon } from 'material-file-icons'; import { useFilesAPI, type DirEntry } from '../../hooks/useFilesAPI'; +/** + * `searchForFile` turns file rows into real links when the editor owns the address bar. Folder rows stay + * buttons on purpose: expanding a directory is disclosure, not navigation — it opens nothing, it changes + * no selection, and there would be nothing for a new tab to show. + */ type FileTreeProps = { root: string; basePath: string; onOpenFile: (path: string, name: string) => void; + searchForFile?: ((path: string) => string) | null; }; type TreeNodeProps = { @@ -14,6 +21,7 @@ type TreeNodeProps = { parentPath: string; root: string; onOpenFile: (path: string, name: string) => void; + searchForFile?: ((path: string) => string) | null; depth: number; }; @@ -28,7 +36,7 @@ const sortEntries = (entries: DirEntry[]) => { return [...dirs, ...files]; }; -const TreeNode = ({ entry, parentPath, root, onOpenFile, depth }: TreeNodeProps) => { +const TreeNode = ({ entry, parentPath, root, onOpenFile, searchForFile, depth }: TreeNodeProps) => { const [expanded, setExpanded] = useState(false); const [children, setChildren] = useState(null); const [loading, setLoading] = useState(false); @@ -54,31 +62,43 @@ const TreeNode = ({ entry, parentPath, root, onOpenFile, depth }: TreeNodeProps) setExpanded((prev) => !prev); }; + const rowClass = + 'flex items-center gap-1 w-full px-1 py-0.5 text-sm rounded cursor-pointer transition-colors text-left text-[#ccc] hover:bg-white/5'; + const rowStyle = { paddingLeft: `${depth * 12 + 4}px` }; + + const body = ( + <> + {isDir ? ( + <> + {expanded ? ( + + ) : ( + + )} + + + ) : ( + <> + + + + )} + {entry.name} + {loading && ...} + + ); + return (
- + {!isDir && searchForFile ? ( + + {body} + + ) : ( + + )} {isDir && expanded && children && (
{children.map((child) => ( @@ -88,6 +108,7 @@ const TreeNode = ({ entry, parentPath, root, onOpenFile, depth }: TreeNodeProps) parentPath={fullPath} root={root} onOpenFile={onOpenFile} + searchForFile={searchForFile} depth={depth + 1} /> ))} @@ -97,7 +118,7 @@ const TreeNode = ({ entry, parentPath, root, onOpenFile, depth }: TreeNodeProps) ); }; -export const FileTree = ({ root, basePath, onOpenFile }: FileTreeProps) => { +export const FileTree = ({ root, basePath, onOpenFile, searchForFile }: FileTreeProps) => { const { listDir } = useFilesAPI(root); const [entries, setEntries] = useState(null); const [loading, setLoading] = useState(false); @@ -134,6 +155,7 @@ export const FileTree = ({ root, basePath, onOpenFile }: FileTreeProps) => { parentPath={basePath} root={root} onOpenFile={onOpenFile} + searchForFile={searchForFile} depth={0} /> ))} diff --git a/src/workspaces/officerdev/src/apps/CodeEditor/useEditorState.ts b/src/workspaces/officerdev/src/apps/CodeEditor/useEditorState.ts index 8b9c658b..91f65263 100644 --- a/src/workspaces/officerdev/src/apps/CodeEditor/useEditorState.ts +++ b/src/workspaces/officerdev/src/apps/CodeEditor/useEditorState.ts @@ -1,4 +1,5 @@ import { useState } from 'react'; +import { useSearchParams } from 'react-router'; export type OpenFile = { path: string; @@ -8,29 +9,65 @@ export type OpenFile = { isDirty: boolean; }; -export const useEditorState = () => { +/** + * Which file the editor is showing, when it is the editor that owns the address bar. Opt-in for the same + * reason `?path=` is in the file browser: this app also mounts as a dashboard panel, and a dashboard can + * hold two of them, which one shared param would drive in lockstep. + */ +export const EDITOR_FILE_PARAM = 'file'; + +/** + * The *set* of open tabs is deliberately not in the URL. It is a working session, not an address — it + * grows without bound, every entry costs a read on load, and nobody links someone else to a tab bar. + * `file` is the selection, and a link to one opens it; the tabs accumulate around it as you browse. + */ +export const useEditorState = (urlState = false) => { const [files, setFiles] = useState([]); - const [activePath, setActivePath] = useState(null); + const [localActive, setLocalActive] = useState(null); + const [searchParams, setSearchParams] = useSearchParams(); + + const activePath = urlState ? searchParams.get(EDITOR_FILE_PARAM) : localActive; + + /** `replace` for a close — shutting a tab is a mutation, not somewhere you navigated to. */ + const setActivePath = (path: string | null, replace = false) => { + if (path === activePath) return; + if (!urlState) { + setLocalActive(path); + return; + } + setSearchParams( + (prev) => { + const next = new URLSearchParams(prev); + if (path) next.set(EDITOR_FILE_PARAM, path); + else next.delete(EDITOR_FILE_PARAM); + return next; + }, + { replace }, + ); + }; + + /** Query string for a file, so a tree row and a tab can be real links. */ + const searchForFile = (path: string) => { + const next = new URLSearchParams(searchParams); + next.set(EDITOR_FILE_PARAM, path); + return next.toString(); + }; const openFile = (path: string, name: string, content: string) => { setFiles((prev) => { - const existing = prev.find((f) => f.path === path); - if (existing) return prev; + if (prev.some((f) => f.path === path)) return prev; return [...prev, { path, name, content, originalContent: content, isDirty: false }]; }); setActivePath(path); }; + // The next-active pick is computed here rather than inside the updater it used to sit in: a state + // updater must be pure, and React invokes it twice in development precisely to catch this. const closeFile = (path: string) => { - setFiles((prev) => { - const next = prev.filter((f) => f.path !== path); - if (activePath === path) { - const idx = prev.findIndex((f) => f.path === path); - const newActive = next[Math.min(idx, next.length - 1)] ?? null; - setActivePath(newActive?.path ?? null); - } - return next; - }); + const idx = files.findIndex((f) => f.path === path); + const next = files.filter((f) => f.path !== path); + setFiles(next); + if (activePath === path) setActivePath(next[Math.min(idx, next.length - 1)]?.path ?? null, true); }; const setContent = (path: string, content: string) => { @@ -45,9 +82,18 @@ export const useEditorState = () => { ); }; - const getActiveFile = (): OpenFile | null => { - return files.find((f) => f.path === activePath) ?? null; - }; + const getActiveFile = (): OpenFile | null => files.find((f) => f.path === activePath) ?? null; - return { files, activePath, setActivePath, openFile, closeFile, setContent, markSaved, getActiveFile }; + return { + files, + activePath, + setActivePath, + openFile, + closeFile, + setContent, + markSaved, + getActiveFile, + // Null when this editor does not own the address bar — rows and tabs then stay buttons. + searchForFile: urlState ? searchForFile : null, + }; };