feat(desktop): v2 sidebar changes tab improvements#3246
Merged
Conversation
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:
📝 WalkthroughWalkthroughReplaced letter status markers with icon-based indicators and updated colors; memoized list components and grouping; introduced ChangesHeader and ChangesTabContent components and new types; moved filtering/rendering into ChangesTabContent; added a shared 300ms debounced git-query invalidation for both Changes
Sequence Diagram(s)sequenceDiagram
participant Event as WorkspaceEventEmitter
participant Hook as useChangesTab Hook
participant Timer as DebounceTimer
participant Query as QueryClient
Event->>Hook: emit "git:changed" / "fs:events"
Hook->>Timer: start or reset 300ms debounce
Note right of Timer: subsequent events reset timer
Timer-->>Hook: debounce elapsed
Hook->>Query: invalidate git-related queries
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 makes two improvements to the v2 workspace Changes tab: (1) replaces letter-based status badges (A/M/D/R/U) with VSCode-style diff icons from Key changes:
Confidence Score: 5/5Safe to merge — changes are straightforward UI improvements and a well-structured event subscription addition with no logic errors or regressions. The debounce implementation is correct (stable ref, proper cleanup on unmount, shared across both event sources). The icon replacement is complete and type-safe (all FileStatus values are handled). The only finding is a cosmetic TypeScript signature mismatch that compiles and behaves correctly. No files require special attention.
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/ChangesFileList.tsx | Swaps letter badges for VSCode diff icons; icon/color mapping is complete for all FileStatus values; minor: STATUS_COLORS has no fallback for unexpected statuses, but type safety prevents this in practice. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/useChangesTab.tsx | Adds fs:events subscription + shared 300ms debounce; debounce implementation and cleanup are correct; debouncedInvalidate ignores the FsWatchEvent argument passed by useWorkspaceEvent, which is fine in JS/TS but slightly mismatches the declared signature. |
Sequence Diagram
sequenceDiagram
participant FS as File System
participant Git as Git
participant EB as EventBus
participant Hook as useChangesTab
participant Debounce as debounceRef (300ms)
participant TRPC as tRPC Cache
FS->>EB: fs:events (file edited)
EB->>Hook: useWorkspaceEvent("fs:events") fires debouncedInvalidate
Hook->>Debounce: clearTimeout + setTimeout(300ms)
Note over FS,Debounce: Rapid saves reset the timer each time
Git->>EB: git:changed (stage/commit)
EB->>Hook: useWorkspaceEvent("git:changed") fires debouncedInvalidate
Hook->>Debounce: clearTimeout + setTimeout(300ms)
Debounce->>TRPC: invalidate git.getStatus + git.listCommits
TRPC-->>Hook: re-fetches → UI updates
Reviews (1): Last reviewed commit: "debounce" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/useChangesTab.tsx (1)
246-250: Consider clearing the debounce timer when dependencies change.The cleanup effect has an empty dependency array, so it only runs on unmount. If
workspaceIdchanges while a debounce is pending, the stale timer could fire and invalidate queries for the old workspace. While this may be benign (just an extra invalidation), it's cleaner to clear the timer when dependencies change.♻️ Suggested improvement
useEffect(() => { return () => { if (debounceRef.current) clearTimeout(debounceRef.current); }; - }, []); + }, [workspaceId]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/useChangesTab.tsx around lines 246 - 250, The cleanup effect currently only runs on unmount; update the useEffect that references debounceRef so it also runs when workspaceId (and any other values that can change the pending debounce) changes: include workspaceId in the dependency array and keep the clearTimeout(debounceRef.current) call in the cleanup so any pending timer is cancelled when workspaceId changes, preventing stale timer callbacks from invalidating the old workspace's queries.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/useChangesTab.tsx:
- Around line 246-250: The cleanup effect currently only runs on unmount; update
the useEffect that references debounceRef so it also runs when workspaceId (and
any other values that can change the pending debounce) changes: include
workspaceId in the dependency array and keep the
clearTimeout(debounceRef.current) call in the cleanup so any pending timer is
cancelled when workspaceId changes, preventing stale timer callbacks from
invalidating the old workspace's queries.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4381e5b7-af48-4a07-a14c-0ce74614b29a
📒 Files selected for processing (2)
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/useChangesTab.tsx
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/useChangesTab.tsx (1)
246-250: Add comment to clarify intentionalworkspaceIddependency.Static analysis flags
workspaceIdas unnecessary since it's not referenced in the effect body. However, per the commit message, this is intentional: includingworkspaceIdensures the cleanup runs when switching workspaces, preventing stale timers from firing for the wrong workspace.Consider adding a comment or suppression to make this intent explicit:
📝 Suggested clarification
useEffect(() => { return () => { if (debounceRef.current) clearTimeout(debounceRef.current); }; + // eslint-disable-next-line react-hooks/exhaustive-deps -- Clear pending timer when workspaceId changes }, [workspaceId]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/useChangesTab.tsx around lines 246 - 250, The useEffect cleanup currently depends on workspaceId only to ensure pending timers are cleared when switching workspaces (debounceRef.current) but static analysis flags the dependency as unused; add an inline comment next to the useEffect (or above it) explaining that workspaceId is intentionally included to force cleanup when workspace changes, referencing debounceRef and workspaceId and noting it prevents stale timers from firing for the previous workspace, or add an ESLint suppression comment (e.g., // eslint-disable-next-line react-hooks/exhaustive-deps) with a short justification mentioning debounceRef and workspaceId so the intent is explicit.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/useChangesTab.tsx:
- Around line 246-250: The useEffect cleanup currently depends on workspaceId
only to ensure pending timers are cleared when switching workspaces
(debounceRef.current) but static analysis flags the dependency as unused; add an
inline comment next to the useEffect (or above it) explaining that workspaceId
is intentionally included to force cleanup when workspace changes, referencing
debounceRef and workspaceId and noting it prevents stale timers from firing for
the previous workspace, or add an ESLint suppression comment (e.g., //
eslint-disable-next-line react-hooks/exhaustive-deps) with a short justification
mentioning debounceRef and workspaceId so the intent is explicit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 83903300-856a-4c18-9e8f-223a8d08c1af
📒 Files selected for processing (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/useChangesTab.tsx
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/useChangesTab.tsx (1)
336-340: Consider adding a comment to clarify the intentionalworkspaceIddependency.The static analyzer flags
workspaceIdas unnecessary because the effect body doesn't reference it. However, including it is intentional—whenworkspaceIdchanges, the cleanup runs to clear any pending debounced invalidation for the old workspace. This is a valid pattern but not immediately obvious.A brief comment would silence linter confusion and clarify intent for future readers:
💡 Suggested comment
+ // Clear pending debounce on unmount or when workspaceId changes + // (workspaceId in deps is intentional to cancel stale invalidations) useEffect(() => { return () => { if (debounceRef.current) clearTimeout(debounceRef.current); }; }, [workspaceId]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/useChangesTab.tsx around lines 336 - 340, Add a brief inline comment above the useEffect that includes workspaceId in its dependency array to explain the intent: this effect's cleanup clears any pending timeout stored in debounceRef.current when the workspace changes, preventing old debounced invalidations from running for a new workspace; reference the useEffect containing debounceRef.current and the workspaceId dependency so reviewers and linters understand the inclusion is intentional.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/useChangesTab.tsx:
- Around line 336-340: Add a brief inline comment above the useEffect that
includes workspaceId in its dependency array to explain the intent: this
effect's cleanup clears any pending timeout stored in debounceRef.current when
the workspace changes, preventing old debounced invalidations from running for a
new workspace; reference the useEffect containing debounceRef.current and the
workspaceId dependency so reviewers and linters understand the inclusion is
intentional.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1e4b633b-fd4c-471e-be34-f425481e22e5
📒 Files selected for processing (2)
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/useChangesTab.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
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesHeader/ChangesHeader.tsx:
- Around line 79-85: The rename icon button currently rendered in the
ChangesHeader needs an accessible name: update the button element that calls
startEditing (the button wrapping the <Pencil /> icon) to include an appropriate
aria-label (e.g., "Rename workspace" or a dynamic label using the workspace
name) and add a matching title attribute for hover/tooltips; ensure the label
text is concise and specific to the action so screen readers can announce the
button meaningfully.
- Around line 68-73: The onBlur currently always calls handleSubmit which allows
Escape to be ignored and can double-submit after Enter; modify ChangesHeader to
distinguish submit vs cancel by adding a persistent ref flag (e.g.,
didSubmitRef/didCancelRef) and update it in the onKeyDown handler for Enter (set
didSubmitRef true and call handleSubmit) and Escape (set didCancelRef true and
call setIsEditing(false)); then change the onBlur handler to only call
handleSubmit when neither didSubmitRef nor didCancelRef is set, and reset those
refs when editing state opens/closes so subsequent edits behave correctly.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesTabContent/ChangesTabContent.tsx:
- Line 24: The uncommitted view is collapsing staged/unstaged semantics by
passing a single category into ChangesFileList; update ChangesTabContent so when
the top-level category is "uncommitted" you split the mixed rows into separate
staged and unstaged arrays (or compute a per-row fileCategory) and render
ChangesFileList for each subcategory (or pass the computed fileCategory per row
into the row renderer) instead of a single category prop—ensure the code paths
around the category prop, the fileCategory type, and the rendering logic in
ChangesFileList (and the rows handling in lines ~92-101) use the correct
per-file fileCategory so staged rows are reported and rendered as staged and
unstaged as unstaged.
🪄 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: 69bfa4b6-4738-49a9-9f31-3277fc31a4f2
📒 Files selected for processing (7)
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/ChangesHeader/ChangesHeader.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesHeader/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/components/ChangesTabContent/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/types.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/useChangesTab.tsx
✅ Files skipped from review due to trivial changes (2)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesHeader/index.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/types.ts
🚧 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
![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 7 files (changes from recent commits).
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/useChangesTab.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/useChangesTab.tsx:88">
P2: Include workspaceId in the cleanup effect dependencies so pending debounce timers are cleared when the workspace changes.</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesTabContent/ChangesTabContent.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesTabContent/ChangesTabContent.tsx:100">
P2: When viewing uncommitted changes, files from both staged and unstaged arrays are merged into `filteredFiles`, but a single `fileCategory` is passed to `ChangesFileList`. This means `onSelectFile` reports every file with the same category regardless of whether it's staged or unstaged, which will fetch the wrong diff for files in the other category. Consider passing per-file category information or splitting into staged/unstaged sections for the uncommitted view.</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesHeader/ChangesHeader.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesHeader/ChangesHeader.tsx:70">
P2: Pressing Escape triggers `setIsEditing(false)`, which unmounts the input and fires `onBlur` → `handleSubmit()`, so Escape actually submits the rename instead of canceling. Similarly, Enter causes a double submit (once directly, once via blur). Use a ref to skip blur-driven submission after Escape/Enter, and reset `editValue` on cancel.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
9 tasks
7 tasks
MocA-Love
pushed a commit
to MocA-Love/superset
that referenced
this pull request
Apr 10, 2026
* diff icons * Handle changes * debounce * Lint * fix: clear debounce timer on workspaceId change * Refactor * memoize * Lint * Update colors * clean up * Sidebar scroll * Unstaged and staged
14 tasks
3 tasks
This was referenced Apr 23, 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
VscDiffAdded,VscDiffModified, etc.) matching v1 stylingfs:eventssubscription so the changes tab updates on working-tree file edits (not just.gitdirectory changes)git:changedandfs:eventsto batch rapid events into one git status refreshTest plan
Summary by cubic
Updated the v2 Changes tab with VSCode‑style diff icons, theme diff colors, and faster, memoized rendering; live updates now debounce
git:changedandfs:eventswith safe cleanup. The Uncommitted view now splits files into Staged and Unstaged sections.New Features
Refactors
ChangesHeaderandChangesTabContent; memoized the file list, groups, and rows.git:changedandfs:eventswith cleanup on unmount/workspace change; deduped “All” files; branch rename only when no upstream; base‑branch selector popover now scrolls within a max height.Written for commit 6da13ea. Summary will update on new commits.
Summary by CodeRabbit
New Features
Style
Performance
Bug Fixes