feat(browser-tabs): split panes — drag tabs to edges for an IDE layout#3338
Open
adamleithp wants to merge 22 commits into
Open
feat(browser-tabs): split panes — drag tabs to edges for an IDE layout#3338adamleithp wants to merge 22 commits into
adamleithp wants to merge 22 commits into
Conversation
Migrating tabs out of bluebird surfaced three persistence bugs, all traced to writes computed from stale renderer state: - Apply every tab mutation's returned snapshot to the renderer mirror synchronously, plus optimistic activate/replace application, so a navigation racing the IPC round-trip can never target the previous tab (nav clicks opened duplicate tabs; tab switches overwrote other tabs' contents). - Guard decideTabNavigation against dead history tags (windowTabIds) and validate setActiveTab through a new setWindowActiveTab shared primitive so a closed tab's id replayed via back/forward can't persist a dangling activeTabId. - Replace the incomplete inline blank-tab check in __root.tsx with useActiveTabIsBlank (it missed channelId/appView). Adds appView as a tab identity dimension (migration 0017) so top-level pages (home, inbox, agents, skills, mcp-servers, command-center) get proper tab dedup and in-place navigation, plus 29 new browser-tabs tests including a regression suite for each reported bug. Generated-By: PostHog Code Task-Id: 0bf0f3b8-5bf9-4755-b8cf-e9f0b2a7de5c
Tab switches looked ignored for up to a second: route loaders are all synchronous, so the router committed navigation in one main-thread block with the OLD route frozen on screen through the destination's expensive mount (task detail's chat thread measurement effects, terminal focus, canvas grid — profiled at 400-800ms of layout/render work). Give heavy routes a single-frame `yieldToPaint()` loader (double rAF — one guaranteed painted frame) plus a per-route-kind `pendingComponent` skeleton (task detail, canvas, channel pages, top-level app pages). The click now commits the URL, moves the tab highlight, and paints the skeleton before the heavy mount runs behind it. Router config: `defaultPendingMinMs: 0` so skeletons aren't held for the 500ms default minimum, and `defaultPreloadStaleTime: 0` so a hover preload can't satisfy the loader and skip the pending paint (which would silently reintroduce the frozen-screen path for sidebar links). Parent layouts (/code/inbox, /code/agents) only pend while entering — child navigations keep the match in `success` and reload in the background, so no skeleton flash on inner navigation. Verified over CDP screencast against the live app: skeleton frame paints mid-switch, destination replaces it after mount. Generated-By: PostHog Code Task-Id: 0bf0f3b8-5bf9-4755-b8cf-e9f0b2a7de5c
Every task visited in a browser tab parked its xterm on unmount — terminal buffers, DOM subtree, and addons kept alive forever in the hidden parking container. Measured over CDP: xterm count grew monotonically with each distinct task visited (~3MB heap per task, GC-stable), the dominant tab-driven heap growth on the way to the 1.5GB budget. Keep at most 4 parked terminals; evict the least-recently-parked by destroying it. Safe because detach already serializes scrollback into the terminal store (restored via initialState on recreate), the shell session keeps running server-side and reattaches by id, and a parked terminal receives no live output anyway — the data subscription lives in the mounted Terminal component. Re-measured: xterm count plateaus at the cap across 12 task visits; residual growth drops to ~1MB/task (React Query's 5-minute gc window plus small per-task view stores). Generated-By: PostHog Code Task-Id: 0bf0f3b8-5bf9-4755-b8cf-e9f0b2a7de5c
…time Two more unbounded per-visit retainers behind tab-driven heap growth: - freeformChatStore kept a thread (canvas source + up to 50 whole-document version copies) for every canvas ever visited, for the app's lifetime. Cap at the 8 most-recently-used threads; eviction only runs from the mount-time seeding paths and skips threads with a save in flight. An evicted thread reseeds from the saved record on the next visit — every committed edit autosaves, so only an in-progress version-browse position is lost. - File-content queries (staleTime: Infinity) held full file bodies and base64 blobs under the default 5-minute gcTime, keeping every file viewed across every task resident simultaneously. Bound them to 30s after unmount; re-opens re-read from local disk. Generated-By: PostHog Code Task-Id: 0bf0f3b8-5bf9-4755-b8cf-e9f0b2a7de5c
Fixes from a review pass over the tab persistence + performance work: - freeformChatStore: eviction now skips (rather than aborts on) threads with a save in flight, so one slow autosave can't block the cap for every thread behind it; threadOrder moved into store state (visible to devtools/tests, HMR can't desync it from threads); revert() guards against evicted/never-seeded threads — patch() would materialize an empty thread and persist() would save code:"" over the real record. - BrowserTabStrip: activeTabId validates the history-state tab id against the live tab list before preferring it (a dead id from back/forward blanked the strip highlight and pointed Cmd+W at a nonexistent tab); optimistic activate/replace writes derive from the store's current snapshot instead of the effect closure's, so they can't regress the mirror past a mutation onSuccess; local primaryWindow duplicate replaced with the @posthog/shared export. - Route skeletons: swapped @radix-ui/themes Skeleton for @posthog/quill (quill-first rule); ChannelSkeleton/AppPageSkeleton share one ListPageSkeleton silhouette; new withRouteSkeleton(skeleton) factory replaces the pendingComponent + loader boilerplate copy-pasted across 12 route files. - Sidebar merge regressions: adopt the legacy Code sidebar width (sidebar-storage) into channels-sidebar the first time the unified store persists, instead of resetting users to the 240px default; restore the title-bar-left width transition with drag-time suppression (it snapped while the sidebar animated); re-fire the legacy enter_space/leave_space analytics from the Channels toggle so space-adoption dashboards stay continuous. Generated-By: PostHog Code Task-Id: 0bf0f3b8-5bf9-4755-b8cf-e9f0b2a7de5c
Generated-By: PostHog Code Task-Id: 0bf0f3b8-5bf9-4755-b8cf-e9f0b2a7de5c
The migration meta files were hand-built during a rebase conflict resolution (jq output + manual edit) and didn't match Biome's formatter, failing the quality check. Generated-By: PostHog Code Task-Id: 0bf0f3b8-5bf9-4755-b8cf-e9f0b2a7de5c
Adding a new AppView tab target now fails to compile in goToTab until its canonical route is wired, instead of silently falling through with no navigation. Closes the one drift gap in the appView tab-identity surface (APP_VIEW_META is already Record-forced, and shared/schema/DB treat appView as an opaque string). Generated-By: PostHog Code Task-Id: 0bf0f3b8-5bf9-4755-b8cf-e9f0b2a7de5c
Tab switches went sluggish (up to ~2s of frozen UI) once parked terminals started being torn down on switch. CPU profile pinned it: ~3.9s in `get offsetWidth`, from xterm's renderRows reading offsetWidth per row inside the WebGL-addon / term dispose path. The parking container used visibility:hidden (+ 0x0), which still participates in layout — so every per-row offsetWidth read during dispose forced a full-document reflow. Switch it to display:none: a display:none subtree has no layout box, so those reads return 0 with zero reflow. Parked terminals render nothing and refit on re-attach, so losing the layout box while parked is harmless. Generated-By: PostHog Code Task-Id: 0bf0f3b8-5bf9-4755-b8cf-e9f0b2a7de5c
…king The LRU eviction (cap parked terminals, destroy least-recently-used) made term.dispose() run on tab switches, which froze the app when leaving a tab whose terminal was on screen. The display:none parking change was a follow-on perf fix for that same dispose path. Both are reverted to main's terminal behavior — verified the freeze is gone. Terminal memory bounding is dropped from this PR; if we revisit it, it needs a teardown path that doesn't synchronously re-render/serialize on switch. Generated-By: PostHog Code Task-Id: 0bf0f3b8-5bf9-4755-b8cf-e9f0b2a7de5c
New tabs and the last-tab-close landing were routing through the /website index, which redirects to channels[0] — so a fresh tab (or an emptied strip) opened a random first channel. Land on a single flag-aware default instead, keyed off the channels toggle (not the current route, which lags a flip): - channels on -> the private #me channel (provisioned lazily) - channels off -> the Code new-task screen Unified new-tab (handleNewTab) and close-landing (applyCloseResult) through one landOnDefault() helper that never touches the channels index. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…s, drop dead code - browserTabsStore.setSnapshot bails on value-equal snapshots so a mutation's authoritative echo doesn't re-render every subscriber and re-run the nav effect when an optimistic write already set the same value. - Extract applyOptimistic() for the nav effect's activate/replace optimistic mirror writes instead of hand-inlining the same read-transform-set twice. - Dedupe concurrent #me provisioning in landOnDefault via a shared in-flight promise — a double-fired Cmd+T can't create two "me" folders. - Delete the dead ProjectSwitcher "button" trigger variant (no callers left). - Remove the now-unreachable FeedbackModal "leaving" mode + its test case. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Introduce panes into the browser-tabs domain as the foundation for VS Code-style split panes (stacked on #3221): - shared: TabsSnapshot gains flat pane rows + a recursive per-window layout tree (leaf/split with sizes) and focusedPaneId; tabs gain paneId; window activeTabId moves to the pane. New pure tree math in browser-pane-layout.ts (insert/remove/resize/normalize) and pane transforms (splitPane, moveTabToPane, closePane, setFocusedPane, setPaneActiveTab, setPaneSizes) plus ensureSnapshotIntegrity load-time healing. Tab transforms are pane-scoped; closing a pane's last tab backfills a blank tab (renderer-minted id); a pane emptied by a move/split collapses. decideTabNavigation is pane-scoped and gains a focusPane decision (window-level identity dedup). - workspace-server: browser_panes table + window layout/focusedPaneId via migration 0020 (backfills one root pane per window, reparents tabs, drops browser_windows.active_tab_id); repository loads/saves panes; service heals on boot and exposes the five pane procedures. - host-router/ui: new procedures forwarded; client facade extended; strip/DnD/order helpers bridged to the primary window's focused pane (single-pane behaviour unchanged until the pane tree renders). 166 new/updated shared tests (tree math, healing, pane transforms); migration verified against a live SQLite upgrade path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replace the window-wide hash router with createAppRouter(): each pane gets its own router over the shared route tree with in-memory history, so per-pane back/forward falls out by construction. Boot creates the focused pane's router once the tabs mirror seeds, with the initial entry restored from sessionStorage (paneLocationPersistence — what keeps Cmd+R/HMR reloads on the same screen now that the URL hash no longer carries the route) or the active tab's canonical href (hrefForTab, kept in sync with goToTab by test). Forward availability moves into a per-router tracker installed by the factory (component state would reset when the title bar swaps routers on focus change); usePaneHistoryControls exposes it and the title-bar buttons consume it. RouterDevtools attaches to the focused pane's router via the registry; the visibility watchdog reads the focused router's href instead of window.location.hash. Also includes the phase-2 chrome inversion (AppShell/PaneChrome/ paneRouterRegistry) — see the diff's shell/ and router/ layers. Verified live over CDP: boot restore from active tab, navigation persisting pane locations, full-reload restore with tabs intact. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Drag a tab pill onto a pane edge or a content-area edge to split the UI (arbitrary nesting, VS Code style); drop it on another pane's strip or center to move it. Nav sidebar and title bar stay stationary. - panes/: PaneTreeRenderer (recursive react-resizable-panels over the window layout; lone leaf renders bare — single-pane mode is pixel-safe), BrowserPane (focus boundary + lazily created per-pane router + registry cleanup), PaneDropZones (5-zone hit + VS Code-style result preview, mounted over the content slot only so strip drops stay reachable), RootDropZones (18px window edges, high collision priority), paneDragStore (transient drag state). - BrowserTabStrip is per-pane: mounted inside its pane's router, its navigation effect reconciles that pane's own location; hotkeys (Cmd+T/W/1-9) gate on pane focus so a split doesn't multi-fire; the focusPane decision routes an identity open elsewhere to its pane; strip gains a close-pane X in split mode. - BrowserTabsDnd: same-pane reorder unchanged; cross-pane drops resolve to moveTabToPane/splitPane (one applyLocalTransform + one persistWrite each) plus imperative history pushes so affected panes show their new active tabs. Pill sortable groups are per-pane and the horizontal-axis lock is gone (the reorder preview never depended on dnd transforms). - Pane sizes commit on resize-gesture end (uncontrolled panels; a per-frame mirror write would hold the tabsSync gate open) and persist in the layout tree. - AppShell's router context follows the focused pane reactively (useFocusedPaneRouter); AGENTS.md's parked split-view section is replaced with the shipped architecture. Verified live over CDP: edge-drag split (+restart persistence), nested splits, cross-pane strip drop with source collapse, resize persistence, close-pane collapse, focus-follow history buttons, per-pane TaskDetail with its own terminals. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
React Doctor found 17 issues in 5 files · 1 error & 16 warnings. Errors
16 warnings
Reviewed by React Doctor for commit |
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
Contributor
|
Reviews (1): Last reviewed commit: "feat(browser-tabs): split panes — drag t..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| UPDATE `browser_windows` SET | ||
| `layout` = json_object('type', 'leaf', 'paneId', `id` || '-root'), | ||
| `focused_pane_id` = `id` || '-root';--> statement-breakpoint | ||
| ALTER TABLE `browser_windows` DROP COLUMN `active_tab_id`; No newline at end of file |
Contributor
There was a problem hiding this comment.
This migration drops browser_windows.active_tab_id in the same rollout that introduces pane-owned focus. During a rolling deploy, an old workspace-server process can still read or write that column after the migration runs, and tab startup or persistence will fail with a missing-column database error.
Rule Used: When removing database fields, do not remove them ... (source)
Learned From
PostHog/posthog#32876
Comment on lines
+265
to
+268
| const historyTabIsLive = | ||
| !!historyTabId && snapshot.tabs.some((t) => t.id === historyTabId); | ||
| const activeTabId = | ||
| (historyTabIsLive ? historyTabId : null) ?? pane?.activeTabId ?? null; |
Contributor
There was a problem hiding this comment.
This strip validates a history tabId against every tab in the snapshot, not just this pane. If a tab is moved to another pane and this pane later replays an old history entry tagged with that tab, this pane can highlight and close the other pane's tab before the reconciliation effect fixes the stale entry.
Suggested change
| const historyTabIsLive = | |
| !!historyTabId && snapshot.tabs.some((t) => t.id === historyTabId); | |
| const activeTabId = | |
| (historyTabIsLive ? historyTabId : null) ?? pane?.activeTabId ?? null; | |
| const historyTabIsLive = | |
| !!historyTabId && | |
| snapshot.tabs.some((t) => t.id === historyTabId && t.paneId === paneId); | |
| const activeTabId = | |
| (historyTabIsLive ? historyTabId : null) ?? pane?.activeTabId ?? null; |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Replace the hairline between panes with an 8px gutter and a small centered rounded pill (4x32) that highlights on hover/drag — the whole gutter is the hit area. The width override needs Tailwind important because globals.css pins [data-panel-resize-handle-enabled] to 1px for the task-detail panels feature. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replace the JS-driven accent ring on the focused pane with a pure CSS focus-within ring (ring-1 ring-primary, inset so the overflow-hidden Panel wrapper doesn't clip it). Panels get a border + rounded frame and the split gets breathing room; the divider pill sits on the background gutter. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
:focus-within misses clicks on non-focusable content (no DOM focus moves) and drops the ring when navigation blurs the pane. Ring off data-focused instead — focusedPaneId is already maintained by every path that should highlight a pane: the pane's pointerdown capture, tab activation, and in-tab navigation (the shared transforms all set window.focusedPaneId). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
An inset ring on the pane element paints under its children, so content flush with the edge (scrollers, section headers) covered parts of it. Draw it on a pointer-transparent absolute overlay above the content instead (below the drop zones), still toggled by data-focused. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Stacked on #3221.
True IDE-like split panes for the browser-tab system: drag a tab pill onto the left/right/top/bottom edge of the content area (or of any pane) to split the UI, with arbitrary VS Code-style nesting. The nav sidebar and title bar stay stationary; each pane gets its own tab strip and its own back/forward history, and the global title-bar back/forward buttons drive whichever pane is focused.
How it works
Data model (
@posthog/shared+ migration0020): windows now hold a recursive pane layout (leaf/split nodes with persisted sizes) plus flat pane rows carrying per-paneactiveTabId; tabs gainpaneId;window.focusedPaneIdnames the pane that owns shortcuts and imperative navigation. All tab transforms are pane-scoped, andensureSnapshotIntegrityheals the snapshot on boot (migration backfills one root pane per window — existing tabs are preserved).One router per pane: the window-wide hash router is gone. Each pane hosts its own TanStack router over the shared route tree with in-memory history (
createAppRouter), so per-pane back/forward falls out by construction. Chrome inverted out of__rootintoAppShell, wrapped in aRouterContextProviderbound to the focused pane's router — every existing router hook in the sidebar/title bar follows pane focus automatically. Pane locations persist to sessionStorage so Cmd+R/HMR reloads restore; across relaunches each pane's active tab is the source of truth. Settings stays a route in the pane's history but renders as a full-window portal overlay.Interactions:
Sync keeps the local-first model from #3221: every mutation is one synchronous pure-transform apply to the mirror + one background persist, with renderer-minted ids (
paneId,blankTabId) so optimistic and durable state agree.Verification
🤖 Generated with Claude Code