diff --git a/apps/desktop/src/renderer/lib/clickPolicy/usePierreRowClickPolicy.ts b/apps/desktop/src/renderer/lib/clickPolicy/usePierreRowClickPolicy.ts index ecac2382b8b..8544080b6b8 100644 --- a/apps/desktop/src/renderer/lib/clickPolicy/usePierreRowClickPolicy.ts +++ b/apps/desktop/src/renderer/lib/clickPolicy/usePierreRowClickPolicy.ts @@ -34,9 +34,12 @@ interface UsePierreRowClickPolicyResult { * - folder rows → `folderIntentFor` (meta=reveal/no-op, metaShift=external) * - file rows → settings-driven via the injected `filePolicy` * - * Unbound tiers and plain "pane" defer to Pierre's own `onSelectionChange` - * so the visual selection stays in sync; intercepting would swallow the - * click and leave Pierre out of date. + * Every resolved action is intercepted (preventDefault + stopPropagation) — + * we never defer to Pierre's own click → `onSelectionChange` pipeline. + * Pierre's `selectOnlyPath` no-ops when the clicked row is already selected, + * which would otherwise silently drop legitimate re-clicks (click-to-pin, + * reopen after Cmd+W). Pierre's selection is reconciled separately via the + * reveal flow keyed off the active file pane. */ export function usePierreRowClickPolicy({ filePolicy, @@ -78,9 +81,12 @@ export function usePierreRowClickPolicy({ return; } - const { tier, action } = filePolicy.resolve(e); + const { action } = filePolicy.resolve(e); if (action === null) return; - if (tier === "plain" && action === "pane") return; + // Always intercept — never defer to Pierre's own selection-change + // pipeline. Pierre's selectOnlyPath no-ops when the clicked row is + // already selected, which silently drops legitimate re-clicks + // (e.g. click-to-pin, or reopening a file after Cmd+W). e.preventDefault(); e.stopPropagation(); if (action === "external") openInExternalEditor(trimmed); diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx index f7c83a51103..b2d6cefc4c8 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx @@ -172,6 +172,13 @@ export function FilesTab({ handlersRef.current.onRename = (event) => void handleRename(event); handlersRef.current.onSelect = (treePath) => { const abs = toAbs(rootPath, treePath); + // Skip the reveal-induced echo. The reveal flow programmatically + // selects the just-opened file's row, which fires onSelectionChange + // synchronously. Without this guard, the echo re-enters onSelectFile + // → openFilePaneFromTreeClick, which sees active === target and + // pins the pane we just opened. Real keyboard nav (selection moves + // to a different file) still gets through. + if (selectedFilePath === abs) return; lastSelectedFromUserRef.current = abs; onSelectFile(abs); }; diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/hooks/useFilesTabActions/useFilesTabActions.ts b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/hooks/useFilesTabActions/useFilesTabActions.ts index 7c465de9220..88204652cf5 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/hooks/useFilesTabActions/useFilesTabActions.ts +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/hooks/useFilesTabActions/useFilesTabActions.ts @@ -3,10 +3,12 @@ import { alert } from "@superset/ui/atoms/Alert"; import { toast } from "@superset/ui/sonner"; import { workspaceTrpc } from "@superset/workspace-client"; import { useCallback } from "react"; +import { FILE_EXPLORER_ROW_HEIGHT } from "../../constants"; import { deriveCreationParent, pickPlaceholderName, } from "../../utils/creationPaths"; +import { scrollTreeToRow } from "../../utils/scrollTreeToRow"; import { asDirectoryHandle, basename, @@ -98,7 +100,27 @@ export function useFilesTabActions({ } requestAnimationFrame(() => { + // Visual row highlight comes from `data-item-selected`, not focus. + // FileTree's public API doesn't expose selectOnlyPath, so emulate + // it via deselect-then-select on the item handles. Pierre uses + // trailing-slash keys for directories. Empty-selection emissions + // between deselect and select are filtered out by FilesTab's + // onSelectionChange handler (it ignores `last === undefined`, + // and folder-shaped paths get skipped before onSelectFile). + const targetKey = isDirectory ? `${rel}/` : rel; + for (const selectedPath of model.getSelectedPaths()) { + if (selectedPath === targetKey) continue; + model.getItem(selectedPath)?.deselect(); + } + model.getItem(targetKey)?.select(); model.focusPath(rel); + + scrollTreeToRow( + model, + bridge.knownPaths, + targetKey, + FILE_EXPLORER_ROW_HEIGHT, + ); }); }, [model, rootPath, bridge.fetchDir, bridge.knownPaths], diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/hooks/useFilesTabBridge/useFilesTabBridge.ts b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/hooks/useFilesTabBridge/useFilesTabBridge.ts index 86d733abb10..bc560b7dd47 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/hooks/useFilesTabBridge/useFilesTabBridge.ts +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/hooks/useFilesTabBridge/useFilesTabBridge.ts @@ -80,7 +80,12 @@ export function useFilesTabBridge({ // across renders. const knownPathsRef = useRef(new Set()); const loadedDirsRef = useRef(new Set()); - const loadingDirsRef = useRef(new Set()); + // Track in-flight loads as promises (not a Set) so concurrent callers + // await the same fetch instead of short-circuiting. Pierre's `expand()` + // notifies subscribers synchronously, and our model.subscribe hook fires + // fetchDir before reveal's own `await fetchDir` runs — without shared + // promises, reveal would resolve before children land in knownPaths. + const inflightDirsRef = useRef(new Map>()); const pendingCreatesRef = useRef(new Map()); // Bumped on workspace/root change so async listings started against an @@ -90,32 +95,47 @@ export function useFilesTabBridge({ const fetchDir = useCallback( async (relDir: string): Promise => { if (!rootPath || !workspaceId) return; - if (loadingDirsRef.current.has(relDir)) return; if (loadedDirsRef.current.has(relDir)) return; - loadingDirsRef.current.add(relDir); + const existing = inflightDirsRef.current.get(relDir); + if (existing) return existing; + const startVersion = versionRef.current; - try { - const result = await utils.filesystem.listDirectory.fetch({ - workspaceId, - absolutePath: toAbs(rootPath, relDir), - }); - if (versionRef.current !== startVersion) return; - const ops: { type: "add"; path: string }[] = []; - for (const entry of result.entries) { - const rel = toRel(rootPath, entry.absolutePath); - const treePath = entry.kind === "directory" ? `${rel}/` : rel; - if (knownPathsRef.current.has(treePath)) continue; - knownPathsRef.current.add(treePath); - ops.push({ type: "add", path: treePath }); + const promise = (async () => { + try { + const result = await utils.filesystem.listDirectory.fetch({ + workspaceId, + absolutePath: toAbs(rootPath, relDir), + }); + if (versionRef.current !== startVersion) return; + const ops: { type: "add"; path: string }[] = []; + for (const entry of result.entries) { + const rel = toRel(rootPath, entry.absolutePath); + const treePath = entry.kind === "directory" ? `${rel}/` : rel; + if (knownPathsRef.current.has(treePath)) continue; + knownPathsRef.current.add(treePath); + ops.push({ type: "add", path: treePath }); + } + if (ops.length > 0) model.batch(ops); + loadedDirsRef.current.add(relDir); + } catch (error) { + if (versionRef.current !== startVersion) return; + console.error("[v2 FilesTab] listDirectory failed", { + relDir, + error, + }); } - if (ops.length > 0) model.batch(ops); - loadedDirsRef.current.add(relDir); - } catch (error) { - if (versionRef.current !== startVersion) return; - console.error("[v2 FilesTab] listDirectory failed", { relDir, error }); - } finally { - loadingDirsRef.current.delete(relDir); - } + })(); + inflightDirsRef.current.set(relDir, promise); + // Identity-check before deleting: on a workspace switch the map is + // cleared and a new promise can be registered under the same key. + // Without this guard, a late-resolving stale promise would evict + // the live one and reopen duplicate fetches. + void promise.finally(() => { + if (inflightDirsRef.current.get(relDir) === promise) { + inflightDirsRef.current.delete(relDir); + } + }); + return promise; }, [model, rootPath, workspaceId, utils.filesystem.listDirectory], ); @@ -168,7 +188,7 @@ export function useFilesTabBridge({ versionRef.current += 1; knownPathsRef.current.clear(); loadedDirsRef.current.clear(); - loadingDirsRef.current.clear(); + inflightDirsRef.current.clear(); pendingCreatesRef.current.clear(); model.resetPaths([]); void fetchDir(""); diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/utils/scrollTreeToRow/index.ts b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/utils/scrollTreeToRow/index.ts new file mode 100644 index 00000000000..6724f7e3fb1 --- /dev/null +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/utils/scrollTreeToRow/index.ts @@ -0,0 +1 @@ +export { scrollTreeToRow } from "./scrollTreeToRow"; diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/utils/scrollTreeToRow/scrollTreeToRow.ts b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/utils/scrollTreeToRow/scrollTreeToRow.ts new file mode 100644 index 00000000000..a16f4f47171 --- /dev/null +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/utils/scrollTreeToRow/scrollTreeToRow.ts @@ -0,0 +1,78 @@ +import { type FileTree, prepareFileTreeInput } from "@pierre/trees"; +import { asDirectoryHandle } from "../treePath"; + +/** + * Center `targetKey` in the file-tree viewport. + * + * Pierre auto-scrolls focused rows only when DOM focus lives inside the tree + * (FileTreeView shouldOwnDomFocus gate), so programmatic reveals don't scroll. + * The public FileTree API doesn't expose the focused row index or a reveal/ + * scrollTo method, so we replicate Pierre's sort + visibility math: + * + * 1. Sort knownPaths via Pierre's own `prepareFileTreeInput` (directories + * before files at each depth, case-insensitive natural sort within). + * 2. Walk the sorted list, skipping paths whose ancestors are collapsed, + * to find the target's visible index. + * 3. scrollTop = index * itemHeight, centered in the viewport. + * + * Returns true if it scrolled (or the row was already in view), false if it + * couldn't locate the scroll element or target. + */ +export function scrollTreeToRow( + model: FileTree, + knownPaths: ReadonlySet, + targetKey: string, + itemHeight: number, +): boolean { + const scrollEl = model + .getFileTreeContainer() + ?.shadowRoot?.querySelector('[data-file-tree-virtualized-scroll="true"]'); + if (!(scrollEl instanceof HTMLElement)) return false; + + const visibleIndex = computeVisibleRowIndex(targetKey, knownPaths, model); + if (visibleIndex < 0) return false; + + const viewportHeight = scrollEl.clientHeight; + const targetTop = visibleIndex * itemHeight; + const targetBottom = targetTop + itemHeight; + const currentTop = scrollEl.scrollTop; + const currentBottom = currentTop + viewportHeight; + + if (targetTop >= currentTop && targetBottom <= currentBottom) return true; + scrollEl.scrollTop = Math.max( + 0, + targetTop - (viewportHeight - itemHeight) / 2, + ); + return true; +} + +function computeVisibleRowIndex( + targetKey: string, + knownPaths: ReadonlySet, + model: FileTree, +): number { + const prepared = prepareFileTreeInput(Array.from(knownPaths)); + let index = 0; + for (const path of prepared.paths) { + if (path === targetKey) { + return isPathVisible(path, model) ? index : -1; + } + if (isPathVisible(path, model)) index++; + } + return -1; +} + +function isPathVisible(path: string, model: FileTree): boolean { + const trimmed = path.endsWith("/") ? path.slice(0, -1) : path; + let lastSlash = trimmed.lastIndexOf("/"); + if (lastSlash < 0) return true; + let parent = trimmed.slice(0, lastSlash); + while (parent) { + const handle = asDirectoryHandle(model.getItem(`${parent}/`)); + if (!handle?.isExpanded()) return false; + lastSlash = parent.lastIndexOf("/"); + if (lastSlash < 0) break; + parent = parent.slice(0, lastSlash); + } + return true; +} diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useWorkspaceFileNavigation/useWorkspaceFileNavigation.ts b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useWorkspaceFileNavigation/useWorkspaceFileNavigation.ts index bcc0e8ed0c7..ce75ee071f5 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useWorkspaceFileNavigation/useWorkspaceFileNavigation.ts +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useWorkspaceFileNavigation/useWorkspaceFileNavigation.ts @@ -30,6 +30,7 @@ export function useWorkspaceFileNavigation({ setRightSidebarTab: V2UserPreferencesApi["setRightSidebarTab"]; }): { openFilePane: (filePath: string, openInNewTab?: boolean) => void; + openFilePaneFromTreeClick: (filePath: string, openInNewTab?: boolean) => void; revealPath: ( path: string, options?: { @@ -119,13 +120,23 @@ export function useWorkspaceFileNavigation({ }); return; } - const active = state.getActivePane(); - if ( - active?.pane.kind === "file" && - (active.pane.data as FilePaneData).filePath === absoluteFilePath - ) { - state.setPanePinned({ paneId: active.pane.id, pinned: true }); - return; + // Focus an existing pane for this file (anywhere in any tab) before + // opening anything new. The previous pin-on-same-file branch turned + // re-picks into pin operations — which broke the preview/overwrite + // flow: once pinned, the next pick couldn't find an unpinned pane + // to replace and got split into a new pane. Pinning is now + // explicit only (header click, dirty edit). + for (const tab of state.tabs) { + for (const pane of Object.values(tab.panes)) { + if ( + pane.kind === "file" && + (pane.data as FilePaneData).filePath === absoluteFilePath + ) { + state.setActiveTab(tab.id); + state.setActivePane({ tabId: tab.id, paneId: pane.id }); + return; + } + } } state.openPane({ pane: { @@ -140,6 +151,32 @@ export function useWorkspaceFileNavigation({ [store, worktreePath, recordView], ); + // Sidebar tree clicks layer the VS-Code-style "click an already-active row + // to pin it" pattern on top of openFilePane. The picker and other callers + // stay on plain openFilePane so re-picks just refocus without pinning. + const openFilePaneFromTreeClick = useCallback( + (filePath: string, openInNewTab?: boolean) => { + if (openInNewTab) { + openFilePane(filePath, true); + return; + } + const absoluteFilePath = worktreePath + ? toAbsoluteWorkspacePath(worktreePath, filePath) + : filePath; + const state = store.getState(); + const active = state.getActivePane(); + if ( + active?.pane.kind === "file" && + (active.pane.data as FilePaneData).filePath === absoluteFilePath + ) { + state.setPanePinned({ paneId: active.pane.id, pinned: true }); + return; + } + openFilePane(filePath); + }, + [openFilePane, store, worktreePath], + ); + const revealPath = useCallback( (path: string, options?: { isDirectory?: boolean }) => { setRightSidebarOpen(true); @@ -152,6 +189,7 @@ export function useWorkspaceFileNavigation({ return { openFilePane, + openFilePaneFromTreeClick, revealPath, selectedFilePath, pendingReveal, diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsx b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsx index 3bcdc0df6b1..b0f45d802fb 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsx +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsx @@ -170,6 +170,7 @@ function V2WorkspaceContent({ const { openFilePane, + openFilePaneFromTreeClick, revealPath, selectedFilePath, pendingReveal, @@ -235,6 +236,17 @@ function V2WorkspaceContent({ }, [closeQuickOpen], ); + // Picking a file from Quick Open should surface the sidebar/Files tab so + // the reveal (expand + highlight + scroll) is actually visible. Tree + // clicks and other openFilePane callers already have the sidebar open. + const handleQuickOpenSelectFile = useCallback( + (filePath: string, openInNewTab?: boolean) => { + setRightSidebarOpen(true); + setRightSidebarTab("files"); + openFilePane(filePath, openInNewTab); + }, + [openFilePane, setRightSidebarOpen, setRightSidebarTab], + ); const defaultPaneActions = useDefaultPaneActions({ launcher }); const onBeforeCloseTab = useDirtyTabCloseGuard(); @@ -369,7 +381,7 @@ function V2WorkspaceContent({ >