sidebar-cards by AviPeltz · Pull Request #696 · superset-sh/superset

AviPeltz · 2026-01-09T22:01:02Z

Description

Related Issues

Type of Change

Testing

Screenshots (if applicable)

Additional Notes

Summary by CodeRabbit

Release Notes

New Features
- Local git changes now display in workspace list items
- Added branch switcher for workspace management
- Enhanced hover interactions with context menus for workspaces
Improvements
- Better UI spacing and layout in workspace sidebar
- Improved styling and readability of workspace status badges
- Unified diff stats display with fallback logic

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2026-01-09T22:01:14Z

📝 Walkthrough

Walkthrough

The PR enhances WorkspaceSidebar components with lazy-loaded local git changes, interactive close workflows for worktrees, and refined UI styling. It adds optional props to WorkspaceDiffStats for close actions and active states, integrates local diff statistics via TRPC queries, and updates component layout and styling.

Changes

Cohort / File(s)	Summary
Diff Stats & Close Workflow `apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceDiffStats.tsx`, `apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx`	WorkspaceDiffStats gains optional `onClose` and `isActive` props to support interactive close button on hover and active state styling. WorkspaceListItem now lazy-loads local git diffs via TRPC on hover, computes unified diffStats (prioritizing local over PR-based), and wires close callback for worktree deletion. UI structure refactored with richer hover interactions (HoverCard for worktrees, Tooltip for branches), inline rename input, and close/delete workflow.
UI Styling & Polish `apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectSection.tsx`, `apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceStatusBadge.tsx`	Minor spacing adjustments in ProjectSection (bottom/top padding updates). WorkspaceStatusBadge refinements: badge border-radius changed to `rounded-md`, text size reduced to `text-[10px]`, PR number formatted with `tabular-nums`.

Sequence Diagram

sequenceDiagram
    actor User
    participant UI as WorkspaceListItem
    participant TRPC as TRPC Query
    participant Server as Backend
    participant UI2 as WorkspaceDiffStats

    User->>UI: Hover over worktree item
    activate UI
    UI->>TRPC: Query worktree diffs (lazy-load)
    activate TRPC
    TRPC->>Server: Fetch staged/unstaged/untracked diffs
    Server-->>TRPC: Return diff stats
    deactivate TRPC
    TRPC-->>UI: diffStats (additions/deletions)
    UI->>UI2: Render with onClose & diffStats
    activate UI2
    UI2-->>User: Display diff stats + close button on hover
    deactivate UI2
    User->>UI2: Click close button
    UI2->>UI: Trigger onClose callback
    UI->>Server: Delete/close worktree
    deactivate UI

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly Related PRs

feature(desktop): manage worktrees better #407 — Adds TRPC endpoints for worktree open/close operations that this PR's lazy-loading and close workflow directly depend upon.
feature (desktop): sidebar style and close functionality #618 — Implements close/delete worktree UI in WorkspaceListItem sidebar with hover context menu, directly aligned with this PR's close button and workflow enhancements.

Poem

🐰 Behold, the sidebar springs to life with flair,
Local diffs now hover in the air,
Close buttons bloom on worktrees with a gentle click,
Spacing refined, styling so slick—
The workspace awaits, responsive and bright! ✨

🚥 Pre-merge checks | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The PR description consists entirely of an empty template with no substantive content, making it impossible to understand the rationale, context, or implications of the changes.	Fill in all relevant template sections with concrete details about what changed, why, related issues, type of change, testing performed, and any additional context.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title 'sidebar-cards' is a vague, generic term that does not clearly describe the main changes in the changeset, which include workspace diff stats enhancements, hover-based git change loading, and UI restructuring.	Revise the title to be more descriptive of the primary change, such as 'Add lazy-loaded diff stats and improve workspace sidebar interactions' or 'Enhance WorkspaceListItem with local git changes and hover interactions'.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

📝 Generate docstrings

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (4)

apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceDiffStats.tsx (1)
19-23: Consider extracting conditional styles for readability.

The container has cursor-pointer but lacks an onClick handler—only the nested button is clickable. This may confuse users expecting the entire area to be interactive.
♻️ Suggested improvement
 		<div
 			className={cn(
-				"group/diff flex items-center text-[10px] font-mono tabular-nums px-1.5 py-0.5 rounded relative cursor-pointer",
-				isActive ? "bg-foreground/10 group-hover:bg-transparent" : "bg-muted/50 group-hover:bg-transparent",
+				"group/diff flex items-center text-[10px] font-mono tabular-nums px-1.5 py-0.5 rounded relative",
+				isActive
+					? "bg-foreground/10 group-hover:bg-transparent"
+					: "bg-muted/50 group-hover:bg-transparent",
 			)}
 		>
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx (3)
115-122: Consider renaming the stale time constant for clarity.

Using GITHUB_STATUS_STALE_TIME for local git changes may be misleading. Consider extracting a more generic constant or renaming to reflect broader usage.
// In constants.ts, consider:
export const STATUS_STALE_TIME = 30_000; // or whatever value
// Then use STATUS_STALE_TIME for both queries
215-217: Refactor the long conditional for readability.

The current line is dense and hard to parse quickly.
♻️ Suggested refactor
-	// Show diff stats from PR if available, otherwise from local changes
-	const diffStats = localDiffStats || (pr && (pr.additions > 0 || pr.deletions > 0) ? { additions: pr.additions, deletions: pr.deletions } : null);
+	// Show diff stats from local changes if available, otherwise from PR
+	const prDiffStats = pr && (pr.additions > 0 || pr.deletions > 0)
+		? { additions: pr.additions, deletions: pr.deletions }
+		: null;
+	const diffStats = localDiffStats ?? prDiffStats;
Note: The comment also appears inverted ("from PR if available, otherwise from local") when the code prioritizes local first.
447-478: Simplify redundant conditional check.

showDiffStats is defined as !!diffStats (line 217), so showDiffStats && diffStats is redundant—if showDiffStats is true, diffStats is guaranteed to be truthy.
♻️ Suggested simplification
 								{/* Diff stats (transforms to X on hover) or close button for worktree workspaces */}
 								{!isBranchWorkspace && (
-									showDiffStats && diffStats ? (
+									diffStats ? (
 										<WorkspaceDiffStats
 											additions={diffStats.additions}
 											deletions={diffStats.deletions}

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0a83467 and ac909f8.

📒 Files selected for processing (4)

apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectSection.tsx
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceDiffStats.tsx
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceStatusBadge.tsx

🧰 Additional context used

📓 Path-based instructions (6)

apps/desktop/**/*.{ts,tsx}

📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)

apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined in src/lib/trpc
Use alias as defined in tsconfig.json when possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from @trpc/server/observable instead of async generators, as the library explicitly checks isObservable(result) and throws an error otherwise

Files:

apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectSection.tsx
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceStatusBadge.tsx
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceDiffStats.tsx
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx

**/*.{ts,tsx}