[Kitenite]

Summary

Supersedes #3276 (credit @kkjcheng) — applies the same focus-trap fix to both v1 (NewWorkspaceModal) and v2 (DashboardNewWorkspaceModal) Dialogs on the sync-ws-v1-v2 branch.

The monorepo's Dialog wrapper (packages/ui/src/components/ui/dialog.tsx) overrides Radix's default and sets modal = false, so dialogs must opt in to get Radix's focus trap. Without modal, alt-tabbing away and back drops focus outside the dialog.

Changes

• Add modal prop to Dialog in NewWorkspaceModal.tsx (v1)
• Add modal prop to Dialog in DashboardNewWorkspaceModal.tsx (v2)
• Both files already had onFocusOutside={(e) => e.preventDefault()} from earlier work

Test plan

• Open the v1 new workspace modal, alt-tab away and back — typing lands in the prompt input
• Open the v2 dashboard new workspace modal, alt-tab away and back — typing lands in the prompt input
• Escape still closes both modals
• Overlay click still closes both modals

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 113a213a-d165-4471-b92a-a150b7608ec8

📥 Commits

Reviewing files that changed from the base of the PR and between 2367cae and 41aa135.

📒 Files selected for processing (2)

• apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx
• apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/DashboardNewWorkspaceModal.tsx

---

📝 Walkthrough

Walkthrough

Both workspace modal components now pass the modal prop to their Dialog components, switching them to modal dialog behavior while keeping existing open state and onOpenChange/close handlers unchanged.

Changes

Cohort / File(s)	Summary
Workspace Modal Dialogs apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx, apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/DashboardNewWorkspaceModal.tsx	Added modal prop to Dialog components, enabling modal behavior (blocking background interaction) while preserving existing open state and close handlers.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 I hopped in code to tweak a prop,

Dialogs now stand—no background stop.
Quiet clicks and focused view,
A tidy change from me to you.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
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.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: enabling the modal focus trap on both v1 and v2 workspace dialogs, which directly aligns with the changeset.
Description check	✅ Passed	The description covers the main changes, provides related issue context, includes a comprehensive test plan, and maintains the required template structure with all key sections addressed.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/desktop-workspace-modal-focus-trap-v1-v2

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[greptile-apps]

Greptile Summary

This PR applies the focus-trap fix from #3276 to both the v1 (NewWorkspaceModal) and v2 (DashboardNewWorkspaceModal) workspace dialogs by adding modal to the Dialog component and onFocusOutside={(e) => e.preventDefault()} to DialogContent. The fix is correctly implemented and symmetrically applied across both modals.

• modal on Dialog enables Radix UI's built-in focus trap, so alt-tab away/back correctly restores focus to the prompt input
• onFocusOutside prevents the focus-outside event from propagating (defensive against edge-case focus escapes)
• The overlay-click-to-close and Escape-to-close behaviors are unaffected (those go through onInteractOutside / onOpenChange)
• Both files are symmetric in their implementation, consistent with the pattern from fix(desktop): restore focus to new workspace modal after alt-tab #3276
• One structural observation: DialogHeader with DialogTitle/DialogDescription is rendered outside DialogContent (as a sibling, not a child). All 28 other dialog files in the codebase nest the header inside DialogContent. With modal={true} Radix aria-hides the main app container, which may affect how screen readers resolve the aria-labelledby target since the title element lives in the main DOM rather than the portal. This may be pre-existing from fix(desktop): restore focus to new workspace modal after alt-tab #3276 and is worth verifying.

Confidence Score: 4/5

Safe to merge — the focus-trap fix is correct and well-targeted; one pre-existing structural inconsistency worth cleaning up.

The core change (adding modal + onFocusOutside) is straightforward and correctly implements the stated fix, matching the reference from #3276. The only concern is the DialogHeader placed outside DialogContent, which is inconsistent with every other dialog in the codebase and could affect screen reader accessibility with modal={true}. Since this appears to originate from #3276 and is not a new regression in keyboard focus behavior (the primary user path), the PR is on the happy path to merge.

Both changed files share the same DialogHeader-outside-DialogContent structural pattern that should ideally be addressed.

Important Files Changed

Filename	Overview
apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx	Adds modal prop to Dialog and onFocusOutside handler to DialogContent to restore focus-trap behavior; DialogHeader with title/description is placed outside DialogContent (inconsistent with all other dialogs in the codebase)
apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/DashboardNewWorkspaceModal.tsx	Same modal/focus-trap fix applied as v1; same DialogHeader placement outside DialogContent; symmetrically implemented with the v1 modal

Sequence Diagram

sequenceDiagram
    participant User
    participant OS as OS/Window Manager
    participant Radix as Radix Dialog (modal=true)
    participant Input as Prompt Input

    User->>Radix: Open workspace modal
    Radix->>Input: Auto-focus prompt input
    User->>OS: Alt-Tab away (window blur)
    OS-->>Radix: Window loses focus
    User->>OS: Alt-Tab back (window focus)
    OS-->>Radix: Window regains focus
    Radix->>Input: Restore focus (focus trap active)
    Note over Radix,Input: onFocusOutside prevents<br/>focus escaping the modal

    User->>Radix: Click overlay
    Radix->>Radix: onOpenChange(false) → closeModal()

    User->>Radix: Press Escape
    Radix->>Radix: onOpenChange(false) → closeModal()

Loading

Comments Outside Diff (1)

1. apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx, line 77-82 (link)

   DialogHeader placed outside DialogContent

   DialogHeader (with DialogTitle and DialogDescription) is a direct child of Dialog (i.e., DialogPrimitive.Root) rather than being nested inside DialogContent. Every other dialog in this codebase—across all 28 files—places the header inside DialogContent. Examples: EditSecretDialog, RangeModal, DashboardSidebarDeleteDialog, etc.

   The practical risk with modal={true} is that DialogPrimitive.Root renders its children inline in the main app DOM tree, while DialogContent renders via a Portal into document.body. When modal={true} is active, Radix adds aria-hidden="true" to the main app container (the portal's sibling elements). This means the DialogTitle and DialogDescription elements—even though they're registered via Radix context and aria-labelledby is set on the content—reside in the aria-hidden portion of the DOM, which can lead to screen readers not announcing the dialog's accessible name correctly.

   The same pattern exists in DashboardNewWorkspaceModal.tsx at line 55.

   Consider moving the header inside DialogContent, matching the rest of the codebase:

_{Reviews (1): Last reviewed commit: "fix(desktop): enable modal focus trap on..." | Re-trigger Greptile}

[cubic-dev-ai]

No issues found across 2 files

[github-actions]

🧹 Preview Cleanup Complete

The following preview resources have been cleaned up:

• ⚠️ Neon database branch
• ⚠️ Electric Fly.io app

Thank you for your contribution! 🎉