Merged
Conversation
各ペインツールバーのSplit Paneボタン左にPop outボタンを追加。 クリックするとそのペインだけが独立した新ウィンドウとして開く。 ウィンドウを閉じるとタブがメインウィンドウに自動返却される。 - WindowManager: 複数BrowserWindowの管理、IPC同期ハンドラ - preloadで同期的にタブデータを注入（Zustand persistとの競合回避） - tearoffPaneIds: ターミナルセッションをkillせずにペインを分離 - 複数ペインのタブでは対象ペインのみ抽出してポップアウト
- TopBar・左サイドバー・右サイドバー(Changes/Files)を非表示 - プリセットボタン群(+Code等)を非表示、SetRunは維持 - ウィンドウドラッグ用バーをタブバー上部に追加 - ポップアウトウィンドウ内ではPop outボタンを非表示（無限増殖防止）
太いドラッグバーを削除し、タブバーの+ボタン右の空きスペースで ウィンドウを掴めるように変更。トラフィックライト用の左パディングも調整。
- 全タブを1回のIPCメッセージでまとめて送信（個別送信による重複を防止） - 受信側で既存タブIDとの重複チェックを追加
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 2 minutes and 50 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughImplements tab tearoff functionality enabling users to detach tabs and panes into separate Electron windows. Adds a WindowManager class for multi-window lifecycle tracking, TRPC endpoint for tearoff creation, preload-level data retrieval, renderer-side hooks for tearoff initialization, store actions for tab/pane hydration, and conditional UI updates to handle tearoff window state. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant Renderer as Renderer<br/>(Main Window)
participant IPC as Electron IPC
participant Main as Main Process
participant WindowMgr as WindowManager
participant NewWindow as New Tearoff<br/>Window
participant Preload as Preload Script
participant NewRenderer as Renderer<br/>(Tearoff Window)
User->>Renderer: Click "Pop out" button on pane
activate Renderer
Renderer->>Renderer: handlePopOut: Prepare tab/panes data
Renderer->>IPC: electronTrpc.tabTearoff.create(tab, panes, workspaceId, screenX, screenY)
deactivate Renderer
activate Main
IPC->>Main: TRPC mutation received
Main->>WindowMgr: setPendingTearoffData(windowId, {tab, panes, workspaceId})
activate WindowMgr
WindowMgr->>WindowMgr: Store data with 30s expiry
deactivate WindowMgr
Main->>WindowMgr: createTearoffWindow({windowId, screenX, screenY})
activate WindowMgr
WindowMgr->>NewWindow: Create BrowserWindow<br/>(frameless, hidden, partition)
WindowMgr->>NewWindow: Pass --tearoff-window-id arg
deactivate WindowMgr
NewWindow->>NewWindow: did-finish-load
WindowMgr->>NewWindow: Show window
deactivate Main
activate NewWindow
NewWindow->>Preload: Load preload script
activate Preload
Preload->>Preload: Parse --tearoff-window-id from argv
Preload->>IPC: ipcRenderer.sendSync("get-tearoff-data", windowId)
IPC->>Main: Sync IPC call
activate Main
Main->>WindowMgr: Retrieve pending data
Main->>IPC: Return {tab, panes, workspaceId}
deactivate Main
Preload->>Preload: Expose tearoffWindowId & tearoffData<br/>on window.App
deactivate Preload
NewWindow->>NewRenderer: Initialize with tearoff data
activate NewRenderer
NewRenderer->>NewRenderer: useTearoffInit(): Hydrate store<br/>from window.App.tearoffData
NewRenderer->>NewRenderer: Render isolated tab/workspace
deactivate NewRenderer
User->>NewRenderer: Work on pane content
User->>NewWindow: Close tearoff window
NewRenderer->>NewRenderer: beforeunload: Package tabs & panes
NewRenderer->>IPC: ipcRenderer.send("tearoff-return-tabs", {tab, panes})
deactivate NewWindow
activate Main
IPC->>Main: IPC handler: tearoff-return-tabs
Main->>IPC: webContents.send to main window:<br/>"tearoff-tab-returned"
deactivate Main
activate Renderer
Renderer->>Renderer: useReturnedTabListener():<br/>hydrateReturnedTab(tab, panes)
Renderer->>Renderer: Merge tab & panes into store
deactivate Renderer
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
メインウィンドウとの意味的な衝突を防止。
Owner
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceLayout/WorkspaceLayout.tsx (1)
37-53:⚠️ Potential issue | 🟠 MajorTearoff windows can still render
ChangesContentfull-width.The new guard only suppresses the
RightSidebarpanel. WhencurrentMode === SidebarMode.Changes, tearoff windows still take theisExpandedbranch and never renderContentView, so the popped-out pane can disappear behind the changes screen.Possible fix
export function WorkspaceLayout({ workspaceId, defaultExternalApp, onOpenInApp, onOpenQuickOpen, }: WorkspaceLayoutProps) { useBrowserLifecycle(); + const isTearoff = isTearoffWindow(); const isSidebarOpen = useSidebarStore((s) => s.isSidebarOpen); const sidebarWidth = useSidebarStore((s) => s.sidebarWidth); const setSidebarWidth = useSidebarStore((s) => s.setSidebarWidth); const isResizing = useSidebarStore((s) => s.isResizing); const setIsResizing = useSidebarStore((s) => s.setIsResizing); const currentMode = useSidebarStore((s) => s.currentMode); - const isExpanded = currentMode === SidebarMode.Changes; + const isExpanded = !isTearoff && currentMode === SidebarMode.Changes; ... - {!isTearoffWindow() && isSidebarOpen && ( + {!isTearoff && isSidebarOpen && ( <ResizablePanel🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceLayout/WorkspaceLayout.tsx` around lines 37 - 53, The tearoff window still shows ChangesContent because isExpanded only checks currentMode; change the expanded logic in WorkspaceLayout so tearoff windows never take the Changes branch: update the isExpanded computation to include the tearoff check (e.g., isExpanded = currentMode === SidebarMode.Changes && !isTearoffWindow()), so when isTearoffWindow() is true the component renders ContentView instead of ChangesContent (this keeps the existing RightSidebar guard intact).
🧹 Nitpick comments (1)
apps/desktop/src/main/windows/main.ts (1)
18-18: Use themain/*alias for this import.This file already uses tsconfig aliases for main-side modules, so the new
../hop adds a second import style for the same layer.As per coding guidelines, "Use aliases as defined in `tsconfig.json` when possible".♻️ Proposed fix
-import { windowManager } from "../lib/window-manager"; +import { windowManager } from "main/lib/window-manager";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/windows/main.ts` at line 18, The import uses a relative path instead of the tsconfig alias; replace the relative import of windowManager (import { windowManager } from "../lib/window-manager") with the main/* alias form (e.g., import { windowManager } from "main/lib/window-manager") so all main-side modules use the same alias-style imports.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/lib/trpc/routers/tab-tearoff.ts`:
- Line 18: The windowId currently uses Date.now() which can collide; replace
creation of windowId (the `const windowId = ...` in the tearoff creation flow)
with a collision-resistant UUID (e.g., crypto.randomUUID() or a nanoid) and use
that same generated id when populating `pendingTearoffData` and adding the entry
in `WindowManager.windows` so keys cannot collide across rapid tearoffs.
In `@apps/desktop/src/main/lib/window-manager/index.ts`:
- Around line 35-58: The ad-hoc IPC handlers in registerIpcHandlers (the
ipcMain.on listeners for "get-tearoff-data", "tearoff-return-tabs" and the
"tearoff-tab-returned" send) must be replaced with the repo's tRPC layer: remove
those ipcMain handlers and instead implement corresponding tRPC procedures
(e.g., tearoff.getTearoffData query that reads pendingTearoffData, and
tearoff.returnTabs mutation that accepts Array<{tab, panes}> and forwards to the
main window via the same getMain().webContents.send logic or via a tRPC
subscription/event); update the preload/renderer code to call the new trpc query
during preload for synchronous data needs (or adapt to an initial trpc query at
startup) and to call the trpc mutation for returning tabs; keep the same data
shape and ensure pendingTearoffData is accessed from the new trpc resolver.
- Around line 40-43: The current ipcMain.on("get-tearoff-data") handler deletes
pendingTearoffData immediately when the preload reads it, so crashes/reloads
before renderer bootstrap lose the only copy; change it to return the data
without deleting and instead delete only when the new window explicitly ACKs
bootstrap. Concretely: update the "get-tearoff-data" listener to read and return
this.pendingTearoffData.get(windowId) but not call delete, add a new ipcMain
listener (e.g., "tearoff-bootstrap-ack" or similar) that accepts the windowId
and removes this.pendingTearoffData.delete(windowId) when the renderer signals
it has hydrated, and optionally add a TTL/cleanup routine to purge stale entries
from pendingTearoffData to avoid unbounded growth.
In `@apps/desktop/src/preload/index.ts`:
- Around line 23-26: Replace the untyped tearoffData any with a proper interface
and expose it on window.App: define a shared type (e.g. TearoffPayload) in a
common types/schema module, import that type into the preload file, change the
declaration const tearoffData: any to const tearoffData: TearoffPayload | null,
cast/validate the ipcRenderer.sendSync("get-tearoff-data", tearoffWindowId)
result to that type (or run a small runtime guard using the shared schema), and
then assign it to window.App.tearoffData typed as TearoffPayload | null so the
renderer sees a typed payload; update references to tearoffData and
tearoffWindowId accordingly.
In `@apps/desktop/src/renderer/hooks/useTearoffInit/useTearoffInit.ts`:
- Around line 39-48: The payload built in tabsWithPanes only returns { tab,
panes } and therefore loses which tab was active; update the construction to
include the active-tab information by referencing the current active tab
identifier in state (e.g., state.activeTabId or similar) and either add an
isActive flag to each entry or include activeTabId at the top-level of the
tearoff payload so the main window can restore the correct active tab; change
the tabsWithPanes mapping (where state.tabs and state.panes are used) to include
that active marker or include state.activeTabId in the message sent.
In `@apps/desktop/src/renderer/lib/trpc-storage.ts`:
- Around line 5-8: The module-level boolean _isTearoffWindow is currently
causing createTrpcStorageAdapter to skip hydration/writes for every persisted
adapter; change the logic so tearoff-only bypass is scoped to the tabs adapter
(trpcTabsStorage) or made opt-in per adapter: remove the global early-return
based on _isTearoffWindow inside createTrpcStorageAdapter and instead either (A)
add an options parameter like { disableInTearoff?: boolean } to
createTrpcStorageAdapter and only skip IO when that option is true and
_isTearoffWindow is true, or (B) keep a special-case that only performs the
tearoff bypass when the adapterId equals "trpcTabsStorage" (or the exported
identifier for the tabs adapter). Update any call sites that construct the tabs
adapter to pass the new option if you choose the opt-in approach and remove the
module-level behavior so theme, hotkeys, ringtone, etc., are no longer affected.
In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/layout.tsx`:
- Around line 108-110: The two hooks useTearoffInit() and
useReturnedTabListener() currently use ipcRenderer directly and must be
refactored to use the electronTrpc client from renderer/lib/electron-trpc
following the same pattern as useWorkspaceShortcuts and
useBrowserNewWindowHandler: replace direct ipcRenderer calls inside
useTearoffInit and useReturnedTabListener with calls to
electronTrpc.procedure/mutation for one-off actions and
electronTrpc.subscription (implemented via observable<T>((emit) => { ... })) for
event streams; move the tearoff event emission logic out of
src/main/lib/window-manager/index.ts into corresponding tRPC
procedures/subscriptions on the main side and wire the renderer hooks to
subscribe/consume those procedures, keeping isTearoffWindow() as needed for
local checks and ensuring you unregister subscriptions in the hooks cleanup.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ContentHeader/ContentHeader.tsx`:
- Around line 18-24: The current header applies paddingLeft: 88 whenever
isTearoff is true; change it to only apply that inset on macOS by combining the
tearoff check with a macOS/platform check (e.g., process.platform === 'darwin'
or an existing isMac helper) in the ContentHeader component where isTearoff is
used; update the style prop so it sets { paddingLeft: 88 } only when isTearoff
&& isMac, otherwise undefined.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/components/PaneToolbarActions/PaneToolbarActions.tsx`:
- Around line 37-50: The pop-out icon-only button (the button with
onClick={onPopOut} that renders <LuArrowUpRight> inside
Tooltip/TooltipTrigger/TooltipContent) lacks an accessible name; add an
accessible label by giving that button an aria-label (e.g., aria-label="Pop out
to new window") or an aria-labelledby pointing to a visually hidden element
containing the same text so screen readers can announce it while keeping the
visual tooltip unchanged.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/index.tsx`:
- Around line 58-112: handlePopOut currently calls tearoffMutation.mutate()
fire-and-forget and immediately detaches the tab, which can leave local state
inconsistent on failure; change to use await tearoffMutation.mutateAsync inside
a try/catch in handlePopOut, only call state.detachTabForTearoff(tab.id) and
updated.detachTabForTearoff(newTabId) after mutateAsync resolves, and on error
rollback the local split created by movePaneToNewTab (implement a small helper
like revertMovePaneToNewTab(newTabId, paneId) or restore the original panes/tabs
in useTabsStore) and surface/log the error.
In `@apps/desktop/src/renderer/stores/tabs/store.ts`:
- Around line 2139-2148: The current fixed 5s timeout that clears tearoffPaneIds
(the setTimeout loop deleting paneId for each paneId) is fragile; replace the
timer-based clearing with an explicit lifecycle acknowledgement: stop using the
setTimeout block and instead clear the pane IDs when the tearoff window emits a
ready/ACK event (e.g., an IPC message like "tearoff-ready" or a lifecycle
callback from the new window) or when a reattach/close event is received; update
the code around tearoffPaneIds, paneIds and the timeout-removal logic so
deletion happens only on that explicit ACK (and also ensure you delete on
failure/abort paths) rather than after a fixed delay.
- Around line 191-213: The seeded tearoff state from _tearoffData is being
overwritten by the persisted rehydrate merge; update the persist call inside
useTabsStore (the create<TabsStore>() → devtools(persist(...)) block) to pass a
custom merge function that, when _tearoffData is present, preserves the seeded
keys (tabs, panes, activeTabIds, focusedPaneIds, tabHistoryStacks) from the
currentState instead of letting savedState overwrite them; implement the merge
to shallow-merge savedState into currentState but if _tearoffData is truthy copy
currentState.tabs, currentState.panes, currentState.activeTabIds,
currentState.focusedPaneIds and currentState.tabHistoryStacks into the merge
result so the tearoff bootstrap survives rehydration.
---
Outside diff comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceLayout/WorkspaceLayout.tsx`:
- Around line 37-53: The tearoff window still shows ChangesContent because
isExpanded only checks currentMode; change the expanded logic in WorkspaceLayout
so tearoff windows never take the Changes branch: update the isExpanded
computation to include the tearoff check (e.g., isExpanded = currentMode ===
SidebarMode.Changes && !isTearoffWindow()), so when isTearoffWindow() is true
the component renders ContentView instead of ChangesContent (this keeps the
existing RightSidebar guard intact).
---
Nitpick comments:
In `@apps/desktop/src/main/windows/main.ts`:
- Line 18: The import uses a relative path instead of the tsconfig alias;
replace the relative import of windowManager (import { windowManager } from
"../lib/window-manager") with the main/* alias form (e.g., import {
windowManager } from "main/lib/window-manager") so all main-side modules use the
same alias-style imports.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b525fefc-e23c-4d6c-b36a-438cf85c1669
📒 Files selected for processing (27)
apps/desktop/src/lib/trpc/routers/index.tsapps/desktop/src/lib/trpc/routers/tab-tearoff.tsapps/desktop/src/lib/window-loader.tsapps/desktop/src/main/lib/window-manager/index.tsapps/desktop/src/main/windows/main.tsapps/desktop/src/preload/index.tsapps/desktop/src/renderer/hooks/useTearoffInit/index.tsapps/desktop/src/renderer/hooks/useTearoffInit/useTearoffInit.tsapps/desktop/src/renderer/lib/trpc-storage.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/layout.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/ContentHeader/ContentHeader.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/BrowserPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/DevToolsPane/DevToolsPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/FileViewerPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/FileViewerToolbar/FileViewerToolbar.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/TabPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/components/BasePaneWindow/BasePaneWindow.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/components/PaneToolbarActions/PaneToolbarActions.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/pane-guards.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/components/PresetsBar/PresetsBar.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceLayout/WorkspaceLayout.tsxapps/desktop/src/renderer/stores/tabs/store.tsapps/desktop/src/renderer/stores/tabs/types.ts
- mainWindow不在時のタブ返却消失にログ警告を追加 - handlePopOutでmutate成功後にdetachするよう修正（タブ消失防止） - Pop outボタンにaria-labelを追加（a11y）
This was referenced Mar 31, 2026
This was referenced Apr 11, 2026
This was referenced Apr 19, 2026
MocA-Love
added a commit
that referenced
this pull request
Apr 24, 2026
upstream 取り込み PR #11: use task title as workspace name (superset-sh#3678)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Architecture
ipcRenderer.sendSyncで取得し、persist middlewareとの競合を完全排除UI
Test plan
Summary by CodeRabbit