feat: Enable alt/option + click to expand and collapse subdirectories recursively in the file browser#738
feat: Enable alt/option + click to expand and collapse subdirectories recursively in the file browser#738scriptease wants to merge 2 commits intoRunMaestro:rcfrom
Conversation
… recursively in the file browser Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughA new recursive folder toggle was added: pressing Alt while clicking a folder calls Changes
Sequence DiagramsequenceDiagram
participant User as User
participant FE as FileExplorerPanel
participant RP as RightPanel
participant Hook as useAppHandlers
participant State as FileTreeState
User->>FE: Click folder (Alt key)
FE->>FE: Detect e.altKey = true
FE->>RP: invoke prop -> toggleFolderRecursive(path, sessionId, setSessions)
RP->>Hook: forward toggleFolderRecursive(...)
Hook->>State: traverse fileTree, collect descendant folder paths
Hook->>State: update fileExplorerExpanded with all collected paths
State-->>Hook: updated
Hook-->>FE: sessions/state changed (re-render)
FE-->>User: folder and descendants expanded/collapsed
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ 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 |
Greptile SummaryThis PR adds alt/option+click support to recursively expand or collapse subdirectories in the file browser. When a user holds Alt and clicks a folder, the target folder and all of its descendant folders are toggled together (expand all or collapse all), complementing the existing single-folder toggle and the global expand/collapse-all buttons. The implementation is clean and consistent with existing patterns:
Two minor style suggestions:
Confidence Score: 4/5Safe to merge — the new feature is additive and does not affect existing click behaviour. The recursive tree traversal logic is correct; expand and collapse paths are both handled; the fallback is sensible; and the prop-drilling chain is complete. Minor concerns are limited to UX discoverability and a cosmetic code-structure point, neither of which affects correctness. No files require special attention beyond the style suggestions already noted. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User Alt+Clicks a folder] --> B{isFolder && e.altKey?}
B -- Yes --> C[toggleFolderRecursive called]
B -- No --> D[toggleFolder called — single toggle]
C --> E[findSubtreeFolders traverses file tree]
E --> F{Target node found?}
F -- Yes --> G[Collect target + all descendant folder paths]
F -- No --> H[Fallback: use only the clicked path]
G --> I{Currently expanded?}
H --> I
I -- Yes --> J[Delete all collected paths from expanded set — collapse]
I -- No --> K[Add all collected paths to expanded set — expand]
J --> L[Update session state]
K --> L
L --> M[File tree re-renders with new expansion state]
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const toggleFolderRecursive = useCallback( | ||
| ( | ||
| path: string, | ||
| sessionId: string, | ||
| setSessionsFn: React.Dispatch<React.SetStateAction<Session[]>> | ||
| ) => { | ||
| setSessionsFn((prev) => | ||
| prev.map((s) => { | ||
| if (s.id !== sessionId) return s; | ||
| if (!s.fileExplorerExpanded || !s.fileTree) return s; | ||
| const expanded = new Set(s.fileExplorerExpanded); | ||
| const isCurrentlyExpanded = expanded.has(path); | ||
|
|
||
| // Find the node at this path and collect all descendant folder paths | ||
| const collectDescendantFolders = ( | ||
| nodes: typeof s.fileTree, | ||
| currentPath: string | ||
| ): string[] => { | ||
| const result: string[] = []; | ||
| for (const node of nodes!) { | ||
| const fullPath = currentPath ? `${currentPath}/${node.name}` : node.name; | ||
| if (node.type === 'folder') { | ||
| result.push(fullPath); | ||
| if (node.children) { | ||
| result.push(...collectDescendantFolders(node.children, fullPath)); | ||
| } | ||
| } | ||
| } | ||
| return result; | ||
| }; | ||
|
|
||
| // Find the subtree starting at the target path | ||
| const findSubtreeFolders = ( | ||
| nodes: typeof s.fileTree, | ||
| currentPath: string | ||
| ): string[] | null => { | ||
| for (const node of nodes!) { | ||
| const fullPath = currentPath ? `${currentPath}/${node.name}` : node.name; | ||
| if (fullPath === path && node.type === 'folder') { | ||
| // Found the target - collect all descendants | ||
| const descendants = node.children | ||
| ? collectDescendantFolders(node.children, fullPath) | ||
| : []; | ||
| return [fullPath, ...descendants]; | ||
| } | ||
| if (node.type === 'folder' && node.children && path.startsWith(fullPath + '/')) { | ||
| // Recurse into this folder | ||
| const found = findSubtreeFolders(node.children, fullPath); | ||
| if (found) return found; | ||
| } | ||
| } | ||
| return null; | ||
| }; | ||
|
|
||
| const allPaths = findSubtreeFolders(s.fileTree, '') || [path]; | ||
|
|
||
| if (isCurrentlyExpanded) { | ||
| // Collapse: remove the folder and all descendants | ||
| for (const p of allPaths) { | ||
| expanded.delete(p); | ||
| } | ||
| } else { | ||
| // Expand: add the folder and all descendants | ||
| for (const p of allPaths) { | ||
| expanded.add(p); | ||
| } | ||
| } | ||
|
|
||
| return { ...s, fileExplorerExpanded: Array.from(expanded) }; | ||
| }) | ||
| ); | ||
| }, | ||
| [] | ||
| ); |
There was a problem hiding this comment.
Both collectDescendantFolders and findSubtreeFolders are declared inside the setSessionsFn state-updater callback. They are recreated on every invocation of the updater. While this is functionally correct and unlikely to be a noticeable bottleneck for most file trees, it is an unusual pattern. Moving them outside the useCallback (as module-level helpers or into useCallback's body before the setSessionsFn call) would be cleaner and avoids any potential issue if the state updater were ever called in rapid succession on a very large tree.
// Suggested: lift helpers out of the updater callback
const collectDescendantFolders = (nodes: FileNode[], currentPath: string): string[] => { ... };
const findSubtreeFolders = (nodes: FileNode[], currentPath: string, targetPath: string): string[] | null => { ... };
const toggleFolderRecursive = useCallback(
(path, sessionId, setSessionsFn) => {
setSessionsFn((prev) =>
prev.map((s) => {
// ... use the lifted helpers
})
);
},
[]
);Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
|
Thanks for the contribution, @scriptease! This is a nice quality-of-life improvement. The implementation is clean — prop threading follows the existing Looks good to merge! 👍 |
…scope Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
3 participants
Just adding a small shortcut in the middle of the collapse and expand all button
Summary by CodeRabbit