fix(desktop): v2 file reveal — expand, highlight, scroll, open sidebar

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);

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);
 	};

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],

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<string>());
 	const loadedDirsRef = useRef(new Set<string>());
-	const loadingDirsRef = useRef(new Set<string>());
+	// 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<string, Promise<void>>());
 	const pendingCreatesRef = useRef(new Map<string, "file" | "folder">());
 
 	// 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<void> => {
 			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("");

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";

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<string>,
+	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<string>,
+	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;
+}

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,

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({
 					>
 						<WorkspaceSidebar
 							workspaceId={workspaceId}
-							onSelectFile={openFilePane}
+							onSelectFile={openFilePaneFromTreeClick}
 							onSelectDiffFile={openDiffPane}
 							onOpenComment={openCommentPane}
 							onSearch={handleQuickOpen}
@@ -383,7 +395,7 @@ function V2WorkspaceContent({
 				workspaceId={workspaceId}
 				open={quickOpenOpen}
 				onOpenChange={handleQuickOpenChange}
-				onSelectFile={openFilePane}
+				onSelectFile={handleQuickOpenSelectFile}
 				variant="v2"
 				recentlyViewedFiles={recentFiles}
 				openFilePaths={openFilePaths}