From 15392ae6c2b5274e640deb07b626cec373ee77ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Thu, 27 Aug 2026 14:13:40 +0000 Subject: [PATCH] preview a commander's files, the way /files does MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit the commander file panel could list a directory and not open anything in it. it already mounts the same `FileBrowserApp` as /files — what it did not mount is the VIEWER, which on /files is a separate ephemeral panel and not part of the browser at all. THE PARAM COULD NOT BE READ, which is why this is more than mounting a panel. on /files the folder is in the PATHNAME, so `?view=notes.md` is a name inside it and that is all it needs to be. a browser mounted anywhere else keeps its folder in panel-local state, so the same name is unresolvable by anything except the component that wrote it — the commander panel was writing `?view=notes.md` with no reader able to tell which of its folders that meant. so off the files route the param carries the whole path, and `viewRoot` with it: a panel that overrides the root is exactly the case where "home" is the wrong guess. `viewRoot` joins VIEWER_PARAMS so closing an overlay clears it — a stale root outliving its view would send the next file to the wrong tree. the split is VERTICAL where /files is horizontal. this panel is already the narrow half of a screen whose other half is a transcript; splitting it sideways again leaves two columns too thin to read anything in. closing the file clears the overlay params by NAME rather than resetting the query, because `files=1` — the thing keeping this whole panel open — lives in the same string, and the obvious version closes the browser along with the file. Co-Authored-By: Claude Opus 5 Officer-Session: 656f5c85-e49a-4f1f-b9f3-042a04b4d043 --- .../FileBrowser/FileBrowserApp/files-route.ts | 4 ++ .../FileBrowserApp/useFileBrowserApp.ts | 17 ++++- .../useCommanderFilesPanel.tsx | 68 ++++++++++++++++--- 3 files changed, 78 insertions(+), 11 deletions(-) diff --git a/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/files-route.ts b/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/files-route.ts index ff5e9872..f10af16b 100644 --- a/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/files-route.ts +++ b/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/files-route.ts @@ -104,6 +104,10 @@ export function relativeToFolder(folder: string, absPath: string): string { export const VIEWER_PARAMS = [ 'view', + // Written only by a browser whose folder is NOT in the pathname, where `view` has to carry the whole + // path and the root it is under. Listed here so closing an overlay clears it with everything else — + // a stale `viewRoot` outliving its `view` would point the next file at the wrong root. + 'viewRoot', 'ephemeral', 'ephemeralRoot', 'ephemeral2', diff --git a/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/useFileBrowserApp.ts b/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/useFileBrowserApp.ts index b2b1de5e..2468fdfd 100644 --- a/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/useFileBrowserApp.ts +++ b/src/workspaces/officerdev/src/apps/FileBrowser/FileBrowserApp/useFileBrowserApp.ts @@ -424,9 +424,20 @@ export const useFileBrowserApp = (basePath: string, rootOverride?: string, urlPa void handleExtract(entry); return; } - // Just the name: the folder is already in the pathname, so `?view=` naming it again was the same - // fact twice, in its least readable form. - setViewerParams({ view: entry.name }); + /* + Just the name when the folder is in the PATHNAME — `/files/Tests` plus `?view=notes.md` says it + once. A browser mounted anywhere else keeps its folder in panel-local state instead, so a name + alone is unresolvable by anything but this component: the commander's file panel wrote + `?view=notes.md` and no reader could tell which of its folders that meant. + + So when the folder is not in the URL, the param carries the whole path — and the root with it, + since a panel that overrides the root is exactly the case where 'home' would be the wrong guess. + */ + if (urlPath) { + setViewerParams({ view: entry.name }); + return; + } + setViewerParams({ view: entryPath(entry.name), ...(rootOverride ? { viewRoot: rootOverride } : {}) }); }; const handleCreateDir = async (name: string) => { diff --git a/src/workspaces/officerdev/src/hooks/useCommanderFilesPanel/useCommanderFilesPanel.tsx b/src/workspaces/officerdev/src/hooks/useCommanderFilesPanel/useCommanderFilesPanel.tsx index f09b96da..b499f3a3 100644 --- a/src/workspaces/officerdev/src/hooks/useCommanderFilesPanel/useCommanderFilesPanel.tsx +++ b/src/workspaces/officerdev/src/hooks/useCommanderFilesPanel/useCommanderFilesPanel.tsx @@ -1,9 +1,11 @@ -import { useCallback, useMemo } from 'react'; +import { useCallback, useMemo, type ReactNode } from 'react'; import { useParams, useSearchParams } from 'react-router'; import { useQuery } from '@tanstack/react-query'; import { useClient } from 'hooks/useClient'; import type { EphemeralPanels, LayoutNode } from '../../components/Workspace'; import { FileBrowserApp } from '../../apps/FileBrowser'; +import { FileViewerProvider, FileViewerHeader, FileViewerBody } from '../../apps/FileViewer'; +import { withoutViewerParams } from '../../apps/FileBrowser/FileBrowserApp/files-route'; import type { CommandHqCommanderDetail } from '../../apps/CommandHQ/types'; import { COMMANDER_FILES_PARAM } from '../../apps/CommandHQ/command-hq-routes'; @@ -32,7 +34,24 @@ import { COMMANDER_FILES_PARAM } from '../../apps/CommandHQ/command-hq-routes'; * root really is all of `user-data` — which is the owner's own data either way, so it bounds the * navigation rather than the account. */ -const layout: LayoutNode = { type: 'panel', id: 'command-hq-files', appType: null }; +const browserOnly: LayoutNode = { type: 'panel', id: 'command-hq-files', appType: null }; + +/** + * Browser above, file below — the same shape `/files` gives you, in the space a side panel has. + * + * Vertical rather than the horizontal split `/files` uses, because this whole panel is already the + * narrow half of a screen whose other half is a transcript. Splitting it again sideways leaves two + * columns too thin to read a file in. + */ +const browserWithViewer: LayoutNode = { + type: 'group', + id: 'command-hq-files-group', + direction: 'vertical', + children: [ + { node: { type: 'panel', id: 'command-hq-files', appType: null }, size: 40 }, + { node: { type: 'panel', id: 'command-hq-viewer', appType: null }, size: 60 }, + ], +}; export const useCommanderFilesPanel = (): EphemeralPanels | null => { const [searchParams, setSearchParams] = useSearchParams(); @@ -60,22 +79,55 @@ export const useCommanderFilesPanel = (): EphemeralPanels | null => { const dirName = data?.dirName ?? null; - const components = useMemo( - () => ({ + /* + `?view=` here is a WHOLE path, not a name. + + On `/files` the folder is in the pathname, so the param names a file inside it. This browser keeps + its folder in panel-local state, so `useFileBrowserApp` writes the full path instead — see the note + beside `setViewerParams`. Read it as given and do not resolve it against anything. + */ + const viewPath = searchParams.get('view'); + const viewRoot = searchParams.get('viewRoot') ?? 'user-data'; + + const onCloseViewer = useCallback( + // By name, not `setSearchParams({})`: this panel's own `files=1` lives in the same query string, and + // wiping it would close the file browser along with the file. + () => setSearchParams((prev) => new URLSearchParams(withoutViewerParams(prev.toString()))), + [setSearchParams], + ); + + const components = useMemo(() => { + const panels: Record = { 'command-hq-files': { component: () => ( ), onClose, }, - }), - [dirName, onClose], - ); + }; + + if (viewPath) { + const fileName = viewPath.split('/').filter(Boolean).pop() ?? viewPath; + panels['command-hq-viewer'] = { + // The provider is what the header and body both read from, which is why it wraps rather than + // being passed down — same wiring as `useFileViewerPanels`. + provider: ({ children }: { children: ReactNode }) => ( + + {children} + + ), + header: FileViewerHeader, + component: FileViewerBody, + onClose: onCloseViewer, + }; + } + return panels as EphemeralPanels['components']; + }, [dirName, onClose, viewPath, viewRoot, onCloseViewer]); // Nothing until the directory name is known. Mounting on `/commanders/null` would list the whole // commanders folder for a beat — every OTHER commander included — before correcting itself. if (!open || !dirName) return null; - return { layout, components, defaultBaseSize: 45, onClose }; + return { layout: viewPath ? browserWithViewer : browserOnly, components, defaultBaseSize: 45, onClose }; }; export type UseCommanderFilesPanelType = ReturnType;