Repository navigation
keep terminal tabs rendered - #147
Conversation
WalkthroughTerminal-manager now spawns shell processes as login shells by computing platform-specific arguments (zsh/bash use "-l") and passing them to pty.spawn. TabsContent refactors tab rendering to compute all workspace tabs within a memo, determine the active tab's display hierarchy, and render all workspace tabs while hiding non-active ones to preserve terminal scrollback. Changes
Sequence Diagram(s)sequenceDiagram
participant Spawner as Terminal Spawner
participant PTY as pty.spawn
participant Shell as Shell Process
rect rgb(200, 220, 255)
note over Spawner,Shell: New Login Shell Flow
Spawner->>Spawner: Detect shell type (zsh/bash/other)
Spawner->>Spawner: Compute shellArgs (["-l"] or [])
Spawner->>PTY: spawn(shell, shellArgs, options)
PTY->>Shell: Execute shell with args
Shell->>Shell: Load profile scripts (pyenv, nvm, etc.)
Shell->>Shell: Load rc files
end
sequenceDiagram
participant Render as TabsContent Render
participant Memo as useMemo
participant Logic as Tab Logic
participant DOM as DOM Render
rect rgb(220, 240, 220)
note over Render,DOM: New Tab Rendering Flow
Render->>Memo: Compute tabToRender + workspaceTabs
Memo->>Logic: Collect all workspace tabs for active workspace
Logic->>Logic: Find active tab, resolve parent hierarchy
Logic->>Memo: Return {tabToRender, workspaceTabs}
Memo->>DOM: Render active tab (visible)
Memo->>DOM: Render hidden workspace tabs (preserve scrollback)
DOM->>DOM: Apply drop overlay only to active tab
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/desktop/src/main/lib/terminal-manager.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Apply Biome formatting, linting, and import organization at the root level for all code files
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsxapps/desktop/src/main/lib/terminal-manager.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid usinganytype; use type-safe approaches instead, unless necessary
Ensure TypeScript type checking passes across all packages using bun run typecheck
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsxapps/desktop/src/main/lib/terminal-manager.ts
apps/**/*.{tsx,ts}
📄 CodeRabbit inference engine (AGENTS.md)
apps/**/*.{tsx,ts}: Structure React applications with one folder per component following the pattern: ComponentName/ComponentName.tsx with index.ts barrel export
Co-locate component dependencies (hooks, utils, constants, tests, stories) next to the file using them
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsxapps/desktop/src/main/lib/terminal-manager.ts
apps/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Use one component per file; do not create multi-component files
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsx
apps/desktop/src/renderer/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Define keyboard shortcuts using the centralized system in apps/desktop/src/renderer/lib/shortcuts.ts with Arc Browser-inspired shortcut definitions
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsx
apps/desktop/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
For Electron interprocess communication, ALWAYS use tRPC as defined in
src/lib/trpc
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsxapps/desktop/src/main/lib/terminal-manager.ts
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: Please use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsxapps/desktop/src/main/lib/terminal-manager.ts
⏰ 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 (3)
apps/desktop/src/main/lib/terminal-manager.ts (1)
90-94: LGTM! Login shell spawning logic is correct.The
-lflag for zsh/bash ensures profile files like.zprofile/.bash_profileare sourced, which is necessary for tools like pyenv, nvm, and rbenv that modify PATH in profile files. Theincludes()check correctly matches shell paths (e.g.,/bin/zsh,/usr/bin/bash), and the fallback to an empty array for other shells (fish, PowerShell) is appropriate since they handle initialization differently.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsx (2)
16-41: LGTM! Well-structured memoized computation.The memo correctly computes both
tabToRenderandworkspaceTabswith proper guards for missing workspace/tab IDs. The dependency array is complete, and the parent group resolution logic correctly handles nested tabs.
67-91: LGTM! Effective approach for preserving terminal scrollback.Using
visibility: hiddeninstead of conditional rendering keeps xterm.js instances mounted, preserving their internal state and scrollback buffer. TheisDropZoneprop is correctly scoped to only the active tab, and the singleDropOverlayrender at the end avoids duplication.
| <div ref={attachDrop} className="flex-1 h-full"> | ||
| <EmptyTabView /> | ||
| {/* Render all workspace tabs hidden to preserve terminal scrollback */} | ||
| {workspaceTabs.map((tab) => ( | ||
| <div | ||
| key={tab.id} | ||
| className="w-full h-full absolute inset-0" | ||
| style={{ visibility: "hidden", pointerEvents: "none" }} | ||
| > | ||
| {tab.type === TabType.Single ? ( | ||
| <SingleTabView tab={tab} isDropZone={false} /> | ||
| ) : ( | ||
| <GroupTabView tab={tab} /> | ||
| )} | ||
| </div> | ||
| ))} | ||
| </div> | ||
| ); |
There was a problem hiding this comment.
Missing relative class on container for absolute-positioned children.
The hidden tabs use absolute inset-0 positioning, but this container lacks relative positioning (unlike the container on line 68). This could cause the hidden tabs to position incorrectly relative to an unintended ancestor.
- <div ref={attachDrop} className="flex-1 h-full">
+ <div ref={attachDrop} className="flex-1 h-full relative">🤖 Prompt for AI Agents
In
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/index.tsx
around lines 47 to 64, the container div wrapping EmptyTabView and the hidden
workspace tab divs is missing the "relative" class so its absolutely-positioned
children (absolute inset-0) may be positioned relative to an unintended
ancestor; add the "relative" class to that container's className (e.g., include
"relative" alongside "flex-1 h-full") so the absolute-positioned tab panes are
correctly contained and positioned.
Summary by CodeRabbit
Release Notes
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.