Skip to content

feat(desktop): cleaner workspace creation modal#481

Merged
AviPeltz merged 4 commits intomainfrom
open-worspace-modal-update
Dec 24, 2025
Merged

feat(desktop): cleaner workspace creation modal#481
AviPeltz merged 4 commits intomainfrom
open-worspace-modal-update

Conversation

@AviPeltz
Copy link
Copy Markdown
Collaborator

@AviPeltz AviPeltz commented Dec 23, 2025

Description

Related Issues

Type of Change

  • Bug fix
  • New feature
  • Documentation
  • Refactor
  • Other (please describe):

Testing

Screenshots (if applicable)

Additional Notes

Summary by CodeRabbit

  • New Features
    • Renamed modal flow to "New Workspace"
    • Added collapsible "Advanced" options for branch configuration
    • Title input now auto-generates a real-time branch preview; branch editable in Advanced
    • Enter key creates workspace when in New mode; improved focus-on-open behavior
    • Added "Open All" action to open multiple closed worktrees with progress feedback

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

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Dec 23, 2025

📝 Walkthrough

Walkthrough

The NewWorkspaceModal UI was reworked: modal title and layout changed, an Input-driven title with live branch preview and an Advanced collapsible for editable branch name were added, focus and Enter-to-submit behavior were introduced, and the create action moved into the dialog content. ExistingWorktreesList gained an "Open All" action to open multiple worktrees with toasts.

Changes

Cohort / File(s) Summary
NewWorkspace modal UI
apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx
Reworked dialog structure: removed DialogFooter, moved Create button into DialogContent, renamed title to "New Workspace", added title Input with real-time branch preview, introduced Collapsible-based Advanced options containing an editable branch-name input, replaced some Popover interactions with the new structure, added keyboard Enter-to-create handler, focus-on-open for title input, added showAdvanced state and titleInputRef, updated imports (Collapsible components, additional HiIcons, useRef/useEffect).
Existing worktrees batch action
apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/ExistingWorktreesList.tsx
Added "Open All" button and handler to iterate and open closed worktrees, with loading/success/error toasts and disable state while opening; imports for Button and toast logic updated; onOpenSuccess invoked after completion.

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant Modal as NewWorkspaceModal (UI)
  participant Collapsible
  participant Backend
  participant Toasts

  Note over User,Modal: User opens modal
  User->>Modal: open
  Modal->>Modal: focus title input (if project selected)
  User->>Modal: type title
  Modal->>Modal: update branch preview
  User->>Collapsible: expand Advanced (optional)
  Collapsible-->>Modal: user edits branch name (optional)
  User->>Modal: click "Create Workspace" or press Enter
  Modal->>Backend: create workspace (title, branch)
  Backend-->>Modal: success / error
  Modal->>Toasts: show loading → success/error
  Toasts-->>User: notify result
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • #461: Modifies NewWorkspaceModal branch/base-branch handling and selection UI — strong overlap with branch-selection changes.
  • #455: Alters how NewWorkspaceModal is opened and integrated in the main UI, affecting modal usage and flow.
  • #391: Changes branch-name input behavior and creation logic in NewWorkspaceModal, directly overlapping implementation.

"I hopped into code with a twitch and a twitchy paw,
Folded branches like clover and smoothed every flaw.
Title and branch now dance in a neat, tiny waltz,
Advanced options hidden—then shown at a pulse.
🐇🍀 New workspaces bloom where once was a brawl."

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description check ⚠️ Warning The description follows the required template structure but lacks critical implementation details: the Description section is empty, Related Issues is blank, Testing steps are missing, and no screenshots or additional context are provided. Fill in the Description section with details about the workspace modal improvements, add testing steps to verify the new UI functionality, and optionally include screenshots and related issue links.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: improving the workspace creation modal UI with a cleaner interface.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch open-worspace-modal-update

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.

@AviPeltz AviPeltz marked this pull request as ready for review December 23, 2025 18:43
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (2)
apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx (2)

280-306: Consider adding ARIA attributes to the mode switcher.

The mode switcher tabs use plain <button> elements. For improved accessibility, consider using role="tablist" on the container and role="tab", aria-selected, and aria-controls on the buttons to properly convey the tab-like semantics to assistive technologies.


143-152: Focus timing with setTimeout is fragile.

Using a 50ms delay for focus can be unreliable across different browsers and hardware. Consider using autoFocus prop on the Input, or leveraging Radix Dialog's built-in focus management via onOpenAutoFocus callback if available.

🔎 Alternative approach
-	// Focus title input when modal opens and project is selected
-	useEffect(() => {
-		if (isOpen && selectedProjectId && mode === "new") {
-			// Small delay to ensure dialog is fully rendered
-			const timer = setTimeout(() => {
-				titleInputRef.current?.focus();
-			}, 50);
-			return () => clearTimeout(timer);
-		}
-	}, [isOpen, selectedProjectId, mode]);

Then on DialogContent:

<DialogContent
  onOpenAutoFocus={(e) => {
    if (selectedProjectId && mode === "new") {
      e.preventDefault();
      titleInputRef.current?.focus();
    }
  }}
  ...
>
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0b5c259 and 9316415.

📒 Files selected for processing (1)
  • apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Avoid using any type in TypeScript - maintain type safety unless absolutely necessary

Files:

  • apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

Run Biome for formatting, linting, import organization, and safe fixes at the root level using bun run lint:fix

Files:

  • apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx
**/{components,features}/**/[!.]*.tsx

📄 CodeRabbit inference engine (AGENTS.md)

Organize project structure with one folder per component: ComponentName/ComponentName.tsx with index.ts barrel export

Files:

  • apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx
**/{components,features}/**/*.{ts,tsx,test.ts,test.tsx,stories.tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Co-locate component dependencies (utils, hooks, constants, config, tests, stories) next to the file using them

Files:

  • apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Never import Node.js modules in renderer process or shared code - use only in main process (src/main/)

Files:

  • apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx
apps/desktop/src/{main,renderer,preload}/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use type-safe IPC communication - define channel types in apps/desktop/src/shared/ipc-channels.ts before implementing handlers

Files:

  • apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx
**/*.{tsx,css}

📄 CodeRabbit inference engine (AGENTS.md)

Use React + TailwindCSS v4 + shadcn/ui for UI development

Files:

  • apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx
**/{components,features}/**/*.tsx

📄 CodeRabbit inference engine (AGENTS.md)

**/{components,features}/**/*.tsx: Nest components in parent's components/ folder if used only once, promote to highest shared parent's components/ if used 2+ times
Use one component per file - do not combine multiple components in a single file

Files:

  • apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx
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/components/NewWorkspaceModal/NewWorkspaceModal.tsx
🧠 Learnings (1)
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Applies to **/*.{tsx,css} : Use React + TailwindCSS v4 + shadcn/ui for UI development

Applied to files:

  • apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx
🔇 Additional comments (1)
apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx (1)

1-491: LGTM overall — clean restructuring of the modal.

The refactor introduces a cleaner UX with collapsible advanced options, improved focus management, and Enter-to-create functionality. The code follows the project guidelines (tRPC for IPC, zustand stores, shadcn/ui components, no Node.js imports in renderer). Address the Enter key handling issue before merging.

Comment on lines +154 to 164
const handleKeyDown = (e: React.KeyboardEvent) => {
if (
e.key === "Enter" &&
!e.shiftKey &&
mode === "new" &&
selectedProjectId
) {
e.preventDefault();
handleCreateWorkspace();
}
};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Enter key handler can trigger unintended workspace creation.

The handleKeyDown is attached to DialogContent, so pressing Enter anywhere in the dialog (e.g., while selecting a branch in the popover) will trigger handleCreateWorkspace. The condition doesn't check if the branch popover is open (baseBranchOpen), leading to accidental creation when the user intends to confirm a branch selection.

Consider either:

  1. Adding !baseBranchOpen to the condition, or
  2. Attaching the handler directly to the title input instead of the dialog
🔎 Suggested fix (option 1)
 const handleKeyDown = (e: React.KeyboardEvent) => {
   if (
     e.key === "Enter" &&
     !e.shiftKey &&
     mode === "new" &&
-    selectedProjectId
+    selectedProjectId &&
+    !baseBranchOpen
   ) {
     e.preventDefault();
     handleCreateWorkspace();
   }
 };

Also applies to: 233-236

🤖 Prompt for AI Agents
In apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx
around lines 154 to 164 (also applies to lines 233 to 236), the Enter key
handler is attached to the DialogContent and doesn't check whether the branch
popover is open, so pressing Enter while interacting with the branch selector
can inadvertently call handleCreateWorkspace; update the condition to also
require the branch popover be closed (i.e., add a !baseBranchOpen check) OR move
the keydown handler from DialogContent to the title input element so Enter only
triggers creation when focus is on the title input.

Comment on lines +322 to +333
{/* Branch preview */}
{title && !showAdvanced && (
<p className="text-xs text-muted-foreground flex items-center gap-1.5">
<GoGitBranch className="size-3" />
<span className="font-mono">
{branchName || generateBranchFromTitle(title)}
</span>
<span className="text-muted-foreground/60">
(optional)
from {effectiveBaseBranch}
</span>
</label>
<Input
id="title"
className="h-8 text-sm"
placeholder="Feature name"
value={title}
onChange={(e) => setTitle(e.target.value)}
/>
</div>

<div className="space-y-1.5">
<label
htmlFor="branch"
className="text-xs text-muted-foreground"
>
Branch Name
</label>
<Input
id="branch"
className="h-8 text-sm font-mono"
placeholder={
title
? generateBranchFromTitle(title)
: "auto-generated"
}
value={branchName}
onChange={(e) => handleBranchNameChange(e.target.value)}
/>
</div>
</p>
)}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Handle null effectiveBaseBranch in preview.

When branchData is still loading or has no default branch, effectiveBaseBranch will be null, rendering "from null" in the UI.

🔎 Suggested fix
-									{title && !showAdvanced && (
+									{title && !showAdvanced && effectiveBaseBranch && (
 										<p className="text-xs text-muted-foreground flex items-center gap-1.5">

Or display a fallback text when the base branch is unknown.

🤖 Prompt for AI Agents
In apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx
around lines 322 to 333, the branch preview renders "from null" when
effectiveBaseBranch is null; update the JSX to handle null/undefined by showing
a fallback string (e.g., "unknown base" or "loading…") or hide the "from …"
segment when effectiveBaseBranch is falsy so the UI never displays "null".

@AviPeltz AviPeltz merged commit 872ee50 into main Dec 24, 2025
4 of 5 checks passed
@github-actions
Copy link
Copy Markdown
Contributor

🧹 Preview Cleanup Complete

The following preview resources have been cleaned up:

Service Status
Neon Database (Neon)

Thank you for your contribution! 🎉


Preview resources have been processed for cleanup

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (2)
apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx (2)

154-164: Enter key handler can trigger unintended workspace creation.

The handleKeyDown is attached to DialogContent (line 235), so pressing Enter anywhere in the dialog—including while selecting a branch in the popover—will trigger handleCreateWorkspace. The condition doesn't check if the branch popover is open (baseBranchOpen), leading to accidental creation when the user intends to confirm a branch selection.

Consider either:

  1. Adding !baseBranchOpen to the condition, or
  2. Attaching the handler directly to the title input instead of the dialog
🔎 Suggested fix (option 1)
 const handleKeyDown = (e: React.KeyboardEvent) => {
   if (
     e.key === "Enter" &&
     !e.shiftKey &&
     mode === "new" &&
-    selectedProjectId
+    selectedProjectId &&
+    !baseBranchOpen
   ) {
     e.preventDefault();
     handleCreateWorkspace();
   }
 };

318-328: Handle null effectiveBaseBranch in preview.

When branchData is still loading or has no default branch, effectiveBaseBranch will be null (line 117), rendering "from null" in the UI.

🔎 Suggested fix
-									{title && !showAdvanced && (
+									{title && !showAdvanced && effectiveBaseBranch && (
 										<p className="text-xs text-muted-foreground flex items-center gap-1.5">

Or display a fallback text when the base branch is unknown, such as "loading…" or "default".

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9316415 and 4c3a1a8.

📒 Files selected for processing (2)
  • apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx
  • apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/ExistingWorktreesList.tsx
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Avoid using any type in TypeScript - maintain type safety unless absolutely necessary

Files:

  • apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/ExistingWorktreesList.tsx
  • apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

Run Biome for formatting, linting, import organization, and safe fixes at the root level using bun run lint:fix

Files:

  • apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/ExistingWorktreesList.tsx
  • apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx
**/{components,features}/**/[!.]*.tsx

📄 CodeRabbit inference engine (AGENTS.md)

Organize project structure with one folder per component: ComponentName/ComponentName.tsx with index.ts barrel export

Files:

  • apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/ExistingWorktreesList.tsx
  • apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx
**/{components,features}/**/*.{ts,tsx,test.ts,test.tsx,stories.tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Co-locate component dependencies (utils, hooks, constants, config, tests, stories) next to the file using them

Files:

  • apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/ExistingWorktreesList.tsx
  • apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Never import Node.js modules in renderer process or shared code - use only in main process (src/main/)

Files:

  • apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/ExistingWorktreesList.tsx
  • apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx
apps/desktop/src/{main,renderer,preload}/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use type-safe IPC communication - define channel types in apps/desktop/src/shared/ipc-channels.ts before implementing handlers

Files:

  • apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/ExistingWorktreesList.tsx
  • apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx
**/*.{tsx,css}

📄 CodeRabbit inference engine (AGENTS.md)

Use React + TailwindCSS v4 + shadcn/ui for UI development

Files:

  • apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/ExistingWorktreesList.tsx
  • apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx
**/{components,features}/**/*.tsx

📄 CodeRabbit inference engine (AGENTS.md)

**/{components,features}/**/*.tsx: Nest components in parent's components/ folder if used only once, promote to highest shared parent's components/ if used 2+ times
Use one component per file - do not combine multiple components in a single file

Files:

  • apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/ExistingWorktreesList.tsx
  • apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx
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/components/NewWorkspaceModal/components/ExistingWorktreesList/ExistingWorktreesList.tsx
  • apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx
🧠 Learnings (1)
📚 Learning: 2025-12-18T23:19:10.415Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-18T23:19:10.415Z
Learning: Applies to **/*.{tsx,css} : Use React + TailwindCSS v4 + shadcn/ui for UI development

Applied to files:

  • apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx
🧬 Code graph analysis (1)
apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx (7)
packages/ui/src/components/ui/dialog.tsx (1)
  • DialogContent (136-136)
packages/ui/src/components/ui/input.tsx (1)
  • Input (29-29)
packages/ui/src/components/ui/collapsible.tsx (3)
  • Collapsible (33-33)
  • CollapsibleTrigger (33-33)
  • CollapsibleContent (33-33)
packages/ui/src/components/ui/button.tsx (1)
  • Button (60-60)
packages/ui/src/components/ui/command.tsx (5)
  • Command (175-175)
  • CommandInput (177-177)
  • CommandList (178-178)
  • CommandEmpty (179-179)
  • CommandItem (181-181)
apps/desktop/src/renderer/lib/formatRelativeTime/formatRelativeTime.ts (1)
  • formatRelativeTime (1-14)
apps/desktop/src/renderer/lib/formatRelativeTime/index.ts (1)
  • formatRelativeTime (1-1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Build
🔇 Additional comments (8)
apps/desktop/src/renderer/components/NewWorkspaceModal/components/ExistingWorktreesList/ExistingWorktreesList.tsx (1)

90-102: Add local pending state tracking for batch operations to prevent button flickering.

During handleOpenAll, the button repeatedly flickers between enabled and disabled states because openWorktree.isPending toggles after each worktree completes in the sequential loop. Use a local pending state to track the entire batch operation instead of relying on the mutation hook's isPending.

apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx (7)

1-50: LGTM! Clean imports with proper sources.

All imports follow the project structure correctly, with no Node.js modules in renderer code and appropriate use of internal UI components.


80-81: LGTM! Proper state and ref initialization.

The useRef<HTMLInputElement>(null) is correctly typed for React 19, and local form state with useState is appropriate for this modal component.


143-152: LGTM! Proper focus management with cleanup.

The auto-focus effect is justified for UX and correctly handles the async dialog rendering with appropriate cleanup. The effect will properly trigger when switching between modes.


132-141: LGTM! Complete form reset.

The resetForm function properly includes the new showAdvanced state in its reset logic.


309-316: LGTM! Well-implemented title input with focus management.

The title input properly uses the ref for focus management and provides clear placeholder text indicating the Enter key behavior.


330-455: LGTM! Well-structured Advanced options with excellent UX.

The Collapsible implementation is clean and the base branch selector handles all states properly:

  • Error and loading states covered
  • Manual filtering with shouldFilter={false} is correct for client-side search
  • Branch search cleared on selection
  • Accessibility labels present
  • Clear visual indicators (default badge, check icon)

457-463: LGTM! Proper button placement and state handling.

The Create Workspace button is correctly positioned in the content area with appropriate disabled states. The optional title/branch name is intentional based on the backend API (lines 179-180).

Comment on lines +40 to +60
const handleOpenAll = async () => {
if (closedWorktrees.length === 0) return;

const count = closedWorktrees.length;
toast.promise(
(async () => {
for (const wt of closedWorktrees) {
await openWorktree.mutateAsync({ worktreeId: wt.id });
}
})(),
{
loading: `Opening ${count} workspaces...`,
success: () => {
onOpenSuccess();
return `Opened ${count} workspaces`;
},
error: (err) =>
err instanceof Error ? err.message : "Failed to open workspaces",
},
);
};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Parallelize worktree opening and improve error handling.

The current implementation opens worktrees sequentially, which can be slow for multiple worktrees. Additionally, if any single worktree fails to open, the entire operation stops and subsequent worktrees are never opened.

Consider these improvements:

  1. Use Promise.allSettled() to open all worktrees in parallel
  2. Provide granular feedback for each success/failure
  3. Continue opening remaining worktrees even if one fails
🔎 Recommended refactor for parallel execution with granular error handling
 const handleOpenAll = async () => {
 	if (closedWorktrees.length === 0) return;
 
-	const count = closedWorktrees.length;
-	toast.promise(
-		(async () => {
-			for (const wt of closedWorktrees) {
-				await openWorktree.mutateAsync({ worktreeId: wt.id });
-			}
-		})(),
-		{
-			loading: `Opening ${count} workspaces...`,
-			success: () => {
-				onOpenSuccess();
-				return `Opened ${count} workspaces`;
-			},
-			error: (err) =>
-				err instanceof Error ? err.message : "Failed to open workspaces",
-		},
-	);
+	const openPromises = closedWorktrees.map((wt) =>
+		openWorktree
+			.mutateAsync({ worktreeId: wt.id })
+			.then(() => ({ status: 'success' as const, branch: wt.branch }))
+			.catch((err) => ({
+				status: 'error' as const,
+				branch: wt.branch,
+				error: err instanceof Error ? err.message : 'Unknown error',
+			})),
+	);
+
+	const toastId = toast.loading(`Opening ${closedWorktrees.length} workspaces...`);
+	const results = await Promise.all(openPromises);
+
+	const succeeded = results.filter((r) => r.status === 'success');
+	const failed = results.filter((r) => r.status === 'error');
+
+	if (failed.length === 0) {
+		toast.success(`Opened ${succeeded.length} workspaces`, { id: toastId });
+		onOpenSuccess();
+	} else if (succeeded.length === 0) {
+		toast.error(`Failed to open all workspaces`, { id: toastId });
+	} else {
+		toast.warning(
+			`Opened ${succeeded.length} of ${closedWorktrees.length} workspaces`,
+			{ id: toastId }
+		);
+		onOpenSuccess();
+	}
+
+	// Optionally show individual errors
+	for (const failure of failed) {
+		toast.error(`Failed to open ${failure.branch}: ${failure.error}`);
+	}
 };
📝 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 handleOpenAll = async () => {
if (closedWorktrees.length === 0) return;
const count = closedWorktrees.length;
toast.promise(
(async () => {
for (const wt of closedWorktrees) {
await openWorktree.mutateAsync({ worktreeId: wt.id });
}
})(),
{
loading: `Opening ${count} workspaces...`,
success: () => {
onOpenSuccess();
return `Opened ${count} workspaces`;
},
error: (err) =>
err instanceof Error ? err.message : "Failed to open workspaces",
},
);
};
const handleOpenAll = async () => {
if (closedWorktrees.length === 0) return;
const openPromises = closedWorktrees.map((wt) =>
openWorktree
.mutateAsync({ worktreeId: wt.id })
.then(() => ({ status: 'success' as const, branch: wt.branch }))
.catch((err) => ({
status: 'error' as const,
branch: wt.branch,
error: err instanceof Error ? err.message : 'Unknown error',
})),
);
const toastId = toast.loading(`Opening ${closedWorktrees.length} workspaces...`);
const results = await Promise.all(openPromises);
const succeeded = results.filter((r) => r.status === 'success');
const failed = results.filter((r) => r.status === 'error');
if (failed.length === 0) {
toast.success(`Opened ${succeeded.length} workspaces`, { id: toastId });
onOpenSuccess();
} else if (succeeded.length === 0) {
toast.error(`Failed to open all workspaces`, { id: toastId });
} else {
toast.warning(
`Opened ${succeeded.length} of ${closedWorktrees.length} workspaces`,
{ id: toastId }
);
onOpenSuccess();
}
// Optionally show individual errors
for (const failure of failed) {
toast.error(`Failed to open ${failure.branch}: ${failure.error}`);
}
};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant