leaving a folder closes what was open in it
A pane shows a file from the folder you were in. Walking into another folder kept it open, so the listing said one place and the pane said another — and the URL carried a `?view=` that no longer belonged to its own pathname. Navigating now drops the overlay params. Only those: anything else in the query is left alone, because `useGlobalQueryString` puts real state there and a wholesale reset takes it with it. ── The actual fix is that the list has an owner now ── Which params belong to the overlay was a private array inside `useFileViewerPanels`, so every other site that touched the query worked from memory. That is exactly how `onCloseViewer` came to wipe the folder along with the pane, and it is the thing TODO.md flagged as the reason a fifth handler would get it wrong the same way. `VIEWER_PARAMS` and `withoutViewerParams` now live in `files-route.ts` — a leaf with no imports, so the hook and the browser can both depend on it without depending on each other. Three readers, one list. Writing this caught a fourth site immediately: `setViewerParams` had been REPLACING the whole search since the folder left the query string, so opening a file would have discarded global UI state. It clears the overlay and keeps the rest, like everything else now does. 3 more tests, including that every key is covered rather than the obvious two. tsgo clean, frontend builds, 840 pass / 7 fail — +3 new, same 7. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -6,6 +6,8 @@ import {
|
|||||||
folderHref,
|
folderHref,
|
||||||
relativeToFolder,
|
relativeToFolder,
|
||||||
resolveInFolder,
|
resolveInFolder,
|
||||||
|
VIEWER_PARAMS,
|
||||||
|
withoutViewerParams,
|
||||||
} from './files-route';
|
} from './files-route';
|
||||||
|
|
||||||
// The folder is in the URL path now, so every character a filesystem allows has to survive a round trip
|
// The folder is in the URL path now, so every character a filesystem allows has to survive a round trip
|
||||||
@@ -150,3 +152,26 @@ describe('view param, relative to the folder in the path', () => {
|
|||||||
expect(resolveInFolder('/docs', '')).toBe('');
|
expect(resolveInFolder('/docs', '')).toBe('');
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('withoutViewerParams', () => {
|
||||||
|
test('drops the overlay, keeps everything else', () => {
|
||||||
|
// The "everything else" is the point: useGlobalQueryString puts arbitrary state in the URL, and a
|
||||||
|
// wholesale reset to '' would take it with it — which is the bug that sent people home on close.
|
||||||
|
const out = new URLSearchParams(withoutViewerParams('view=a.md&ephemeral=b.mp3&chatType=file&theme=dark&x=1'));
|
||||||
|
expect(out.get('view')).toBeNull();
|
||||||
|
expect(out.get('ephemeral')).toBeNull();
|
||||||
|
expect(out.get('chatType')).toBeNull();
|
||||||
|
expect(out.get('theme')).toBe('dark');
|
||||||
|
expect(out.get('x')).toBe('1');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('every overlay key is covered, not just the obvious two', () => {
|
||||||
|
const all = VIEWER_PARAMS.map((k) => `${k}=x`).join('&');
|
||||||
|
expect(withoutViewerParams(all)).toBe('');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('an empty or overlay-free query is unchanged', () => {
|
||||||
|
expect(withoutViewerParams('')).toBe('');
|
||||||
|
expect(withoutViewerParams('a=1')).toBe('a=1');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -91,3 +91,36 @@ export function relativeToFolder(folder: string, absPath: string): string {
|
|||||||
const rest = absPath.slice(prefix.length);
|
const rest = absPath.slice(prefix.length);
|
||||||
return rest && !rest.includes('/') ? rest : absPath;
|
return rest && !rest.includes('/') ? rest : absPath;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ── Which params belong to the overlay, in one place ──
|
||||||
|
//
|
||||||
|
// The list used to live inside `useFileViewerPanels` and nowhere else, so every other site that touched
|
||||||
|
// the query string worked from memory. That is how `onCloseViewer` came to wipe the whole search — the
|
||||||
|
// folder included — and why walking into a folder had to decide for itself what to carry. One list, three
|
||||||
|
// readers, no memory.
|
||||||
|
//
|
||||||
|
// This module is a leaf with no imports of its own, which is why the list lives here rather than in the
|
||||||
|
// hook: both the hook and the browser can depend on it without depending on each other.
|
||||||
|
|
||||||
|
export const VIEWER_PARAMS = [
|
||||||
|
'view',
|
||||||
|
'ephemeral',
|
||||||
|
'ephemeralRoot',
|
||||||
|
'ephemeral2',
|
||||||
|
'ephemeral2Root',
|
||||||
|
'ephemeral2Auto',
|
||||||
|
'chatContext',
|
||||||
|
'chatType',
|
||||||
|
] as const;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The query string with every overlay param removed, and everything else left alone.
|
||||||
|
*
|
||||||
|
* "Everything else" is not hypothetical — `useGlobalQueryString` puts arbitrary state in the URL, and a
|
||||||
|
* wholesale `''` would take it with it.
|
||||||
|
*/
|
||||||
|
export function withoutViewerParams(search: string): string {
|
||||||
|
const next = new URLSearchParams(search);
|
||||||
|
VIEWER_PARAMS.forEach((key) => next.delete(key));
|
||||||
|
return next.toString();
|
||||||
|
}
|
||||||
|
|||||||
@@ -5,7 +5,7 @@ import { useFilesAPI, type DirEntry } from '../../../hooks/useFilesAPI';
|
|||||||
import { usePinnedFiles } from '../usePinnedFiles';
|
import { usePinnedFiles } from '../usePinnedFiles';
|
||||||
import { EDITOR_FILE_PARAM } from '../../CodeEditor/useEditorState';
|
import { EDITOR_FILE_PARAM } from '../../CodeEditor/useEditorState';
|
||||||
import { getFileType } from '../../FileViewer';
|
import { getFileType } from '../../FileViewer';
|
||||||
import { folderFromPathname, folderHref, relativeToFolder } from './files-route';
|
import { folderFromPathname, folderHref, relativeToFolder, withoutViewerParams } from './files-route';
|
||||||
import { useTasks, type TaskSummary } from '../useTasks';
|
import { useTasks, type TaskSummary } from '../useTasks';
|
||||||
import { useAgents, type AgentSummary } from '../useAgents';
|
import { useAgents, type AgentSummary } from '../useAgents';
|
||||||
import { useUserState } from 'state/useUserState';
|
import { useUserState } from 'state/useUserState';
|
||||||
@@ -37,9 +37,11 @@ export const useFileBrowserApp = (basePath: string, rootOverride?: string, urlPa
|
|||||||
// default a new dashboard's cwd to it.
|
// default a new dashboard's cwd to it.
|
||||||
const currentPath = urlPath ? folderFromPathname(location.pathname) : localPath;
|
const currentPath = urlPath ? folderFromPathname(location.pathname) : localPath;
|
||||||
const setCurrentPath = (path: string) => {
|
const setCurrentPath = (path: string) => {
|
||||||
// The search string rides along, so walking into a folder does not close an open pane — the same
|
// Leaving a folder closes what was open in it. A pane shows a file from the folder you were in, so
|
||||||
// property the old `withPath` had, now expressed as "change the pathname, keep the query".
|
// carrying it into the next one leaves the two disagreeing — the listing says one place, the pane
|
||||||
if (urlPath) navigate({ pathname: folderHref(path), search: location.search });
|
// another. Only the OVERLAY params go; anything else in the query (global UI state) is left alone,
|
||||||
|
// which is the distinction a wholesale reset kept getting wrong.
|
||||||
|
if (urlPath) navigate({ pathname: folderHref(path), search: withoutViewerParams(location.search) });
|
||||||
else setLocalPath(path);
|
else setLocalPath(path);
|
||||||
if (!scoped) setGlobalPath(path);
|
if (!scoped) setGlobalPath(path);
|
||||||
};
|
};
|
||||||
@@ -55,7 +57,15 @@ export const useFileBrowserApp = (basePath: string, rootOverride?: string, urlPa
|
|||||||
* open. The folder is no longer among them — it is the pathname — so it survives without being copied
|
* open. The folder is no longer among them — it is the pathname — so it survives without being copied
|
||||||
* across, which is what used to be needed here.
|
* across, which is what used to be needed here.
|
||||||
*/
|
*/
|
||||||
const setViewerParams = (params: Record<string, string>) => setSearchParams(new URLSearchParams(params));
|
const setViewerParams = (params: Record<string, string>) =>
|
||||||
|
setSearchParams((prev) => {
|
||||||
|
// Clear the previous overlay — opening a file closes whatever was open — but only the overlay.
|
||||||
|
// Replacing the whole search would take global UI state with it, which is the mistake this file
|
||||||
|
// has now made twice in two different places.
|
||||||
|
const next = new URLSearchParams(withoutViewerParams(prev.toString()));
|
||||||
|
for (const [key, value] of Object.entries(params)) next.set(key, value);
|
||||||
|
return next;
|
||||||
|
});
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Links from before the folder moved into the path still say `?path=/Tests`. Rewrite them once, in
|
* Links from before the folder moved into the path still say `?path=/Tests`. Rewrite them once, in
|
||||||
|
|||||||
@@ -9,17 +9,7 @@ import {
|
|||||||
singleChatLayout,
|
singleChatLayout,
|
||||||
} from './layouts';
|
} from './layouts';
|
||||||
import { ViewerProvider, EphemeralProvider, Ephemeral2Provider, ChatEphemeralBody } from './Providers';
|
import { ViewerProvider, EphemeralProvider, Ephemeral2Provider, ChatEphemeralBody } from './Providers';
|
||||||
|
import { withoutViewerParams } from '../../apps/FileBrowser/FileBrowserApp/files-route';
|
||||||
const EPHEMERAL_KEYS = [
|
|
||||||
'view',
|
|
||||||
'ephemeral',
|
|
||||||
'ephemeralRoot',
|
|
||||||
'ephemeral2',
|
|
||||||
'ephemeral2Root',
|
|
||||||
'ephemeral2Auto',
|
|
||||||
'chatContext',
|
|
||||||
'chatType',
|
|
||||||
];
|
|
||||||
|
|
||||||
export const useFileViewerPanels = (): EphemeralPanels | null => {
|
export const useFileViewerPanels = (): EphemeralPanels | null => {
|
||||||
const [searchParams, setSearchParams] = useSearchParams();
|
const [searchParams, setSearchParams] = useSearchParams();
|
||||||
@@ -82,12 +72,7 @@ export const useFileViewerPanels = (): EphemeralPanels | null => {
|
|||||||
* `onCloseEphemeral` directly below has always done.
|
* `onCloseEphemeral` directly below has always done.
|
||||||
*/
|
*/
|
||||||
const onCloseViewer = useCallback(
|
const onCloseViewer = useCallback(
|
||||||
() =>
|
() => setSearchParams((prev) => new URLSearchParams(withoutViewerParams(prev.toString()))),
|
||||||
setSearchParams((prev) => {
|
|
||||||
const next = new URLSearchParams(prev);
|
|
||||||
EPHEMERAL_KEYS.forEach((k) => next.delete(k));
|
|
||||||
return next;
|
|
||||||
}),
|
|
||||||
[setSearchParams],
|
[setSearchParams],
|
||||||
);
|
);
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user