fix(desktop): file tree clicks + sidebar changes tab#1122
Merged
Conversation
Contributor
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughReplaces react-arborist with a headless-tree implementation for the Files view (useTree + async dataLoader), removes FileTreeNode/FileTreeContextMenu, adds FileTreeItem/FileSearchResultItem, updates create/rename/delete flows to use DirectoryEntry, adds Changes tab visibility driven by branch/status queries, and centralizes file-viewer mode resolution. Changes
Sequence DiagramsequenceDiagram
participant User
participant FilesView
participant HeadlessTree
participant FileTreeItem
participant FileViewerPane
participant ExternalEditor
User->>FileTreeItem: click / double-click / open context menu
FileTreeItem->>FilesView: onActivate / onOpenInEditor / context action (entry)
FilesView->>HeadlessTree: tree.getItem / tree.getChildren (async)
HeadlessTree-->>FilesView: entry / children (async)
alt Open externally
FilesView->>ExternalEditor: openFileInEditor(entry.path)
ExternalEditor-->>User: external editor opens file
else Open in app
FilesView->>FileViewerPane: addFileViewerPane({ path: entry.path, viewMode })
FileViewerPane-->>User: file viewer opens
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/FilesView.tsx (1)
143-185:⚠️ Potential issue | 🟡 MinorCancel pending opens on workspace change to avoid stale opens
The debounce captures
workspaceId/worktreePath; a quick workspace switch can open a file in the wrong context. Also extract the delay constant.🛠️ Proposed fix
+const FILE_OPEN_DEBOUNCE_MS = 300; @@ - pendingOpenRef.current = setTimeout(() => { + pendingOpenRef.current = setTimeout(() => { pendingOpenRef.current = null; addFileViewerPane(workspaceId, { filePath: node.data.relativePath, viewMode: "raw", }); - }, 300); + }, FILE_OPEN_DEBOUNCE_MS); }, [workspaceId, worktreePath, addFileViewerPane], ); @@ - useEffect(() => { - return () => { - if (pendingOpenRef.current) { - clearTimeout(pendingOpenRef.current); - pendingOpenRef.current = null; - } - }; - }, []); + useEffect(() => { + return () => { + cancelPendingOpen(); + }; + }, [cancelPendingOpen]); + + useEffect(() => { + cancelPendingOpen(); + }, [cancelPendingOpen, workspaceId, worktreePath]);As per coding guidelines: Avoid magic numbers by extracting them to named constants at module top.
🤖 Fix all issues with AI agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/components/FileTreeNode/FileTreeNode.tsx`:
- Around line 3-61: In FileTreeNode, extract the 250ms magic number into a named
constant (e.g., CLICK_DELAY_MS) at module top and ensure the pending timeout
stored in clickTimeoutRef is cleared on unmount: add a useEffect with a cleanup
that checks clickTimeoutRef.current, calls clearTimeout(...) and sets it to
null; keep existing handleClick logic and references to node.activate,
node.toggle, openInEditor and onCancelOpen unchanged.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/index.tsx (1)
63-69: Extract the status refetch interval to a named constantThis makes the polling cadence self-documenting and easier to tune.
♻️ Suggested refactor
+const STATUS_REFETCH_INTERVAL_MS = 2500; @@ - refetchInterval: 2500, + refetchInterval: STATUS_REFETCH_INTERVAL_MS,As per coding guidelines: Avoid magic numbers by extracting them to named constants at module top.
Comment on lines
+3
to
+61
| import { useRef } from "react"; | ||
| import { LuChevronDown, LuChevronRight } from "react-icons/lu"; | ||
| import type { FileTreeNode as FileTreeNodeType } from "shared/file-tree-types"; | ||
| import { usePathActions } from "../../../ChangesView/hooks"; | ||
| import { getFileIcon } from "../../utils"; | ||
|
|
||
| type FileTreeNodeProps = NodeRendererProps<FileTreeNodeType>; | ||
| type FileTreeNodeProps = NodeRendererProps<FileTreeNodeType> & { | ||
| worktreePath: string; | ||
| onCancelOpen: () => void; | ||
| }; | ||
|
|
||
| export function FileTreeNode({ node, style, dragHandle }: FileTreeNodeProps) { | ||
| export function FileTreeNode({ | ||
| node, | ||
| style, | ||
| dragHandle, | ||
| worktreePath, | ||
| onCancelOpen, | ||
| }: FileTreeNodeProps) { | ||
| const { data } = node; | ||
| const { icon: Icon, color } = getFileIcon( | ||
| data.name, | ||
| data.isDirectory, | ||
| node.isOpen, | ||
| ); | ||
| const { openInEditor } = usePathActions({ | ||
| absolutePath: data.path ?? null, | ||
| relativePath: data.relativePath, | ||
| cwd: worktreePath, | ||
| }); | ||
|
|
||
| const clickTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null); | ||
|
|
||
| const handleClick = (e: React.MouseEvent) => { | ||
| e.stopPropagation(); | ||
| node.select(); | ||
| if (data.isDirectory) { | ||
| node.toggle(); | ||
| if (e.detail === 1) { | ||
| node.toggle(); | ||
| } | ||
| return; | ||
| } | ||
| }; | ||
|
|
||
| const handleDoubleClick = (e: React.MouseEvent) => { | ||
| e.stopPropagation(); | ||
| node.activate(); | ||
| // Double-click: open in external editor | ||
| if (e.detail === 2) { | ||
| if (clickTimeoutRef.current) { | ||
| clearTimeout(clickTimeoutRef.current); | ||
| clickTimeoutRef.current = null; | ||
| } | ||
| onCancelOpen(); | ||
| openInEditor(); | ||
| return; | ||
| } | ||
|
|
||
| // Single-click: delay activate to allow double-click detection | ||
| if (clickTimeoutRef.current) { | ||
| clearTimeout(clickTimeoutRef.current); | ||
| } | ||
| clickTimeoutRef.current = setTimeout(() => { | ||
| clickTimeoutRef.current = null; | ||
| node.activate(); | ||
| }, 250); |
Contributor
There was a problem hiding this comment.
Clear the pending click timeout on unmount to avoid late activations
A queued 250ms timer can still fire after unmount; also extract the delay to a named constant.
🛠️ Proposed fix
-import { useRef } from "react";
+import { useEffect, useRef } from "react";
@@
+const SINGLE_CLICK_DELAY_MS = 250;
+
export function FileTreeNode({
@@
const clickTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null);
+
+ useEffect(() => {
+ return () => {
+ if (clickTimeoutRef.current) {
+ clearTimeout(clickTimeoutRef.current);
+ clickTimeoutRef.current = null;
+ }
+ };
+ }, []);
@@
- }, 250);
+ }, SINGLE_CLICK_DELAY_MS);
};As per coding guidelines: Avoid magic numbers by extracting them to named constants at module top.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import { useRef } from "react"; | |
| import { LuChevronDown, LuChevronRight } from "react-icons/lu"; | |
| import type { FileTreeNode as FileTreeNodeType } from "shared/file-tree-types"; | |
| import { usePathActions } from "../../../ChangesView/hooks"; | |
| import { getFileIcon } from "../../utils"; | |
| type FileTreeNodeProps = NodeRendererProps<FileTreeNodeType>; | |
| type FileTreeNodeProps = NodeRendererProps<FileTreeNodeType> & { | |
| worktreePath: string; | |
| onCancelOpen: () => void; | |
| }; | |
| export function FileTreeNode({ node, style, dragHandle }: FileTreeNodeProps) { | |
| export function FileTreeNode({ | |
| node, | |
| style, | |
| dragHandle, | |
| worktreePath, | |
| onCancelOpen, | |
| }: FileTreeNodeProps) { | |
| const { data } = node; | |
| const { icon: Icon, color } = getFileIcon( | |
| data.name, | |
| data.isDirectory, | |
| node.isOpen, | |
| ); | |
| const { openInEditor } = usePathActions({ | |
| absolutePath: data.path ?? null, | |
| relativePath: data.relativePath, | |
| cwd: worktreePath, | |
| }); | |
| const clickTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null); | |
| const handleClick = (e: React.MouseEvent) => { | |
| e.stopPropagation(); | |
| node.select(); | |
| if (data.isDirectory) { | |
| node.toggle(); | |
| if (e.detail === 1) { | |
| node.toggle(); | |
| } | |
| return; | |
| } | |
| }; | |
| const handleDoubleClick = (e: React.MouseEvent) => { | |
| e.stopPropagation(); | |
| node.activate(); | |
| // Double-click: open in external editor | |
| if (e.detail === 2) { | |
| if (clickTimeoutRef.current) { | |
| clearTimeout(clickTimeoutRef.current); | |
| clickTimeoutRef.current = null; | |
| } | |
| onCancelOpen(); | |
| openInEditor(); | |
| return; | |
| } | |
| // Single-click: delay activate to allow double-click detection | |
| if (clickTimeoutRef.current) { | |
| clearTimeout(clickTimeoutRef.current); | |
| } | |
| clickTimeoutRef.current = setTimeout(() => { | |
| clickTimeoutRef.current = null; | |
| node.activate(); | |
| }, 250); | |
| import { useEffect, useRef } from "react"; | |
| import { LuChevronDown, LuChevronRight } from "react-icons/lu"; | |
| import type { FileTreeNode as FileTreeNodeType } from "shared/file-tree-types"; | |
| import { usePathActions } from "../../../ChangesView/hooks"; | |
| import { getFileIcon } from "../../utils"; | |
| const SINGLE_CLICK_DELAY_MS = 250; | |
| type FileTreeNodeProps = NodeRendererProps<FileTreeNodeType> & { | |
| worktreePath: string; | |
| onCancelOpen: () => void; | |
| }; | |
| export function FileTreeNode({ | |
| node, | |
| style, | |
| dragHandle, | |
| worktreePath, | |
| onCancelOpen, | |
| }: FileTreeNodeProps) { | |
| const { data } = node; | |
| const { icon: Icon, color } = getFileIcon( | |
| data.name, | |
| data.isDirectory, | |
| node.isOpen, | |
| ); | |
| const { openInEditor } = usePathActions({ | |
| absolutePath: data.path ?? null, | |
| relativePath: data.relativePath, | |
| cwd: worktreePath, | |
| }); | |
| const clickTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null); | |
| useEffect(() => { | |
| return () => { | |
| if (clickTimeoutRef.current) { | |
| clearTimeout(clickTimeoutRef.current); | |
| clickTimeoutRef.current = null; | |
| } | |
| }; | |
| }, []); | |
| const handleClick = (e: React.MouseEvent) => { | |
| if (data.isDirectory) { | |
| if (e.detail === 1) { | |
| node.toggle(); | |
| } | |
| return; | |
| } | |
| // Double-click: open in external editor | |
| if (e.detail === 2) { | |
| if (clickTimeoutRef.current) { | |
| clearTimeout(clickTimeoutRef.current); | |
| clickTimeoutRef.current = null; | |
| } | |
| onCancelOpen(); | |
| openInEditor(); | |
| return; | |
| } | |
| // Single-click: delay activate to allow double-click detection | |
| if (clickTimeoutRef.current) { | |
| clearTimeout(clickTimeoutRef.current); | |
| } | |
| clickTimeoutRef.current = setTimeout(() => { | |
| clickTimeoutRef.current = null; | |
| node.activate(); | |
| }, SINGLE_CLICK_DELAY_MS); | |
| }; |
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/components/FileTreeNode/FileTreeNode.tsx`
around lines 3 - 61, In FileTreeNode, extract the 250ms magic number into a
named constant (e.g., CLICK_DELAY_MS) at module top and ensure the pending
timeout stored in clickTimeoutRef is cleared on unmount: add a useEffect with a
cleanup that checks clickTimeoutRef.current, calls clearTimeout(...) and sets it
to null; keep existing handleClick logic and references to node.activate,
node.toggle, openInEditor and onCancelOpen unchanged.
Single-click opens file in pane (after 250ms delay), double-click opens in external editor. Uses e.detail to detect double-click since react-arborist's drag handling prevents dblclick event from firing.
Kitenite
force-pushed
the
kitenite/filtree-pt-2-1
branch
from
ae6e343 to
58a57ba
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/components/FileTreeContextMenu/FileTreeContextMenu.tsx`:
- Around line 40-42: The parentPath logic uses worktreePath for file entries
causing "New File/Folder" to target the workspace root; update
FileTreeContextMenu so parentPath is the file's parent directory when entry is a
file by computing a renderer-safe string dirname instead of importing Node's
path: add a small helper (e.g., getParentPath or dirnameString) in the same file
that returns the substring up to the last '/' (handling trailing slashes and
empty cases), then set parentPath = entry?.isDirectory ? entry.path :
getParentPath(entry.path ?? worktreePath) and leave targetPath as-is.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/FilesView.tsx`:
- Around line 123-129: The handler handleOpenInEditor currently sends every
DirectoryEntry to openFileInEditorMutation, which lets FileTreeItem
double-clicks open folders; change handleOpenInEditor (the useCallback that
takes entry: DirectoryEntry) to first check the entry's type/property (e.g.,
entry.type === 'file' or !entry.isDirectory) and return early for directories,
only calling openFileInEditorMutation.mutate({ path: entry.path, cwd:
worktreePath }) when the entry is a file; keep the existing worktreePath guard
and dependency array.
- Around line 41-75: The current ID encoding/parsing in getChildren and getItem
uses raw ":::", which can collide with file names/paths; change both sides to
use a reversible encoding for each segment (e.g., encodeURIComponent or Base64)
when constructing IDs in getChildren (where entries.map builds
`${e.path}:::${e.name}:::...`) and decode in getItem (where itemId.split(":::")
is used) so that splitting is always safe; ensure the same encoder/decoder is
used for all segments (path, name, relativePath, isDirectory) and update
DirectoryEntry parsing to use the decoded values.
- Around line 205-213: When toggling hidden files in the FilesView prop
onToggleHiddenFiles, update state and also rebuild the file tree so expanded
folders refetch children: inside the handler that currently calls
setShowHiddenFiles((v) => !v) (the onToggleHiddenFiles passed to the component),
call tree.rebuildTree() (or the file-tree instance/method used by FilesView)
immediately after toggling to force a refresh of entries with the new
showHiddenFiles flag.
🧹 Nitpick comments (3)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/index.tsx (1)
63-70: Extract polling interval to a named constant.The
refetchInterval: 2500is a magic number. Per coding guidelines, extract it to a named constant at the module top for clarity and maintainability.♻️ Suggested refactor
Add at module top:
const STATUS_POLL_INTERVAL_MS = 2500;Then update the query:
const { data: status } = electronTrpc.changes.getStatus.useQuery( { worktreePath: worktreePath || "", defaultBranch: effectiveBaseBranch }, { enabled: !!worktreePath, - refetchInterval: 2500, + refetchInterval: STATUS_POLL_INTERVAL_MS, refetchOnWindowFocus: true, }, );apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/components/FileSearchResultItem/FileSearchResultItem.tsx (1)
23-45: Refactor helper to params object for guideline compliance.The local helper uses positional params; prefer a named params object.
♻️ Suggested refactor
-function truncatePathStart(value: string, maxLength: number): string { +function truncatePathStart({ + value, + maxLength, +}: { + value: string; + maxLength: number; +}): string { if (value.length <= maxLength) { return value; } const sliceLength = Math.max(1, maxLength - 3); return `...${value.slice(value.length - sliceLength)}`; } ... -const folderLabelDisplay = truncatePathStart( - folderLabel, - PATH_LABEL_MAX_CHARS, -); +const folderLabelDisplay = truncatePathStart({ + value: folderLabel, + maxLength: PATH_LABEL_MAX_CHARS, +});As per coding guidelines: Functions with 2+ parameters should accept a single params object with named properties instead of positional arguments.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/FilesView.tsx (1)
172-174: Rename flow TODO left unimplemented.If you want, I can draft a headless‑tree rename implementation or open a follow‑up issue to track it.
Comment on lines
40
to
42
| const targetPath = entry?.path ?? worktreePath; | ||
| const parentPath = entry?.isDirectory ? entry.path : worktreePath; | ||
|
|
Contributor
There was a problem hiding this comment.
Fix parent path for file targets in context menu.
When right‑clicking a file, “New File/Folder” should use the file’s parent directory, not the workspace root.
A small string-based parent-path helper keeps this renderer-safe.
🛠️ Suggested fix
+function getParentPath(path: string): string | null {
+ const normalized = path.replace(/\\/g, "/");
+ const lastSlash = normalized.lastIndexOf("/");
+ if (lastSlash <= 0) return null;
+ return normalized.slice(0, lastSlash);
+}
+
export function FileTreeContextMenu({
children,
entry,
worktreePath,
onNewFile,
onNewFolder,
onRename,
onDelete,
}: FileTreeContextMenuProps) {
const targetPath = entry?.path ?? worktreePath;
- const parentPath = entry?.isDirectory ? entry.path : worktreePath;
+ const parentPath = entry
+ ? entry.isDirectory
+ ? entry.path
+ : getParentPath(entry.path) ?? worktreePath
+ : worktreePath;As per coding guidelines: apps/desktop/src/{renderer,lib/electron-router-dom.ts}/**/*.{ts,tsx} — Never import Node.js modules in renderer process or shared code in Electron apps.
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/components/FileTreeContextMenu/FileTreeContextMenu.tsx`
around lines 40 - 42, The parentPath logic uses worktreePath for file entries
causing "New File/Folder" to target the workspace root; update
FileTreeContextMenu so parentPath is the file's parent directory when entry is a
file by computing a renderer-safe string dirname instead of importing Node's
path: add a small helper (e.g., getParentPath or dirnameString) in the same file
that returns the substring up to the last '/' (handling trailing slashes and
empty cases), then set parentPath = entry?.isDirectory ? entry.path :
getParentPath(entry.path ?? worktreePath) and leave targetPath as-is.
Comment on lines
+41
to
+75
| getItem: async (itemId: string): Promise<DirectoryEntry> => { | ||
| if (itemId === "root") { | ||
| return { | ||
| ...entry, | ||
| children: entriesToNodes(cachedChildren), | ||
| id: "root", | ||
| name: "root", | ||
| path: worktreePath ?? "", | ||
| relativePath: "", | ||
| isDirectory: true, | ||
| }; | ||
| } | ||
|
|
||
| return { ...entry, children: null }; | ||
| }); | ||
| }, | ||
| [childrenCache, currentExpandedFolders], | ||
| ); | ||
|
|
||
| const treeData = useMemo((): FileTreeNodeType[] => { | ||
| if (!rootEntries) return []; | ||
| return entriesToNodes(rootEntries); | ||
| }, [rootEntries, entriesToNodes]); | ||
|
|
||
| const loadChildren = useCallback( | ||
| async (folderPath: string) => { | ||
| if ( | ||
| !worktreePath || | ||
| childrenCache[folderPath] || | ||
| loadingFolders.has(folderPath) | ||
| ) { | ||
| return; | ||
| } | ||
|
|
||
| setLoadingFolders((prev) => new Set(prev).add(folderPath)); | ||
|
|
||
| try { | ||
| const children = await trpcUtils.filesystem.readDirectory.fetch({ | ||
| dirPath: folderPath, | ||
| rootPath: worktreePath, | ||
| includeHidden: showHiddenFiles, | ||
| }); | ||
|
|
||
| setChildrenCache((prev) => ({ | ||
| ...prev, | ||
| [folderPath]: children, | ||
| })); | ||
| } catch (error) { | ||
| console.error("[FilesView] Failed to load children:", { | ||
| folderPath, | ||
| error, | ||
| }); | ||
| } finally { | ||
| setLoadingFolders((prev) => { | ||
| const next = new Set(prev); | ||
| next.delete(folderPath); | ||
| return next; | ||
| }); | ||
| } | ||
| const parts = itemId.split(":::"); | ||
| return { | ||
| id: itemId, | ||
| name: parts[1] ?? itemId, | ||
| path: parts[0] ?? itemId, | ||
| relativePath: parts[2] ?? "", | ||
| isDirectory: parts[3] === "true", | ||
| }; | ||
| }, | ||
| getChildren: async (itemId: string): Promise<string[]> => { | ||
| if (!worktreePath) return []; | ||
| const dirPath = | ||
| itemId === "root" ? worktreePath : itemId.split(":::")[0]; | ||
| if (!dirPath) return []; | ||
|
|
||
| try { | ||
| const entries = await trpcUtils.filesystem.readDirectory.fetch({ | ||
| dirPath, | ||
| rootPath: worktreePath, | ||
| includeHidden: showHiddenFiles, | ||
| }); | ||
| return entries.map( | ||
| (e) => | ||
| `${e.path}:::${e.name}:::${e.relativePath}:::${e.isDirectory}`, | ||
| ); |
Contributor
There was a problem hiding this comment.
Avoid delimiter collisions in item IDs.
Using a raw ":::" separator can break parsing if any path/name contains that sequence. Encode each segment to keep IDs reversible.
🛠️ Suggested fix
+const ITEM_ID_SEPARATOR = ":::";
+const encodeIdPart = (value: string) => encodeURIComponent(value);
+const decodeIdPart = (value: string) => decodeURIComponent(value);
+
const tree = useTree<DirectoryEntry>({
rootItemId: "root",
getItemName: (item) => item.getItemData()?.name ?? "",
isItemFolder: (item) => item.getItemData()?.isDirectory ?? false,
dataLoader: {
getItem: async (itemId: string): Promise<DirectoryEntry> => {
if (itemId === "root") {
return {
id: "root",
name: "root",
path: worktreePath ?? "",
relativePath: "",
isDirectory: true,
};
}
- const parts = itemId.split(":::");
+ const parts = itemId.split(ITEM_ID_SEPARATOR);
return {
id: itemId,
- name: parts[1] ?? itemId,
- path: parts[0] ?? itemId,
- relativePath: parts[2] ?? "",
+ name: decodeIdPart(parts[1] ?? itemId),
+ path: decodeIdPart(parts[0] ?? itemId),
+ relativePath: decodeIdPart(parts[2] ?? ""),
isDirectory: parts[3] === "true",
};
},
getChildren: async (itemId: string): Promise<string[]> => {
if (!worktreePath) return [];
const dirPath =
itemId === "root" ? worktreePath : itemId.split(":::")[0];
if (!dirPath) return [];
try {
const entries = await trpcUtils.filesystem.readDirectory.fetch({
dirPath,
rootPath: worktreePath,
includeHidden: showHiddenFiles,
});
return entries.map(
(e) =>
- `${e.path}:::${e.name}:::${e.relativePath}:::${e.isDirectory}`,
+ `${encodeIdPart(e.path)}${ITEM_ID_SEPARATOR}${encodeIdPart(
+ e.name,
+ )}${ITEM_ID_SEPARATOR}${encodeIdPart(
+ e.relativePath,
+ )}${ITEM_ID_SEPARATOR}${e.isDirectory}`,
);
} catch (error) {
console.error("[FilesView] Failed to load children:", error);
return [];
}
},
},
features: [asyncDataLoaderFeature, selectionFeature, expandAllFeature],
});🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/FilesView.tsx`
around lines 41 - 75, The current ID encoding/parsing in getChildren and getItem
uses raw ":::", which can collide with file names/paths; change both sides to
use a reversible encoding for each segment (e.g., encodeURIComponent or Base64)
when constructing IDs in getChildren (where entries.map builds
`${e.path}:::${e.name}:::...`) and decode in getItem (where itemId.split(":::")
is used) so that splitting is always safe; ensure the same encoder/decoder is
used for all segments (path, name, relativePath, isDirectory) and update
DirectoryEntry parsing to use the decoded values.
Comment on lines
+123
to
129
| const handleOpenInEditor = useCallback( | ||
| (entry: DirectoryEntry) => { | ||
| if (!worktreePath) return; | ||
| setSelectedItems( | ||
| worktreePath, | ||
| nodes.map((n) => n.data.id), | ||
| ); | ||
| }, | ||
| [worktreePath, setSelectedItems], | ||
| ); | ||
|
|
||
| const handleToggle = useCallback( | ||
| (id: string) => { | ||
| if (!worktreePath) return; | ||
| toggleFolder(worktreePath, id); | ||
|
|
||
| const node = treeRef.current?.get(id); | ||
| if (node?.data.isDirectory && !node.isOpen) { | ||
| loadChildren(node.data.path); | ||
| } | ||
| }, | ||
| [worktreePath, toggleFolder, loadChildren], | ||
| ); | ||
|
|
||
| const handleRename = useCallback( | ||
| ({ id, name }: { id: string; name: string }) => { | ||
| const node = treeRef.current?.get(id)?.data; | ||
| if (node) { | ||
| rename(node.path, name); | ||
| } | ||
| openFileInEditorMutation.mutate({ path: entry.path, cwd: worktreePath }); | ||
| }, | ||
| [rename], | ||
| [worktreePath, openFileInEditorMutation], | ||
| ); |
Contributor
There was a problem hiding this comment.
Guard directories in handleOpenInEditor.
FileTreeItem can call this for folders on double‑click; ensure only files invoke the external open call.
🛠️ Suggested fix
const handleOpenInEditor = useCallback(
(entry: DirectoryEntry) => {
- if (!worktreePath) return;
+ if (!worktreePath || entry.isDirectory) return;
openFileInEditorMutation.mutate({ path: entry.path, cwd: worktreePath });
},
[worktreePath, openFileInEditorMutation],
);🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/FilesView.tsx`
around lines 123 - 129, The handler handleOpenInEditor currently sends every
DirectoryEntry to openFileInEditorMutation, which lets FileTreeItem
double-clicks open folders; change handleOpenInEditor (the useCallback that
takes entry: DirectoryEntry) to first check the entry's type/property (e.g.,
entry.type === 'file' or !entry.isDirectory) and return early for directories,
only calling openFileInEditorMutation.mutate({ path: entry.path, cwd:
worktreePath }) when the entry is a file; keep the existing worktreePath guard
and dependency array.
Comment on lines
+205
to
213
| searchTerm={searchTerm} | ||
| onSearchChange={setSearchTerm} | ||
| onNewFile={() => handleNewFile(worktreePath)} | ||
| onNewFolder={() => handleNewFolder(worktreePath)} | ||
| onCollapseAll={handleCollapseAll} | ||
| onRefresh={handleRefresh} | ||
| showHiddenFiles={showHiddenFiles} | ||
| onToggleHiddenFiles={toggleHiddenFiles} | ||
| onToggleHiddenFiles={() => setShowHiddenFiles((v) => !v)} | ||
| /> |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the FilesView.tsx file
find . -name "FilesView.tsx" -path "*/apps/desktop/src/renderer/*"Repository: superset-sh/superset
Length of output: 166
🏁 Script executed:
# Get the file size to determine how much we can read
wc -l apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/FilesView.tsxRepository: superset-sh/superset
Length of output: 168
🏁 Script executed:
# Read the entire FilesView.tsx file to understand the implementation
cat -n apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/FilesView.tsxRepository: superset-sh/superset
Length of output: 11076
Rebuild tree after toggling hidden files.
When showHiddenFiles is toggled, already-expanded folders retain cached children. Call tree.rebuildTree() alongside the state update to refetch entries with the new visibility setting.
Suggested fix
+const handleToggleHiddenFiles = useCallback(() => {
+ setShowHiddenFiles((v) => !v);
+ tree.rebuildTree();
+}, [tree]);
+
...
<FileTreeToolbar
searchTerm={searchTerm}
onSearchChange={setSearchTerm}
onNewFile={() => handleNewFile(worktreePath)}
onNewFolder={() => handleNewFolder(worktreePath)}
onCollapseAll={handleCollapseAll}
onRefresh={handleRefresh}
showHiddenFiles={showHiddenFiles}
- onToggleHiddenFiles={() => setShowHiddenFiles((v) => !v)}
+ onToggleHiddenFiles={handleToggleHiddenFiles}
/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| searchTerm={searchTerm} | |
| onSearchChange={setSearchTerm} | |
| onNewFile={() => handleNewFile(worktreePath)} | |
| onNewFolder={() => handleNewFolder(worktreePath)} | |
| onCollapseAll={handleCollapseAll} | |
| onRefresh={handleRefresh} | |
| showHiddenFiles={showHiddenFiles} | |
| onToggleHiddenFiles={toggleHiddenFiles} | |
| onToggleHiddenFiles={() => setShowHiddenFiles((v) => !v)} | |
| /> | |
| const handleToggleHiddenFiles = useCallback(() => { | |
| setShowHiddenFiles((v) => !v); | |
| tree.rebuildTree(); | |
| }, [tree]); | |
| <FileTreeToolbar | |
| searchTerm={searchTerm} | |
| onSearchChange={setSearchTerm} | |
| onNewFile={() => handleNewFile(worktreePath)} | |
| onNewFolder={() => handleNewFolder(worktreePath)} | |
| onCollapseAll={handleCollapseAll} | |
| onRefresh={handleRefresh} | |
| showHiddenFiles={showHiddenFiles} | |
| onToggleHiddenFiles={handleToggleHiddenFiles} | |
| /> |
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/FilesView.tsx`
around lines 205 - 213, When toggling hidden files in the FilesView prop
onToggleHiddenFiles, update state and also rebuild the file tree so expanded
folders refetch children: inside the handler that currently calls
setShowHiddenFiles((v) => !v) (the onToggleHiddenFiles passed to the component),
call tree.rebuildTree() (or the file-tree instance/method used by FilesView)
immediately after toggling to force a refresh of entries with the new
showHiddenFiles flag.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/components/FileSearchResultItem/FileSearchResultItem.tsx`:
- Around line 90-92: The double-click handler handleDoubleClick should guard
against directories so it only calls onOpenInEditor for files: check the entry
object (e.g., entry.isDirectory or entry.type) inside handleDoubleClick and
return early (or call an alternative action like reveal in explorer) when the
entry is a directory, otherwise call onOpenInEditor(entry); update any tests or
UI expectations accordingly.
- Line 75: The context menu builds parentPath incorrectly: currently parentPath
= entry.isDirectory ? entry.path : worktreePath causes "New File"/"New Folder"
to be created at the worktree root for files; change parentPath calculation
inside FileSearchResultItem so that when entry.isDirectory you use entry.path,
otherwise compute the file's actual parent directory (e.g., dirname of
entry.path or use the project's helper to get parent) and use that parentPath
for the "New File" and "New Folder" context menu actions so new items are
created alongside the selected file.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/components/FileTreeItem/FileTreeItem.tsx`:
- Line 58: The parentPath computed in FileTreeItem (variable parentPath) is
incorrect for file entries because it currently falls back to worktreePath for
non-folders; change the logic so that for files parentPath is the file's actual
parent directory (use a dirname helper, e.g., path.dirname(entry.path) or
equivalent) and only use worktreePath when entry.path is missing; update imports
if needed (e.g., import { dirname } from 'path' or the project's path util) and
ensure the context-menu actions reference this corrected parentPath (same fix
pattern as in FileSearchResultItem).
- Around line 80-83: handleDoubleClick currently always calls
onOpenInEditor(entry), which will try to open directories in an external editor;
guard against that by checking the entry's type (e.g., entry.type,
entry.isDirectory, or entry.kind depending on the entry shape) inside
handleDoubleClick and only call onOpenInEditor(entry) when the entry represents
a file. Keep e.stopPropagation() and ensure directories either do nothing or
trigger the existing expand/collapse behavior instead of invoking
onOpenInEditor.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/components/NewItemInput/NewItemInput.tsx`:
- Around line 86-92: The cancel button in the NewItemInput component is
icon-only and lacks an accessible name; update the button (the one using
onCancel and rendering the LuX icon) to include an accessible label by adding an
aria-label (e.g., aria-label="Cancel") or a visually-hidden text node
(screen-reader-only span) inside the button so screen readers can announce its
purpose while preserving the icon-only visual.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/components/RenameInput/RenameInput.tsx`:
- Around line 72-99: The input and the two icon-only buttons in the RenameInput
component lack accessible labels; add accessible names by setting an aria-label
(or aria-labelledby) on the text input (the element using inputRef and
value/state handlers) and on both buttons that call handleSubmit and onCancel,
and mark decorative icons (LuCheck, LuX) as aria-hidden if you keep separate
labels; ensure the submit button (handleSubmit) and cancel button (onCancel)
have clear labels like "Confirm rename" and "Cancel rename" so screen readers
announce them.
- Around line 78-96: The input's onBlur (which calls handleSubmit) can run
before a confirm click, causing duplicate submits and the cancel button only
handles mouse input; update the blur handler to bail out when focus remains
inside the action container (use the onBlur event's relatedTarget or check
document.activeElement against the container element) so clicks on the
confirm/cancel buttons don't trigger submit, and replace the cancel button's
onMouseDown handler with an onClick that calls onCancel so it is
keyboard-accessible; additionally, make handleSubmit idempotent (e.g., guard
with an isSubmitting/ref flag) to ignore duplicate submits if both blur and
click fire.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/hooks/useFileTreeActions.ts`:
- Around line 38-39: The code in useFileTreeActions.ts computes parentPath by
splitting variables.oldPath on "/" which fails for Windows backslashes; update
the parent path logic to normalize separators before splitting (e.g., replace
all "\" with "/" or use a cross-platform path utility such as Node's
path.normalize/path.dirname or path-browserify) so parentPath is derived
correctly for both Windows and POSIX paths, then call onRefresh(parentPath ||
worktreePath || ""); ensure you update the computation that references
variables.oldPath and keep onRefresh and worktreePath usage unchanged.
- Around line 57-59: Fix the empty-array and Windows-separator issue when
deriving parentPath in useFileTreeActions: first check if variables.paths is
present and has at least one entry (if not, call onRefresh(worktreePath || "")
or return early as intended), then take the first path, normalize Windows
backslashes to "/" (e.g., firstPath.replaceAll("\\\\", "/")) before splitting,
compute parentPath via split("/").slice(0, -1).join("/"), and finally call
onRefresh(parentPath || worktreePath || ""); reference variables.paths,
firstPath, parentPath, onRefresh, and worktreePath to locate and update the
logic.
🧹 Nitpick comments (5)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/components/NewItemInput/NewItemInput.tsx (1)
25-33: PreferautoFocus+onFocusselection instead of a timeduseEffect.The timeout adds a magic number and an effect just to focus/select. You can avoid both and keep the behavior stable. As per coding guidelines, "Do not use effect unless absolutely necessary."
♻️ Suggested refactor
-import { useEffect, useRef, useState } from "react"; +import { useState } from "react"; @@ - const inputRef = useRef<HTMLInputElement>(null); - - useEffect(() => { - const timer = setTimeout(() => { - if (inputRef.current) { - inputRef.current.focus(); - inputRef.current.select(); - } - }, 50); - return () => clearTimeout(timer); - }, []); @@ <input - ref={inputRef} + autoFocus + onFocus={(e) => e.currentTarget.select()} type="text"apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/components/FileTreeItem/FileTreeItem.tsx (1)
137-186: Consider extracting shared context menu content.The context menu structure (lines 137-186) is identical to
FileSearchResultItem. Consider extracting a sharedFileContextMenuContentcomponent to reduce duplication and ensure consistency.apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/FilesView.tsx (2)
95-106: Linear search for item by path may be slow on large trees.The
onRefreshcallback iterates through all tree items viatree.getItems().find()to locate an item by path. For large directory structures, consider maintaining a path-to-itemId map or using the tree's internal lookup if available.
239-247: Redundant mapping of search results.The
searchResultEntriesmemo maps results to objects with the same shape as the input. IfsearchResultsalready conforms toDirectoryEntry, this transformation is unnecessary.♻️ Proposed simplification
-const searchResultEntries = useMemo(() => { - return searchResults.map((result) => ({ - id: result.id, - name: result.name, - path: result.path, - relativePath: result.relativePath, - isDirectory: result.isDirectory, - })); -}, [searchResults]); +const searchResultEntries = searchResults;If type coercion is needed, use a type assertion instead of runtime transformation.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/components/RenameInput/RenameInput.tsx (1)
23-35: Extract magic numbers into named constants.
Keeps intent clear and avoids scattered tweaks.♻️ Suggested refactor
@@ +const FOCUS_DELAY_MS = 50; +const INDENT_PX = 16; +const INDENT_BASE_PX = 4; + interface RenameInputProps { @@ - }, 50); + }, FOCUS_DELAY_MS); @@ - style={{ paddingLeft: `${level * 16 + 4}px` }} + style={{ paddingLeft: `${level * INDENT_PX + INDENT_BASE_PX}px` }}As per coding guidelines: Avoid magic numbers by extracting them to named constants at module top.
Also applies to: 68-68
Comment on lines
+90
to
+92
| const handleDoubleClick = () => { | ||
| onOpenInEditor(entry); | ||
| }; |
Contributor
There was a problem hiding this comment.
Guard directories in handleDoubleClick.
Double-clicking a directory will invoke onOpenInEditor, which attempts to open it in an external editor. This should be guarded to only open files.
🛠️ Proposed fix
const handleDoubleClick = () => {
+ if (entry.isDirectory) return;
onOpenInEditor(entry);
};🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/components/FileSearchResultItem/FileSearchResultItem.tsx`
around lines 90 - 92, The double-click handler handleDoubleClick should guard
against directories so it only calls onOpenInEditor for files: check the entry
object (e.g., entry.isDirectory or entry.type) inside handleDoubleClick and
return early (or call an alternative action like reveal in explorer) when the
entry is a directory, otherwise call onOpenInEditor(entry); update any tests or
UI expectations accordingly.
Comment on lines
+80
to
+83
| const handleDoubleClick = (e: React.MouseEvent) => { | ||
| e.stopPropagation(); | ||
| onOpenInEditor(entry); | ||
| }; |
Contributor
There was a problem hiding this comment.
Guard directories in handleDoubleClick.
Double-clicking a folder invokes onOpenInEditor, which attempts to open it in an external editor. Only files should trigger this action.
🛠️ Proposed fix
const handleDoubleClick = (e: React.MouseEvent) => {
e.stopPropagation();
+ if (isFolder) return;
onOpenInEditor(entry);
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const handleDoubleClick = (e: React.MouseEvent) => { | |
| e.stopPropagation(); | |
| onOpenInEditor(entry); | |
| }; | |
| const handleDoubleClick = (e: React.MouseEvent) => { | |
| e.stopPropagation(); | |
| if (isFolder) return; | |
| onOpenInEditor(entry); | |
| }; |
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/components/FileTreeItem/FileTreeItem.tsx`
around lines 80 - 83, handleDoubleClick currently always calls
onOpenInEditor(entry), which will try to open directories in an external editor;
guard against that by checking the entry's type (e.g., entry.type,
entry.isDirectory, or entry.kind depending on the entry shape) inside
handleDoubleClick and only call onOpenInEditor(entry) when the entry represents
a file. Keep e.stopPropagation() and ensure directories either do nothing or
trigger the existing expand/collapse behavior instead of invoking
onOpenInEditor.
Comment on lines
+86
to
+92
| <button | ||
| type="button" | ||
| onClick={onCancel} | ||
| className="p-0.5 hover:bg-background/50 rounded shrink-0" | ||
| > | ||
| <LuX className="size-3 text-muted-foreground" /> | ||
| </button> |
Contributor
There was a problem hiding this comment.
Add an accessible label to the icon-only cancel button.
This improves screen-reader usability.
✅ Suggested fix
<button
type="button"
onClick={onCancel}
+ aria-label="Cancel"
className="p-0.5 hover:bg-background/50 rounded shrink-0"
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <button | |
| type="button" | |
| onClick={onCancel} | |
| className="p-0.5 hover:bg-background/50 rounded shrink-0" | |
| > | |
| <LuX className="size-3 text-muted-foreground" /> | |
| </button> | |
| <button | |
| type="button" | |
| onClick={onCancel} | |
| aria-label="Cancel" | |
| className="p-0.5 hover:bg-background/50 rounded shrink-0" | |
| > | |
| <LuX className="size-3 text-muted-foreground" /> | |
| </button> |
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/components/NewItemInput/NewItemInput.tsx`
around lines 86 - 92, The cancel button in the NewItemInput component is
icon-only and lacks an accessible name; update the button (the one using
onCancel and rendering the LuX icon) to include an accessible label by adding an
aria-label (e.g., aria-label="Cancel") or a visually-hidden text node
(screen-reader-only span) inside the button so screen readers can announce its
purpose while preserving the icon-only visual.
Comment on lines
+72
to
+99
| <input | ||
| ref={inputRef} | ||
| type="text" | ||
| value={value} | ||
| onChange={(e) => setValue(e.target.value)} | ||
| onKeyDown={handleKeyDown} | ||
| onBlur={handleSubmit} | ||
| className={cn( | ||
| "flex-1 min-w-0 px-1 py-0 text-xs", | ||
| "bg-background border border-ring rounded outline-none", | ||
| )} | ||
| /> | ||
| <button | ||
| type="button" | ||
| onClick={handleSubmit} | ||
| className="p-0.5 hover:bg-background/50 rounded" | ||
| > | ||
| <LuCheck className="size-3 text-muted-foreground" /> | ||
| </button> | ||
| <button | ||
| type="button" | ||
| onMouseDown={(e) => { | ||
| e.preventDefault(); | ||
| onCancel(); | ||
| }} | ||
| className="p-0.5 hover:bg-background/50 rounded" | ||
| > | ||
| <LuX className="size-3 text-muted-foreground" /> |
Contributor
There was a problem hiding this comment.
Add accessible labels for the input and icon-only buttons.
Screen readers will announce these as unnamed controls.
♿ Suggested fix
@@
- <input
+ <input
ref={inputRef}
type="text"
+ aria-label={`Rename ${entry.name}`}
value={value}
onChange={(e) => setValue(e.target.value)}
onKeyDown={handleKeyDown}
onBlur={handleSubmit}
className={cn(
"flex-1 min-w-0 px-1 py-0 text-xs",
"bg-background border border-ring rounded outline-none",
)}
/>
<button
type="button"
+ aria-label="Confirm rename"
onClick={handleSubmit}
className="p-0.5 hover:bg-background/50 rounded"
>
@@
<button
type="button"
+ aria-label="Cancel rename"
onMouseDown={(e) => {
e.preventDefault();
onCancel();
}}
className="p-0.5 hover:bg-background/50 rounded"
>🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/components/RenameInput/RenameInput.tsx`
around lines 72 - 99, The input and the two icon-only buttons in the RenameInput
component lack accessible labels; add accessible names by setting an aria-label
(or aria-labelledby) on the text input (the element using inputRef and
value/state handlers) and on both buttons that call handleSubmit and onCancel,
and mark decorative icons (LuCheck, LuX) as aria-hidden if you keep separate
labels; ensure the submit button (handleSubmit) and cancel button (onCancel)
have clear labels like "Confirm rename" and "Cancel rename" so screen readers
announce them.
Comment on lines
+78
to
+96
| onBlur={handleSubmit} | ||
| className={cn( | ||
| "flex-1 min-w-0 px-1 py-0 text-xs", | ||
| "bg-background border border-ring rounded outline-none", | ||
| )} | ||
| /> | ||
| <button | ||
| type="button" | ||
| onClick={handleSubmit} | ||
| className="p-0.5 hover:bg-background/50 rounded" | ||
| > | ||
| <LuCheck className="size-3 text-muted-foreground" /> | ||
| </button> | ||
| <button | ||
| type="button" | ||
| onMouseDown={(e) => { | ||
| e.preventDefault(); | ||
| onCancel(); | ||
| }} |
Contributor
There was a problem hiding this comment.
Prevent duplicate submits and keep cancel keyboard-accessible.
onBlur can fire before the confirm click, and the cancel button only handles mouse input. Guard blur when focus stays inside the action container and switch cancel to a normal onClick.
💡 Suggested fix
@@
- const inputRef = useRef<HTMLInputElement>(null);
+ const inputRef = useRef<HTMLInputElement>(null);
+ const containerRef = useRef<HTMLDivElement>(null);
@@
const handleKeyDown = (e: React.KeyboardEvent) => {
e.stopPropagation();
if (e.key === "Enter") {
e.preventDefault();
handleSubmit();
}
if (e.key === "Escape") {
e.preventDefault();
onCancel();
}
};
+
+ const handleBlur = (e: React.FocusEvent<HTMLInputElement>) => {
+ const next = e.relatedTarget ?? document.activeElement;
+ if (next instanceof Node && containerRef.current?.contains(next)) {
+ return;
+ }
+ handleSubmit();
+ };
@@
- <div
+ <div
+ ref={containerRef}
className={cn("flex items-center gap-1 px-1 h-7", "bg-accent rounded-sm")}
style={{ paddingLeft: `${level * 16 + 4}px` }}
>
@@
- <input
+ <input
ref={inputRef}
type="text"
value={value}
onChange={(e) => setValue(e.target.value)}
onKeyDown={handleKeyDown}
- onBlur={handleSubmit}
+ onBlur={handleBlur}
className={cn(
"flex-1 min-w-0 px-1 py-0 text-xs",
"bg-background border border-ring rounded outline-none",
)}
/>
@@
- <button
- type="button"
- onMouseDown={(e) => {
- e.preventDefault();
- onCancel();
- }}
- className="p-0.5 hover:bg-background/50 rounded"
- >
+ <button
+ type="button"
+ onClick={onCancel}
+ className="p-0.5 hover:bg-background/50 rounded"
+ >📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| onBlur={handleSubmit} | |
| className={cn( | |
| "flex-1 min-w-0 px-1 py-0 text-xs", | |
| "bg-background border border-ring rounded outline-none", | |
| )} | |
| /> | |
| <button | |
| type="button" | |
| onClick={handleSubmit} | |
| className="p-0.5 hover:bg-background/50 rounded" | |
| > | |
| <LuCheck className="size-3 text-muted-foreground" /> | |
| </button> | |
| <button | |
| type="button" | |
| onMouseDown={(e) => { | |
| e.preventDefault(); | |
| onCancel(); | |
| }} | |
| const inputRef = useRef<HTMLInputElement>(null); | |
| const containerRef = useRef<HTMLDivElement>(null); | |
| const handleKeyDown = (e: React.KeyboardEvent) => { | |
| e.stopPropagation(); | |
| if (e.key === "Enter") { | |
| e.preventDefault(); | |
| handleSubmit(); | |
| } | |
| if (e.key === "Escape") { | |
| e.preventDefault(); | |
| onCancel(); | |
| } | |
| }; | |
| const handleBlur = (e: React.FocusEvent<HTMLInputElement>) => { | |
| const next = e.relatedTarget ?? document.activeElement; | |
| if (next instanceof Node && containerRef.current?.contains(next)) { | |
| return; | |
| } | |
| handleSubmit(); | |
| }; | |
| <div | |
| ref={containerRef} | |
| className={cn("flex items-center gap-1 px-1 h-7", "bg-accent rounded-sm")} | |
| style={{ paddingLeft: `${level * 16 + 4}px` }} | |
| > | |
| <input | |
| ref={inputRef} | |
| type="text" | |
| value={value} | |
| onChange={(e) => setValue(e.target.value)} | |
| onKeyDown={handleKeyDown} | |
| onBlur={handleBlur} | |
| className={cn( | |
| "flex-1 min-w-0 px-1 py-0 text-xs", | |
| "bg-background border border-ring rounded outline-none", | |
| )} | |
| /> | |
| <button | |
| type="button" | |
| onClick={handleSubmit} | |
| className="p-0.5 hover:bg-background/50 rounded" | |
| > | |
| <LuCheck className="size-3 text-muted-foreground" /> | |
| </button> | |
| <button | |
| type="button" | |
| onClick={onCancel} | |
| className="p-0.5 hover:bg-background/50 rounded" | |
| > |
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/components/RenameInput/RenameInput.tsx`
around lines 78 - 96, The input's onBlur (which calls handleSubmit) can run
before a confirm click, causing duplicate submits and the cancel button only
handles mouse input; update the blur handler to bail out when focus remains
inside the action container (use the onBlur event's relatedTarget or check
document.activeElement against the container element) so clicks on the
confirm/cancel buttons don't trigger submit, and replace the cancel button's
onMouseDown handler with an onClick that calls onCancel so it is
keyboard-accessible; additionally, make handleSubmit idempotent (e.g., guard
with an isSubmitting/ref flag) to ignore duplicate submits if both blur and
click fire.
Comment on lines
+38
to
+39
| const parentPath = variables.oldPath.split("/").slice(0, -1).join("/"); | ||
| onRefresh(parentPath || worktreePath || ""); |
Contributor
There was a problem hiding this comment.
Path splitting may fail on Windows.
Using split("/") assumes forward slashes, but Windows paths use backslashes. Consider normalizing the path first or using a cross-platform approach.
🛠️ Proposed fix
- const parentPath = variables.oldPath.split("/").slice(0, -1).join("/");
+ const normalized = variables.oldPath.replace(/\\/g, "/");
+ const parentPath = normalized.split("/").slice(0, -1).join("/");
onRefresh(parentPath || worktreePath || "");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const parentPath = variables.oldPath.split("/").slice(0, -1).join("/"); | |
| onRefresh(parentPath || worktreePath || ""); | |
| const normalized = variables.oldPath.replace(/\\/g, "/"); | |
| const parentPath = normalized.split("/").slice(0, -1).join("/"); | |
| onRefresh(parentPath || worktreePath || ""); |
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/hooks/useFileTreeActions.ts`
around lines 38 - 39, The code in useFileTreeActions.ts computes parentPath by
splitting variables.oldPath on "/" which fails for Windows backslashes; update
the parent path logic to normalize separators before splitting (e.g., replace
all "\" with "/" or use a cross-platform path utility such as Node's
path.normalize/path.dirname or path-browserify) so parentPath is derived
correctly for both Windows and POSIX paths, then call onRefresh(parentPath ||
worktreePath || ""); ensure you update the computation that references
variables.oldPath and keep onRefresh and worktreePath usage unchanged.
Comment on lines
+57
to
+59
| const firstPath = variables.paths[0]; | ||
| const parentPath = firstPath?.split("/").slice(0, -1).join("/"); | ||
| onRefresh(parentPath || worktreePath || ""); |
Contributor
There was a problem hiding this comment.
Same Windows path issue and potential empty array.
Same path-splitting concern as rename. Additionally, if variables.paths is empty, firstPath will be undefined and the parent derivation silently falls back to worktreePath, which may not be intended.
🛠️ Proposed fix
const firstPath = variables.paths[0];
- const parentPath = firstPath?.split("/").slice(0, -1).join("/");
+ const normalized = firstPath?.replace(/\\/g, "/");
+ const parentPath = normalized?.split("/").slice(0, -1).join("/");
onRefresh(parentPath || worktreePath || "");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const firstPath = variables.paths[0]; | |
| const parentPath = firstPath?.split("/").slice(0, -1).join("/"); | |
| onRefresh(parentPath || worktreePath || ""); | |
| const firstPath = variables.paths[0]; | |
| const normalized = firstPath?.replace(/\\/g, "/"); | |
| const parentPath = normalized?.split("/").slice(0, -1).join("/"); | |
| onRefresh(parentPath || worktreePath || ""); |
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/hooks/useFileTreeActions.ts`
around lines 57 - 59, Fix the empty-array and Windows-separator issue when
deriving parentPath in useFileTreeActions: first check if variables.paths is
present and has at least one entry (if not, call onRefresh(worktreePath || "")
or return early as intended), then take the first path, normalize Windows
backslashes to "/" (e.g., firstPath.replaceAll("\\\\", "/")) before splitting,
compute parentPath via split("/").slice(0, -1).join("/"), and finally call
onRefresh(parentPath || worktreePath || ""); reference variables.paths,
firstPath, parentPath, onRefresh, and worktreePath to locate and update the
logic.
- Use invalidateChildrenIds for proper tree refresh after create - Fix parent path for files to create peers in same folder - Clean up NewItemInput, remove focus hacks - Expand folder before showing new item input
This was referenced Feb 4, 2026
9 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Improvements
UI
✏️ Tip: You can customize this high-level summary in your review settings.