feat(desktop): folders + tree view modes for v2 changes sidebar#4442
Merged
Conversation
Adds v1's two grouping modes to the v2 changes sidebar: - Folders mode (new default): files grouped one level deep by parent folder, reusing FileRow; root files under a "Root Path" header. - Tree mode: one PierreFileTree per category section — hierarchy, virtualization, status tints, icons from Pierre; right-click parity with FileRow. View mode persists per workspace via sidebarState.changesViewMode; toggled from ChangesHeader. Extracts the Pierre row click-policy into a shared usePierreRowClickPolicy hook and lifts RowContextMenu up to WorkspaceSidebar/components/PierreRowContextMenu (used by both tabs).
Pierre's FileTree host is `height: 100%; overflow: hidden` when virtualized. Inside a changes section's auto-height container that collapses to 0, so the tree was invisible. Size each tree explicitly to its content: derive the expanded row count from the path list and recompute via model.subscribe when folders collapse/expand.
…e, selection sync - Hover overlay on file rows: Discard (unstaged) + more-actions ⌄ dropdown, matching FileRow. Anchors a light-DOM overlay over Pierre's shadow-root rows. - Right-click on directory rows: Open in Editor + copy-path actions. - Directory rows show a file-count decoration. - Echo the diff pane's open file into the tree's selection when it belongs to the section (with a feedback-loop guard). - Hoist the discard confirm dialog to ChangesTreeView — Pierre tears down renderContextMenu output on close, which would unmount a dialog inside it. Not done (out of reach in v2): per-file Stage/Unstage (host-service git API has no path-scoped staging), per-folder bulk actions, drag-to-copy path (Pierre owns row DOM), colored +N/−N (Pierre row decoration is text/icon only).
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a persisted "folders" vs "tree" view mode to the v2 Changes sidebar, extracts Pierre-specific click routing into Changes Sidebar View ModesChanges sidebar (single cohesive cohort)
Sequence Diagram sequenceDiagram
participant User
participant SidebarUI
participant PierreTree
participant ClickPolicy
participant TRPC
participant GitQueryCache
User->>SidebarUI: toggle viewMode ("folders" / "tree")
SidebarUI->>SidebarUI: persist changesViewMode
SidebarUI->>PierreTree: render tree (when "tree")
User->>PierreTree: click row (shadow DOM)
PierreTree->>ClickPolicy: composedPath -> findFileRow / folder
ClickPolicy-->>PierreTree: route action (select/open/external)
ClickPolicy->>SidebarUI: onSelectFile (open in pane / new tab)
User->>PierreTree: hover row
PierreTree->>SidebarUI: render ShadowRowHoverActions
User->>SidebarUI: request discard
SidebarUI->>TRPC: git.discardChanges(...)
TRPC->>GitQueryCache: invalidate status/diff queries
GitQueryCache-->>SidebarUI: updated file list
Estimated Code Review Effort 🎯 4 (Complex) | ⏱️ ~55 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 ports v1's two grouping modes — Folders (new default, one-level-deep parent-folder grouping) and Tree (full
Confidence Score: 4/5Safe to merge with one fix: the FilesTab click handler lost its null-rootPath guard during extraction and should be restored before shipping. The bulk of the change is additive new code (ChangesTreeView, ChangesFoldersView, ShadowRowHoverActions, ViewModeToggle) that is well-isolated and does not touch existing paths. The one regression risk is in FilesTab: the original capture-phase handler guarded against a null rootPath before constructing any absolute path, and that guard disappeared when the logic was extracted into usePierreRowClickPolicy. Everything else — the schema addition, the selection-echo feedback-loop guard, the hoisted discard dialog, and the closed-set collapse tracking — looks correct and intentional. FilesTab.tsx — null rootPath guard was silently removed during the usePierreRowClickPolicy extraction.
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/ChangesTreeView/ChangesTreeView.tsx | New core component: powers tree view for each changes section. Feedback-loop guard, explicit height sizing, hoisted discard dialog, and git-status sync are all well-reasoned. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx | Refactored to use shared usePierreRowClickPolicy, but the original null-rootPath guard was silently removed — callbacks now call toAbs(undefined, rel) if rootPath is unset at click time. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/ChangesTreeView/components/ShadowRowHoverActions/ShadowRowHoverActions.tsx | New light-DOM overlay for hover actions over Pierre's shadow-root rows. Rect is captured once per row entry and may go stale on reflow; otherwise the logic is solid. |
| apps/desktop/src/renderer/lib/clickPolicy/usePierreRowClickPolicy.ts | Clean extraction of capture-phase Pierre row click logic. composedPath walk, folder vs file branching, and deferred plain-pane handling are all correct. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/ChangesFoldersView/ChangesFoldersView.tsx | New folders-mode view. Closed-set approach for collapse state is an explicit improvement over v1's open-set behavior. Sorting and root-file handling look correct. |
| apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.ts | Adds changesViewMode field to the workspace local-state schema with a folders default. Schema and constant defaults are in sync. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/ChangesTreeView/components/FileRowContextMenuItems/FileRowContextMenuItems.tsx | File-row menu items for tree view — mirrors FileRow's menu vocabulary. Discard-delegation to parent is correctly implemented. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/PierreRowContextMenu/PierreRowContextMenu.tsx | Lifted from FilesTab; now shared between both tabs. Record<string, unknown> spread for data-* attrs loses type safety but is a minimal and documented escape hatch. |
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx:509-516
**Missing `rootPath` null guard after extraction**
The original `handleClickCapture` had an explicit `if (!rootPath) return;` guard before any path construction. The new delegation to `usePierreRowClickPolicy` removes that guard, but the callbacks still close over `rootPath` — if it is `null`/`undefined` at click time, `toAbs(rootPath, rel)` fires with a bad first argument, producing a malformed path (or a runtime error depending on `toAbs`'s implementation). The hook itself has no concept of `rootPath`, so the guard needs to live in each callback passed to it.
### Issue 2 of 2
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/ChangesTreeView/components/ShadowRowHoverActions/ShadowRowHoverActions.tsx:60-70
**Hover overlay rect can become stale mid-hover**
The bounding rect is captured once when `hoverRowRef.current !== row`, then never refreshed. If the Pierre tree reflows while the mouse is still over the same row (e.g., an adjacent section expands/collapses or the sidebar is resized), the `position: fixed` overlay will be painted at the old position until the cursor leaves and re-enters the row. The impact is cosmetic — the action buttons float to the wrong place — but it is noticeable on collapse/expand.
Reviews (1): Last reviewed commit: "feat(desktop): changes tree — hover acti..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
509
to
516
| @@ -564,7 +514,7 @@ export function FilesTab({ | |||
| const abs = toAbs(rootPath, item.path); | |||
| const rel = stripTrailingSlash(item.path); | |||
| return ( | |||
There was a problem hiding this comment.
rootPath null guard after extraction
The original handleClickCapture had an explicit if (!rootPath) return; guard before any path construction. The new delegation to usePierreRowClickPolicy removes that guard, but the callbacks still close over rootPath — if it is null/undefined at click time, toAbs(rootPath, rel) fires with a bad first argument, producing a malformed path (or a runtime error depending on toAbs's implementation). The hook itself has no concept of rootPath, so the guard needs to live in each callback passed to it.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx
Line: 509-516
Comment:
**Missing `rootPath` null guard after extraction**
The original `handleClickCapture` had an explicit `if (!rootPath) return;` guard before any path construction. The new delegation to `usePierreRowClickPolicy` removes that guard, but the callbacks still close over `rootPath` — if it is `null`/`undefined` at click time, `toAbs(rootPath, rel)` fires with a bad first argument, producing a malformed path (or a runtime error depending on `toAbs`'s implementation). The hook itself has no concept of `rootPath`, so the guard needs to live in each callback passed to it.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+60
to
+70
| [findFileRow, menuOpen], | ||
| ); | ||
|
|
||
| const handleMouseLeave = useCallback(() => { | ||
| if (menuOpen) return; | ||
| hoverRowRef.current = null; | ||
| setHover(null); | ||
| }, [menuOpen]); | ||
|
|
||
| return ( | ||
| // biome-ignore lint/a11y/noStaticElementInteractions: wraps a custom-element host with its own keyboard nav |
There was a problem hiding this comment.
The bounding rect is captured once when hoverRowRef.current !== row, then never refreshed. If the Pierre tree reflows while the mouse is still over the same row (e.g., an adjacent section expands/collapses or the sidebar is resized), the position: fixed overlay will be painted at the old position until the cursor leaves and re-enters the row. The impact is cosmetic — the action buttons float to the wrong place — but it is noticeable on collapse/expand.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/ChangesTreeView/components/ShadowRowHoverActions/ShadowRowHoverActions.tsx
Line: 60-70
Comment:
**Hover overlay rect can become stale mid-hover**
The bounding rect is captured once when `hoverRowRef.current !== row`, then never refreshed. If the Pierre tree reflows while the mouse is still over the same row (e.g., an adjacent section expands/collapses or the sidebar is resized), the `position: fixed` overlay will be painted at the old position until the cursor leaves and re-enters the row. The impact is cosmetic — the action buttons float to the wrong place — but it is noticeable on collapse/expand.
How can I resolve this? If you propose a fix, please make it concise.![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
apps/desktop/plans/20260510-changes-sidebar-diffs-tree.md (2)
131-133: 💤 Low valueAdd language identifier to the fenced code block.
The code block at line 131 is missing a language identifier. Adding one enables syntax highlighting.
📝 Proposed fix
-``` +```text apps/desktop/src/renderer/lib/clickPolicy/usePierreRowClickPolicy.ts🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/plans/20260510-changes-sidebar-diffs-tree.md` around lines 131 - 133, The fenced code block showing "apps/desktop/src/renderer/lib/clickPolicy/usePierreRowClickPolicy.ts" is missing a language identifier; update that markdown block to include a language tag (for example add "text" or "bash" after the opening ``` ) so the file path block is rendered with proper syntax highlighting/formatting.
94-113: 💤 Low valueAdd language identifier to the fenced code block.
The code block starting at line 94 is missing a language identifier. Adding one enables syntax highlighting in markdown renderers.
📝 Proposed fix
-``` +```text ChangesFileList/ ├── ChangesFileList.tsx # Orchestration: reads viewMode, picks renderer🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/plans/20260510-changes-sidebar-diffs-tree.md` around lines 94 - 113, The fenced code block that lists the ChangesFileList tree (the block starting with "ChangesFileList/") is missing a language identifier; update the opening fence from ``` to ```text (or another appropriate language tag) so markdown renderers enable syntax highlighting for that block in the document that contains ChangesFileList, ChangesFileList.tsx, and the components/ list.apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesHeader/components/ViewModeToggle/ViewModeToggle.tsx (1)
42-69: ⚖️ Poor tradeoffConsider extracting
ToggleButtonto a separate file.The file contains both
ViewModeToggleandToggleButton. As per coding guidelines, each file should contain only one component. WhileToggleButtonis a small internal helper, strictly following the guidelines would place it incomponents/ToggleButton/ToggleButton.tsx.That said, given its limited scope and single use within this module, keeping it co-located is a reasonable trade-off for simplicity.
As per coding guidelines: "One component per file (no multi-component files)."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesHeader/components/ViewModeToggle/ViewModeToggle.tsx around lines 42 - 69, Extract the internal ToggleButton component into its own component file named ToggleButton (export default) and move the ToggleButtonProps type with it; in the current file replace the local declaration with an import of ToggleButton and ensure ViewModeToggle still uses the same prop names and handlers (icon: Icon, label, active, onClick). Preserve the existing Tooltip/TooltipTrigger/TooltipContent usage and className logic exactly, export types if ViewModeToggle relies on them, and update any local references to ToggleButtonProps or ToggleButton to the new module.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/desktop/plans/20260510-changes-sidebar-diffs-tree.md`:
- Around line 131-133: The fenced code block showing
"apps/desktop/src/renderer/lib/clickPolicy/usePierreRowClickPolicy.ts" is
missing a language identifier; update that markdown block to include a language
tag (for example add "text" or "bash" after the opening ``` ) so the file path
block is rendered with proper syntax highlighting/formatting.
- Around line 94-113: The fenced code block that lists the ChangesFileList tree
(the block starting with "ChangesFileList/") is missing a language identifier;
update the opening fence from ``` to ```text (or another appropriate language
tag) so markdown renderers enable syntax highlighting for that block in the
document that contains ChangesFileList, ChangesFileList.tsx, and the components/
list.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesHeader/components/ViewModeToggle/ViewModeToggle.tsx:
- Around line 42-69: Extract the internal ToggleButton component into its own
component file named ToggleButton (export default) and move the
ToggleButtonProps type with it; in the current file replace the local
declaration with an import of ToggleButton and ensure ViewModeToggle still uses
the same prop names and handlers (icon: Icon, label, active, onClick). Preserve
the existing Tooltip/TooltipTrigger/TooltipContent usage and className logic
exactly, export types if ViewModeToggle relies on them, and update any local
references to ToggleButtonProps or ToggleButton to the new module.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2f70047d-f008-4781-9449-bdf49e4892bc
📒 Files selected for processing (28)
apps/desktop/plans/20260510-changes-sidebar-diffs-tree.mdapps/desktop/src/renderer/lib/clickPolicy/index.tsapps/desktop/src/renderer/lib/clickPolicy/usePierreRowClickPolicy.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/WorkspaceSidebar.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/RowContextMenu/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/PierreRowContextMenu/PierreRowContextMenu.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/PierreRowContextMenu/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/ChangesFileList.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/ChangesFoldersView/ChangesFoldersView.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/ChangesFoldersView/components/FolderHeader/FolderHeader.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/ChangesFoldersView/components/FolderHeader/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/ChangesFoldersView/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/ChangesTreeView/ChangesTreeView.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/ChangesTreeView/components/FileRowContextMenuItems/FileRowContextMenuItems.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/ChangesTreeView/components/FileRowContextMenuItems/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/ChangesTreeView/components/FolderContextMenuItems/FolderContextMenuItems.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/ChangesTreeView/components/FolderContextMenuItems/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/ChangesTreeView/components/ShadowRowHoverActions/ShadowRowHoverActions.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/ChangesTreeView/components/ShadowRowHoverActions/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/ChangesTreeView/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/FileRow/FileRow.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesHeader/ChangesHeader.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesHeader/components/ViewModeToggle/ViewModeToggle.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesHeader/components/ViewModeToggle/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesTabContent/ChangesTabContent.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/useChangesTab.tsxapps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.ts
💤 Files with no reviewable changes (1)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/RowContextMenu/index.ts
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
3 issues found across 28 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/ChangesTreeView/components/FolderContextMenuItems/FolderContextMenuItems.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/ChangesTreeView/components/FolderContextMenuItems/FolderContextMenuItems.tsx:34">
P2: Disable "Open in Editor" when no absolute path can be resolved; it is currently enabled but can no-op before `worktreePath` is available.</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/ChangesFoldersView/ChangesFoldersView.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/ChangesFoldersView/ChangesFoldersView.tsx:62">
P2: Avoid using a root key sentinel that can collide with real folder names; this can create duplicate React keys and unstable group rendering.</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/ChangesTreeView/components/ShadowRowHoverActions/ShadowRowHoverActions.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/ChangesTreeView/components/ShadowRowHoverActions/ShadowRowHoverActions.tsx:74">
P2: Hover actions can become desynced from the actual row during scroll because the overlay is anchored from a stale rect and not reset on scroll events.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| <> | ||
| <DropdownMenuItem | ||
| onSelect={() => onOpenInEditor?.(relativePath)} | ||
| disabled={!onOpenInEditor} |
Contributor
There was a problem hiding this comment.
P2: Disable "Open in Editor" when no absolute path can be resolved; it is currently enabled but can no-op before worktreePath is available.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/ChangesTreeView/components/FolderContextMenuItems/FolderContextMenuItems.tsx, line 34:
<comment>Disable "Open in Editor" when no absolute path can be resolved; it is currently enabled but can no-op before `worktreePath` is available.</comment>
<file context>
@@ -0,0 +1,50 @@
+ <>
+ <DropdownMenuItem
+ onSelect={() => onOpenInEditor?.(relativePath)}
+ disabled={!onOpenInEditor}
+ >
+ <ExternalLink />
</file context>
… section A thin action row below each section header (both folders and tree modes), mirroring FilesTab's header button strip. Folders mode toggles the folder groups; tree mode collapses/expands the Pierre tree's directories (which resizes the section via the existing row-count subscription).
|
Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews. |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/SectionToolbar/SectionToolbar.tsx (1)
41-58: ⚡ Quick winExtract
ToolbarButtonto its own file per coding guidelines.The
ToolbarButtoncomponent should be moved to a separate file to comply with the project guideline. As per coding guidelines, each component should be in its own file with the structureToolbarButton/ToolbarButton.tsx.Since this helper is only used within
SectionToolbar, nest it underSectionToolbar/components/ToolbarButton/ToolbarButton.tsx.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/SectionToolbar/SectionToolbar.tsx around lines 41 - 58, The ToolbarButton component is defined inline in SectionToolbar; extract it into its own component file so it follows the project guideline: create a new component file (e.g., SectionToolbar/components/ToolbarButton/ToolbarButton.tsx) that exports the ToolbarButton function (including ToolbarButtonProps), move the JSX and props there, then replace the inline definition in SectionToolbar with an import of ToolbarButton and keep all behavior/props (icon, label, onClick) unchanged; ensure exports/imports are updated and tests/usage in SectionToolbar still compile.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/SectionToolbar/SectionToolbar.tsx:
- Around line 41-58: The ToolbarButton component is defined inline in
SectionToolbar; extract it into its own component file so it follows the project
guideline: create a new component file (e.g.,
SectionToolbar/components/ToolbarButton/ToolbarButton.tsx) that exports the
ToolbarButton function (including ToolbarButtonProps), move the JSX and props
there, then replace the inline definition in SectionToolbar with an import of
ToolbarButton and keep all behavior/props (icon, label, onClick) unchanged;
ensure exports/imports are updated and tests/usage in SectionToolbar still
compile.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6ec1a433-df96-4562-96c5-3a7244b43b56
📒 Files selected for processing (4)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/ChangesFoldersView/ChangesFoldersView.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/ChangesTreeView/ChangesTreeView.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/SectionToolbar/SectionToolbar.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/SectionToolbar/index.ts
✅ Files skipped from review due to trivial changes (1)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/SectionToolbar/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/ChangesTreeView/ChangesTreeView.tsx
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/ChangesFoldersView/ChangesFoldersView.tsx
… + per-section toolbars - New ChangesToolbar row beneath the changes header: folders/tree toggle on the left (moved out of the header), expand-all / collapse-all on the right. - Removed the per-section collapse/expand toolbars; expand/collapse-all now fans out to every section via a fold-signal prop (folder groups in folders mode, Pierre tree directories in tree mode). - Dropped the muted background tint from the changes header.
The previous estimate (dirs + files) over-counted badly for deep monorepo paths because Pierre flattens single-child directory chains into one row, so the tree host ended up far taller than its content (big gap below the rows). Read the content height Pierre writes to the virtualized list's inline style instead, re-reading after each model mutation.
… header/toolbar divider
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/ChangesTreeView/ChangesTreeView.tsx (1)
210-212: ⚡ Quick winCoalesce RAF reads to avoid stacked measurement callbacks.
Line 210 schedules a new
requestAnimationFrameon every model event without canceling a pending one first.Proposed tweak
const unsubscribe = model.subscribe(() => { + cancelAnimationFrame(raf); raf = requestAnimationFrame(readHeight); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/ChangesTreeView/ChangesTreeView.tsx around lines 210 - 212, The model.subscribe callback currently calls requestAnimationFrame(readHeight) on every event which stacks callbacks; before scheduling a new RAF, cancel any pending one by calling cancelAnimationFrame(raf) (where raf is the variable used to hold the handle), then assign raf = requestAnimationFrame(readHeight); also ensure raf is initialized in the surrounding scope and cleared (cancelAnimationFrame and set raf = null) when unsubscribe is run/cleanup occurs so only one pending readHeight is ever scheduled from model.subscribe.apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesToolbar/ChangesToolbar.tsx (1)
31-31: ⚡ Quick winToolbar alignment doesn’t match the left/right control layout.
Line 31 uses
justify-end, which right-aligns both controls. If the intent is toggle-left and fold-right, switch to a split layout.Proposed tweak
- <div className="flex items-center justify-end gap-1 border-b border-border px-2 py-0.5"> + <div className="flex items-center justify-between gap-1 border-b border-border px-2 py-0.5">🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesToolbar/ChangesToolbar.tsx at line 31, The toolbar container in ChangesToolbar (the top-level div with className "flex items-center justify-end gap-1 border-b border-border px-2 py-0.5") currently right-aligns all controls; change the layout to split controls left and right by replacing justify-end with a split layout (e.g., use "justify-between" on that container and wrap left-side controls in a div and right-side controls in another div, or keep a single container and add two child divs with "flex items-center" to group toggle-left and fold-right respectively) so the toggle sits left and the fold button sits right.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/ChangesFoldersView/ChangesFoldersView.tsx:
- Line 81: The JSX uses a non-unique key expression (key={isRoot ? "__root__" :
group.folderPath}) which can collide with a real folder named "__root__"; update
the key generation in the ChangesFoldersView component to produce
guaranteed-unique keys by including a stable prefix/namespace for each branch
(e.g., use "root:<id>" for the root case and "folder:<folderPath>" for folder
rows) and ensure you reference the existing isRoot flag and group.folderPath
identifiers when constructing the new key so React reconciliation is safe.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/ChangesTreeView/ChangesTreeView.tsx:
- Around line 210-212: The model.subscribe callback currently calls
requestAnimationFrame(readHeight) on every event which stacks callbacks; before
scheduling a new RAF, cancel any pending one by calling
cancelAnimationFrame(raf) (where raf is the variable used to hold the handle),
then assign raf = requestAnimationFrame(readHeight); also ensure raf is
initialized in the surrounding scope and cleared (cancelAnimationFrame and set
raf = null) when unsubscribe is run/cleanup occurs so only one pending
readHeight is ever scheduled from model.subscribe.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesToolbar/ChangesToolbar.tsx:
- Line 31: The toolbar container in ChangesToolbar (the top-level div with
className "flex items-center justify-end gap-1 border-b border-border px-2
py-0.5") currently right-aligns all controls; change the layout to split
controls left and right by replacing justify-end with a split layout (e.g., use
"justify-between" on that container and wrap left-side controls in a div and
right-side controls in another div, or keep a single container and add two child
divs with "flex items-center" to group toggle-left and fold-right respectively)
so the toggle sits left and the fold button sits right.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2a4750ba-fd1c-4f1f-8509-1d5f8a0ca4e2
📒 Files selected for processing (8)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/ChangesFileList.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/ChangesFoldersView/ChangesFoldersView.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/components/ChangesTreeView/ChangesTreeView.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesHeader/ChangesHeader.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesTabContent/ChangesTabContent.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesToolbar/ChangesToolbar.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesToolbar/index.ts
✅ Files skipped from review due to trivial changes (3)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesToolbar/index.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/index.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesHeader/ChangesHeader.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/ChangesFileList.tsx
Contributor
|
You're iterating quickly on this pull request. To help protect your rate limits, cubic has paused automatic reviews on new pushes for now—when you're ready for another review, comment |
…eIcons Move FileIcon / getFileIcon / resolveFileIconAssetUrl / loadFallthroughIcons and the manifest typing into one shared module so the v2 sidebar trees, the diff/pane chrome, and the chat/mention surfaces all draw from the same place (v1 FilesView/utils now just re-exports it). Also give unrecognized file types a real fallback icon in the Pierre trees: loadFallthroughIcons now adds the Material default-file icon to the sprite and remaps Pierre's generic `file` slot to it, matching what the non-tree FileIcon surfaces already do via manifest.defaultIcon.
The native `title` attribute doesn't render reliably in the renderer; use the shadcn Tooltip (same component FileRow uses for its hover hint) so the full folder path shows on hover.
…sTab - New renderer/lib/pierreTree: createPierreTreeStyle (the ~55-line --trees-* CSS-var map, parameterized by row height / indent / search chrome) and the FILE_STATUS_TO_PIERRE git-status mapping — both were duplicated verbatim in FilesTab and ChangesTreeView. - FilesTab (722 → ~290 lines): pull the filesystem actions (create / rename / delete / reveal / collapse-all + the bridge bookkeeping) into a useFilesTabActions hook; move buildPierreGitStatus and the creation-path helpers into utils/; lift the header icon button into its own component; add a local constants.ts (FILE_EXPLORER_ROW_HEIGHT/INDENT/OVERSCAN), which also drops FilesTab's last import from the sunsetting v1 FilesView code. - ChangesTreeView: use the shared style + status map; extract the content- height measurement into a useMeasuredTreeHeight hook and buildTreeShape into utils/. No behavior change.
The setIcons-after-load effect was copy-pasted in FilesTab and ChangesTreeView; move it into renderer/lib/fileIcons as useFallthroughIcons(model).
It was defined in FilesTab's treePath utils and duplicated inline in ChangesTreeView; move it to renderer/lib/pierreTree (the Pierre tree's directory-marker convention) and re-export it from treePath.
Per AGENTS rule #6 (reference docs live in <app>/docs/). Moves the HOST_SERVICE_* and V2_WORKSPACE_* docs; fixes the few path references in plans/ and a code comment, and the GIT_REFS.md relative link inside V2_WORKSPACE_CREATION.md.
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
Drops the "__root__" sentinel key, which could collide with a real
top-level folder named "__root__"; folderPath ("" for root) is already
the unique per-group discriminator.
…cement
The previous commit put a JSX `{/* */}` comment directly after `return (`,
which biome's formatter then mangled into invalid syntax. Move it to a plain
`//` comment above the return.
This was referenced May 13, 2026
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
Brings v1's two grouping modes into the v2 changes sidebar, reusing the Pierre file tree (
@pierre/trees) the v2 files explorer already uses.FileRow; root-level files sit under a "Root Path" header. Files under a folder show just the basename (no redundant dir prefix).PierreFileTreeper category section (Unstaged / Staged / Against base / Committed) — full directory hierarchy, expand/collapse, virtualization, git-status tints, and filetype icons all from Pierre. The tree is sized to its content (Pierre's host isheight:100%when virtualized, which collapses to 0 in the section's auto-height container).sidebarState.changesViewMode.Tree-mode parity with
FileRow:⌄dropdown (Open Diff / Open Diff in New Tab / Open File / Open File in New Tab / Open in Editor / copy-path). Anchored as a light-DOM overlay since Pierre owns row DOM in an open shadow root.+N/−N.usePierreRowClickPolicyhook extracted fromFilesTab.Refactors along the way:
usePierreRowClickPolicy—FilesTab's capture-phase click logic extracted tolib/clickPolicyand reused by the changes tree.RowContextMenulifted fromFilesTab/components/toWorkspaceSidebar/components/PierreRowContextMenu(both tabs use it now).renderContextMenuoutput, which Pierre unmounts on close — so tree-menu Discard would never have shown its dialog. It's now hoisted toChangesTreeView.Not included (out of reach in v2)
FileRowonly has a hover Discard).dragstartonly fires ondraggableelements and enabling Pierre's DnD would turn drags into file moves.+N/−Nin tree mode — Pierre's row decoration is text/icon only, no markup.Test plan
⌄dropdown opens and all items fireFilesTabstill behaves identically after the click-policy extractionSummary by cubic
Adds Folders and Tree view modes to the v2 changes sidebar, plus a unified toolbar for filter/stats, view toggle, and global expand/collapse. The mode persists per workspace, trees auto-size to real content, and the hover overlay now hides while scrolling.
New Features
@pierre/trees): one tree per section with hierarchy, virtualization, status tints, and icons; file‑count on dirs and +N/−N on files; hover Discard (unstaged) and more‑actions dropdown; right‑click menus for files/folders; selection sync with the open diff; settings‑driven click tiers; explicit content‑height sizing.Refactors
usePierreRowClickPolicy; lifted the row context menu toWorkspaceSidebar/components/PierreRowContextMenu; hoisted the Discard confirm dialog so it isn’t unmounted when a menu closes.renderer/lib/fileIconswithuseFallthroughIcons;loadFallthroughIconsremaps Pierre’s genericfileslot to the Material default icon.renderer/lib/pierreTree(createPierreTreeStyle,FILE_STATUS_TO_PIERRE,useMeasuredTreeHeight, helpers) and decomposed FilesTab (actions hook, constants, header button, utils); no behavior change.apps/desktop/docs/and fixed references.ChangesFoldersView; drop the tree hover overlay on scroll.Written for commit fe701b4. Summary will update on new commits.
Summary by CodeRabbit