Repository navigation
feat(desktop): close settings with Escape key - #3466
Conversation
Press Escape on any settings page to navigate back to where settings was opened from. Skips when focus is in a form field, contenteditable, or any open Radix layer (dialog, popover, dropdown).
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdded Escape key handling to the settings layout component. When pressed, it checks for open UI elements; if none exist, it navigates back to the origin route using the newly introduced Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
✨ 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 |
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
Greptile SummaryThis PR adds an Escape key shortcut to close the settings layout in the desktop app by navigating back to Key changes:
Confidence Score: 5/5Safe to merge — single focused change, correct guard conditions, consistent with existing codebase patterns. The implementation is a clean, minimal addition that mirrors existing hotkey patterns in the codebase ( No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant U as User
participant HK as useHotkeys (Escape)
participant DOM as document.querySelector
participant R as Radix Overlay
participant Nav as TanStack navigate
U->>HK: Press Escape (focus NOT in form/contenteditable)
HK->>DOM: querySelector('[data-state="open"]')
alt Radix overlay is open
DOM-->>HK: element found
HK->>R: return early — overlay handles Escape natively
R->>R: closes dialog/popover/dropdown
else No overlay open
DOM-->>HK: null
HK->>HK: event.preventDefault()
HK->>Nav: navigate({ to: originRoute })
Nav-->>U: navigates back to origin route
end
Note over U,HK: Escape inside input/textarea/select/contenteditable<br/>is never captured (enableOnFormTags: false)
Reviews (1): Last reviewed commit: "feat(desktop): close settings with Escap..." | Re-trigger Greptile |
Press Escape on any settings page to navigate back to where settings was opened from. Skips when focus is in a form field, contenteditable, or any open Radix layer (dialog, popover, dropdown).
Two adjustments on top of the cherry-picked upstream commits: 1. apps/desktop/src/main/index.ts — upstream ParadevAI#3461 uses getHostServiceCoordinator() at the dev-reload call site, but fork imports that symbol as getHostServiceManager to minimize diff with the quit-lifecycle code that still uses the legacy name. Rewrite the call to use the local alias so the desktop main process still builds. 2. apps/desktop/src/renderer/routes/_authenticated/settings/layout.tsx — upstream ParadevAI#3466 uses [data-state="open"] to suppress Escape when a Radix overlay is open, but that selector also matches Radix Collapsible (settings/agents AgentCard uses Collapsible with data-state="open"), so expanding any card silently disabled the new Escape-to-close behavior. Narrow the selector to role-based overlay elements only (dialog, alertdialog, menu, listbox).
…ndle chore(upstream): PR1 低リスク修正5件バンドル (ParadevAI#3457 ParadevAI#3464 ParadevAI#3462 ParadevAI#3466 ParadevAI#3461)
-s ours merge to record that upstream commits a3e34bf through de70163 (13 commits) are semantically already present on origin/main via the PR1-6 cherry-pick series (PRs #176, #177, #178, #179, #180, #182), plus fork-adaptation fixes layered on top. This merge target is de70163 specifically (not upstream/main) so newer upstream commits (9fff075 and later) remain visible in future behind counts. Upstream commits covered by this audit merge: - a3e34bf fix(desktop): restore cmd+click requirement for v1 terminal file links (ParadevAI#3457) [PR1/#176] - 57557f8 fix(desktop): gate v2 workspace children on collection readiness (ParadevAI#3464) [PR1/#176] - 4ee2e61 fix(desktop): use native clipboard for copy path in v2 sidebar (ParadevAI#3462) [PR1/#176] - 87d6e93 feat(desktop): close settings with Escape key (ParadevAI#3466) [PR1/#176] - 9c7f5f4 chore(desktop): auto-restart host-service on bundle change in dev (ParadevAI#3461) [PR1/#176] - 93140d9 fix(mcp): accept MCP resource URL as valid OAuth audience (ParadevAI#3459) [PR2/#177] - be9e000 fix(desktop): drive tray menu off events, fetch real org name (ParadevAI#3458) [PR2/#177] - c5f791e feat(v2): unify workspace delete through host-service (ParadevAI#3443) [PR3/#178] - 2c24d93 feat(desktop): paginated branch picker with checkout + open actions (ParadevAI#3397) [PR4/#179] - 2bf1049 feat(desktop/hotkeys): v1 directional pane focus + best-effort v1 override migrator (ParadevAI#3460) [PR5/#180] - 1294a7d feat(desktop/hotkeys): restore Cmd+Alt+Arrow for tab/workspace nav (ParadevAI#3472) [PR5/#180] - de70163 feat(desktop): v2 review tab first pass — PR info, checks, comments (ParadevAI#3463) [PR6/#182] Intentionally skipped (version bump, fork has independent versioning): - 1e23353 chore(desktop): bump version to 1.5.5 (ParadevAI#3473) Fork-adaptation fixes layered on top of the cherry-picks: - PR1: host-service-coordinator alias import fix, settings Escape selector narrowing (role-based + popper wrapper), Escape close uses replace navigation - PR2: dual quit mode preservation (requestQuit "release"/"stop"), trayUpdateToken guard for stale async fetchHostInfo results - PR4: ChangesHeader.normalizeBranchName regex rewrite (lint false positive), worktree add uses fullRef for remote-tracking refs, syncTimedOut reset on pendingId change, GIT_REFS.md barrel example fix - PR5: migrate.ts re-sanitize of existing localStorage overrides (v2 marker bump intent), FOCUS_PANE_* enabled:isActive for KeepAliveWorkspaces, CATEGORY_ORDER merges Navigation (upstream) and Browser (fork) - PR6: normalizeThreadsToComments flattens all thread.comments (not just first), CommentPane overrides <a> (openUrl) and <img> (SafeImage), zero-badge suppression, pr-null comments gate Fork features verified intact (Explore agent audit of combined 36d4de4..35d95f3 range): - BROWSER_RELOAD / BROWSER_HARD_RELOAD hotkeys - dual quit mode menu in tray - v1 terminal cold-restore + retry reconnect (out of range but unaffected) - KeepAliveWorkspaces (FOCUS_PANE_* gated on isActive) - useCommandPalette + addMemoTab in v2 workspace - host-service-coordinator rename alias pattern
Summary
originRoute).react-hotkeys-hookwithenableOnFormTags: falseandenableOnContentEditable: false, so Escape inside inputs, textareas, selects, and contenteditable fields is left to the browser/component.[data-state="open"]), so Escape-to-close on dialogs/popovers/dropdowns/sheets keeps working as expected.Test plan
Summary by cubic
Close Settings with Escape in the desktop app. Pressing Escape on any Settings page returns you to where Settings was opened.
react-hotkeys-hookwith form and contenteditable inputs excluded, so Escape inside fields is not intercepted.Written for commit 6206829. Summary will update on new commits.
Summary by CodeRabbit