From ccf37484a05511b664b73c5aac75f5a720bbf73a Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 22 Apr 2026 22:22:26 -0700 Subject: [PATCH] fix(browser): stop webview reload when moving tab between groups (#959) --- src/renderer/src/components/Terminal.tsx | 63 ++++-- .../browser-pane/BrowserPaneOverlayLayer.tsx | 195 ++++++++++++++++++ .../browser-pane/browser-pane-slots.ts | 29 +++ .../components/tab-group/TabGroupPanel.tsx | 46 +++-- .../tab-group/useTabGroupWorkspaceModel.ts | 9 - 5 files changed, 303 insertions(+), 39 deletions(-) create mode 100644 src/renderer/src/components/browser-pane/BrowserPaneOverlayLayer.tsx create mode 100644 src/renderer/src/components/browser-pane/browser-pane-slots.ts diff --git a/src/renderer/src/components/Terminal.tsx b/src/renderer/src/components/Terminal.tsx index ecf2c52f5be..1964a51a6e9 100644 --- a/src/renderer/src/components/Terminal.tsx +++ b/src/renderer/src/components/Terminal.tsx @@ -26,7 +26,9 @@ import { } from './editor/editor-autosave' import { isUpdaterQuitAndInstallInProgress } from '@/lib/updater-beforeunload' import EditorAutosaveController from './editor/EditorAutosaveController' +import type { TabGroupLayoutNode } from '../../../shared/types' import BrowserPane, { destroyPersistentWebview } from './browser-pane/BrowserPane' +import BrowserPaneOverlayLayer from './browser-pane/BrowserPaneOverlayLayer' import { reconcileTabOrder } from './tab-bar/reconcile-order' import TabGroupSplitLayout from './tab-group/TabGroupSplitLayout' import { shouldAutoCreateInitialTerminal } from './terminal/initial-terminal' @@ -1013,19 +1015,13 @@ function Terminal(): React.JSX.Element | null { // so the terminal/browser surface hides on the tasks page too. const isVisible = activeView === 'terminal' && worktree.id === activeWorktreeId return ( -
- - -
+ worktreeId={worktree.id} + layout={layout} + focusedGroupId={activeGroupIdByWorktree[worktree.id]} + isVisible={isVisible} + /> ) })} @@ -1237,4 +1233,47 @@ function Terminal(): React.JSX.Element | null { ) } +// Why: each TabGroupPanel tags its body element with an `anchor-name`, and +// a single worktree-level BrowserPaneOverlayLayer renders every browser tab +// for this worktree once — keyed by browserTab.id only — and pins each pane +// to the owning group's anchor via CSS `position-anchor`. Moving a tab +// between groups now only changes which anchor-name the overlay references, +// so the `` is never reparented (and never reloads). Mirrors +// VS Code's OverlayWebview claim/release pattern, with the browser doing all +// layout tracking for free. +// +// Why `React.memo`: Terminal.tsx has many store subscriptions and re-renders +// on unrelated updates (terminal keystrokes, editor edits, focus changes). +// Without memoization, every Terminal re-render would cascade into +// BrowserPaneOverlayLayer and its BrowserPane subtrees. Memoizing here means +// the surface only re-renders when its own props (worktreeId / layout / +// focusedGroupId / isVisible) actually change. +const WorktreeSplitSurface = React.memo(function WorktreeSplitSurface({ + worktreeId, + layout, + focusedGroupId, + isVisible +}: { + worktreeId: string + layout: TabGroupLayoutNode + focusedGroupId?: string + isVisible: boolean +}): React.JSX.Element { + return ( +
+ + + +
+ ) +}) + export default React.memo(Terminal) diff --git a/src/renderer/src/components/browser-pane/BrowserPaneOverlayLayer.tsx b/src/renderer/src/components/browser-pane/BrowserPaneOverlayLayer.tsx new file mode 100644 index 00000000000..4aaf88619df --- /dev/null +++ b/src/renderer/src/components/browser-pane/BrowserPaneOverlayLayer.tsx @@ -0,0 +1,195 @@ +import { memo, useCallback, useMemo } from 'react' +import { useShallow } from 'zustand/react/shallow' +import { useAppStore } from '../../store' +import type { BrowserTab as BrowserTabState, Tab, TabGroup } from '../../../../shared/types' +import BrowserPane from './BrowserPane' +import { browserSlotAnchorName } from './browser-pane-slots' + +// Why: Electron `` destroys its guest contents whenever its DOM +// parent changes. Rendering one BrowserPane per tab at the worktree level +// (keyed only by browserTab.id) means moving a tab between groups never +// remounts the pane and never reparents the webview — it only updates the +// overlay's CSS `position-anchor` so the pane tracks the new owning group's +// body via native CSS anchor positioning. + +type BrowserOverlayAssignment = { + groupId: string + isActiveInGroup: boolean +} + +const EMPTY_BROWSER_TABS: readonly BrowserTabState[] = [] +const EMPTY_UNIFIED_TABS: readonly Tab[] = [] +const EMPTY_GROUPS: readonly TabGroup[] = [] + +type BrowserOverlaySlotProps = { + browserTab: BrowserTabState + // Why: `undefined` means this browser tab has no owning group (an "orphan" — + // present in `browserTabs` but not referenced by any group's unified-tab + // list). See the fallback branch below for why we keep such tabs mounted. + groupId: string | undefined + isActive: boolean + // Why: the legacy architecture rendered BrowserPane inside TabGroupPanel, so + // React events from the pane bubbled through TabGroupPanel's + // `onPointerDown={focusGroup}` / `onFocusCapture={focusGroup}`. Now that + // BrowserPane lives in a worktree-level overlay that is a SIBLING of + // TabGroupSplitLayout, those events no longer reach TabGroupPanel — so in + // split view, clicking the browser chrome would leave + // `activeGroupIdByWorktree` stale. The overlay slot re-implements that + // focus sync directly, targeting the owning group. + onFocusOwningGroup: ((groupId: string) => void) | undefined +} + +// Why: each overlay slot is memoized so its BrowserPane subtree only re-renders +// when its own `browserTab`, `groupId`, or `isActive` changes. Without this, +// any unrelated worktree mutation (terminal keystrokes, editor updates, etc.) +// that re-renders the parent overlay layer would cascade into every +// BrowserPane — defeating the "never reparent/reload the webview" goal of +// this layer by constantly re-running props diffing on heavy subtrees. +const BrowserOverlaySlot = memo(function BrowserOverlaySlot({ + browserTab, + groupId, + isActive, + onFocusOwningGroup +}: BrowserOverlaySlotProps): React.JSX.Element { + const anchorName = groupId !== undefined ? browserSlotAnchorName(groupId) : undefined + // Why: each overlay pins itself to the owning TabGroupPanel's body via CSS + // anchor positioning. `anchor()` resolves top/left relative to the viewport, + // and the overlay's own `position: absolute` inside a positioned ancestor + // (the worktree surface div) converts those to the surface's coordinate + // space. `anchor-size()` fills the slot exactly. When the tab moves between + // groups, only `positionAnchor` changes and the browser relayouts on its + // own — no measurement or state updates. + // + // The orphan branch (no anchorName) keeps the pane mounted at 0×0 + // display:none so the DOM parent stays stable and the `` guest + // survives until the tab is reassigned (e.g. mid-move) or explicitly + // destroyed via `closeBrowserTab`. + const style: React.CSSProperties = useMemo( + () => + anchorName + ? { + position: 'absolute', + positionAnchor: anchorName, + top: `anchor(${anchorName} top)`, + left: `anchor(${anchorName} left)`, + width: `anchor-size(${anchorName} width)`, + height: `anchor-size(${anchorName} height)`, + display: isActive ? 'flex' : 'none', + pointerEvents: isActive ? 'auto' : 'none' + } + : { + position: 'absolute', + top: 0, + left: 0, + width: 0, + height: 0, + display: 'none', + pointerEvents: 'none' + }, + [anchorName, isActive] + ) + const handleFocus = useCallback(() => { + if (groupId !== undefined && onFocusOwningGroup) { + onFocusOwningGroup(groupId) + } + }, [groupId, onFocusOwningGroup]) + + return ( +
+ +
+ ) +}) + +// Why: memoize so parent re-renders (e.g. `WorktreeSplitSurface` re-rendering +// because `focusedGroupId` changed — a prop this component doesn't consume) +// don't rerun the overlay's zustand selector or the assignments mapping. +// The child `BrowserOverlaySlot` is already memoized, but skipping this layer +// entirely when its own props are unchanged keeps the fast path fastest. +const BrowserPaneOverlayLayer = memo(function BrowserPaneOverlayLayer({ + worktreeId, + isWorktreeActive +}: { + worktreeId: string + isWorktreeActive: boolean +}): React.JSX.Element { + const { browserTabs, unifiedTabs, groups } = useAppStore( + useShallow((state) => ({ + browserTabs: state.browserTabsByWorktree[worktreeId] ?? EMPTY_BROWSER_TABS, + unifiedTabs: state.unifiedTabsByWorktree[worktreeId] ?? EMPTY_UNIFIED_TABS, + groups: state.groupsByWorktree[worktreeId] ?? EMPTY_GROUPS + })) + ) + const focusGroup = useAppStore((state) => state.focusGroup) + + // Why: stable callback identity so BrowserOverlaySlot's memo isn't broken by + // a fresh function reference every render. The group id is passed in at call + // time so the same callback serves every slot regardless of which group owns + // that tab. + const focusOwningGroup = useCallback( + (groupId: string) => focusGroup(worktreeId, groupId), + [focusGroup, worktreeId] + ) + + // Why: derive the lookup OUTSIDE the zustand selector so shallow equality + // holds across unrelated store mutations. If we built the object inside the + // selector, every store change would create a new reference and useShallow + // would never find equality — the overlay would re-render on every + // keystroke in an unrelated terminal. + const groupActiveTabById = useMemo(() => { + const lookup: Record = {} + for (const group of groups) { + lookup[group.id] = group.activeTabId + } + return lookup + }, [groups]) + + // Map each browser tab to the group that owns it (if any) and whether it's + // the currently active tab in that group. Tabs that exist in `browserTabs` + // but are not referenced by any group's unified-tab list are "orphans": we + // still render the pane (at 0×0 display:none — see fallback branch below) + // so the `` survives until the tab is either reassigned or + // explicitly destroyed. In normal flows this is a transient mid-move + // state, not a steady state: closing a tab calls `closeBrowserTab` which + // removes it from `browserTabs` (and `destroyPersistentWebview` tears + // down the guest), and "Close Group" closes each browser tab before + // collapsing the group shell — no follow-to-sibling migration happens. + const assignments = useMemo(() => { + const entries = new Map() + for (const tab of unifiedTabs) { + if (tab.contentType !== 'browser') { + continue + } + entries.set(tab.entityId, { + groupId: tab.groupId, + isActiveInGroup: groupActiveTabById[tab.groupId] === tab.id + }) + } + return entries + }, [groupActiveTabById, unifiedTabs]) + + return ( + <> + {browserTabs.map((browserTab) => { + const assignment = assignments.get(browserTab.id) + const isActive = Boolean(isWorktreeActive && assignment && assignment.isActiveInGroup) + return ( + + ) + })} + + ) +}) + +export default BrowserPaneOverlayLayer diff --git a/src/renderer/src/components/browser-pane/browser-pane-slots.ts b/src/renderer/src/components/browser-pane/browser-pane-slots.ts new file mode 100644 index 00000000000..a7864bd63b2 --- /dev/null +++ b/src/renderer/src/components/browser-pane/browser-pane-slots.ts @@ -0,0 +1,29 @@ +// Why: Electron `` guest contents are destroyed whenever the host +// element is reparented in the DOM. So browser panes cannot live inside +// TabGroupPanel (which would unmount/remount them when a tab moves between +// groups). Instead, a single worktree-level BrowserPaneOverlayLayer renders +// one stable `` per browser tab and positions it over the +// owning group's body via CSS anchor positioning. This module provides the +// anchor-name bridge between the two: +// +// TabGroupPanel body → `anchor-name: --orca-browser-slot-` +// BrowserPaneOverlayLayer overlay → `position-anchor: --orca-browser-slot-` +// `top: anchor(--… top); left: anchor(--… left);` +// `width: anchor-size(--… width); height: anchor-size(--… height);` +// +// The browser does all layout tracking for free — no ResizeObserver, no +// rect state, no subscribe/notify machinery. Moving a tab between groups +// only changes which anchor-name the overlay references, so the `` +// is never reparented (and never reloads). Mirrors VS Code's +// `OverlayWebview` claim/release pattern, with CSS doing the positioning. + +const ANCHOR_PREFIX = '--orca-browser-slot-' + +/** + * Returns the CSS anchor name for a given tab-group id. Anchor names must be + * ``; groupIds are UUIDs (hex + `-`) so they are already safe + * as suffixes. Prefixed so they cannot collide with unrelated anchors. + */ +export function browserSlotAnchorName(groupId: string): string { + return `${ANCHOR_PREFIX}${groupId}` +} diff --git a/src/renderer/src/components/tab-group/TabGroupPanel.tsx b/src/renderer/src/components/tab-group/TabGroupPanel.tsx index 77aacfe0bd2..26954c185f3 100644 --- a/src/renderer/src/components/tab-group/TabGroupPanel.tsx +++ b/src/renderer/src/components/tab-group/TabGroupPanel.tsx @@ -1,4 +1,4 @@ -import { lazy, Suspense } from 'react' +import { lazy, Suspense, useMemo } from 'react' import { useDroppable } from '@dnd-kit/core' import { Columns2, Ellipsis, Rows2, X } from 'lucide-react' import { useAppStore } from '../../store' @@ -11,7 +11,7 @@ import { } from '@/components/ui/dropdown-menu' import TabBar from '../tab-bar/TabBar' import TerminalPane from '../terminal-pane/TerminalPane' -import BrowserPane from '../browser-pane/BrowserPane' +import { browserSlotAnchorName } from '../browser-pane/browser-pane-slots' import { useTabGroupWorkspaceModel } from './useTabGroupWorkspaceModel' import TabGroupDropOverlay from './TabGroupDropOverlay' import { getTabPaneBodyDroppableId, type TabDropZone } from './useTabDragSplit' @@ -44,7 +44,6 @@ export default function TabGroupPanel({ const model = useTabGroupWorkspaceModel({ groupId, worktreeId }) const { - activeBrowserTab, activeTab, browserItems, commands, @@ -63,6 +62,21 @@ export default function TabGroupPanel({ }, disabled: !isTabDragActive }) + // Why: browser panes for this worktree are rendered once at the worktree + // level (BrowserPaneOverlayLayer) and positioned over the owning group's + // body via CSS anchor positioning. Tagging this body with a per-group + // `anchor-name` lets the overlay reference it via `position-anchor`; + // moving a tab between groups only swaps which anchor-name the overlay + // targets, never reparenting the `` (which would reload it). + const bodyAnchorName = browserSlotAnchorName(groupId) + // Why: memoize the style object so the literal isn't recreated on every + // render. A fresh object every render would make the body `
` appear + // to have a new `style` prop on every parent re-render, which defeats any + // downstream memoization keyed on referential equality. + const bodyAnchorStyle = useMemo( + () => ({ anchorName: bodyAnchorName }) as React.CSSProperties, + [bodyAnchorName] + ) const tabBar = (
-
+
{activeDropZone ? : null} {model.groupTabs .filter((item) => item.contentType === 'terminal') @@ -325,20 +343,12 @@ export default function TabGroupPanel({
)} - {browserItems.map((bt) => ( -
- -
- ))} + {/* Why: browser panes are rendered at the worktree level by + BrowserPaneOverlayLayer and absolutely positioned over this body + element via the slot registered above. Rendering them per-group + here caused moving a browser tab between groups to unmount and + remount the pane, reparenting the Electron `` — which + destroys its guest contents and reloads the page. */}
) diff --git a/src/renderer/src/components/tab-group/useTabGroupWorkspaceModel.ts b/src/renderer/src/components/tab-group/useTabGroupWorkspaceModel.ts index 7f98a2400c0..8d0dfdb895c 100644 --- a/src/renderer/src/components/tab-group/useTabGroupWorkspaceModel.ts +++ b/src/renderer/src/components/tab-group/useTabGroupWorkspaceModel.ts @@ -145,14 +145,6 @@ export function useTabGroupWorkspaceModel({ [groupTabs, worktreeState.browserTabs] ) - const activeBrowserTab = useMemo( - () => - activeTab?.contentType === 'browser' - ? (worktreeState.browserTabs.find((bt) => bt.id === activeTab.entityId) ?? null) - : null, - [activeTab, worktreeState.browserTabs] - ) - const runtimeTerminalTabById = useMemo( () => new Map(worktreeState.runtimeTerminalTabs.map((tab) => [tab.id, tab])), [worktreeState.runtimeTerminalTabs] @@ -381,7 +373,6 @@ export function useTabGroupWorkspaceModel({ return { group, activeTab, - activeBrowserTab, browserItems, editorItems, terminalTabs,