From 3fba1952c8096d9ecac3fef20a01e7e671db4b4b Mon Sep 17 00:00:00 2001 From: Kelvin Amoaba <97001695+AmoabaKelvin@users.noreply.github.com> Date: Fri, 2 Oct 2026 23:47:05 +0000 Subject: [PATCH] perf(tab-bar): a change to one tab no longer re-renders every tab (#24261) With many tabs open, a change to any one tab (a retitle, an agent finishing, a tab switch, a git status write, or a browser tab update on SSH and web clients) re-rendered every tab in the strip, so the strip stuttered. Each tab is now a memoized row that re-renders only when its own values change, with stable handlers, a stable drag id list and stable drag sensor options. Editor tabs get their own git status, and mirrored browser tabs keep their page-id list while the ids don't change. Part of #24241: opening, closing or reordering a tab still re-renders every tab once. --- .../components/tab-bar/EditorFileTab.test.tsx | 2 +- .../src/components/tab-bar/EditorFileTab.tsx | 10 +- ...Bar.client-hosted-row-active-state.test.ts | 2 +- .../tab-bar/TabBar.context-menu.test.ts | 5 + .../src/components/tab-bar/TabBar.tsx | 8 +- .../components/tab-bar/TabBarItemRow.test.tsx | 379 ++++++++++++++++++ .../src/components/tab-bar/TabBarItemRow.tsx | 197 +++++++++ .../components/tab-bar/tab-bar-item-model.ts | 19 + ...urface.client-hosted-active-state.test.tsx | 166 -------- .../tab-bar/tab-bar-item-surface.tsx | 312 ++++---------- .../components/tab-bar/tab-bar-surface.tsx | 10 +- .../tab-bar/tab-title-tooltip.test.tsx | 3 +- .../tab-bar/use-tab-bar-item-actions.ts | 92 +++++ .../use-tab-bar-item-projection.test.tsx | 95 +++++ .../tab-bar/use-tab-bar-item-projection.ts | 10 +- .../tab-group/useTabDragSplit.test.ts | 20 + .../components/tab-group/useTabDragSplit.ts | 12 +- ...-session-tabs-sync-mirror-identity.test.ts | 32 ++ .../mirrored-browser-tabs.ts | 8 +- tests/e2e/helpers/runtime-types.ts | 3 + tests/e2e/helpers/tab-render-recorder.ts | 80 ++++ .../tab-strip-tab-render-isolation.spec.ts | 97 +++++ 22 files changed, 1142 insertions(+), 420 deletions(-) create mode 100644 src/renderer/src/components/tab-bar/TabBarItemRow.test.tsx create mode 100644 src/renderer/src/components/tab-bar/TabBarItemRow.tsx delete mode 100644 src/renderer/src/components/tab-bar/tab-bar-item-surface.client-hosted-active-state.test.tsx create mode 100644 src/renderer/src/components/tab-bar/use-tab-bar-item-actions.ts create mode 100644 src/renderer/src/components/tab-bar/use-tab-bar-item-projection.test.tsx create mode 100644 tests/e2e/helpers/tab-render-recorder.ts create mode 100644 tests/e2e/tab-strip-tab-render-isolation.spec.ts diff --git a/src/renderer/src/components/tab-bar/EditorFileTab.test.tsx b/src/renderer/src/components/tab-bar/EditorFileTab.test.tsx index 8f80a6ce670..f8d930b044c 100644 --- a/src/renderer/src/components/tab-bar/EditorFileTab.test.tsx +++ b/src/renderer/src/components/tab-bar/EditorFileTab.test.tsx @@ -260,7 +260,7 @@ async function renderEditorFileTab( hasTabsToRight: false, hasTabsToLeft: false, tabCount: 1, - statusByRelativePath: new Map(), + gitStatus: null, onActivate, onClose: () => {}, onCloseOthers: () => {}, diff --git a/src/renderer/src/components/tab-bar/EditorFileTab.tsx b/src/renderer/src/components/tab-bar/EditorFileTab.tsx index a9ee9eb37cd..34d173c246a 100644 --- a/src/renderer/src/components/tab-bar/EditorFileTab.tsx +++ b/src/renderer/src/components/tab-bar/EditorFileTab.tsx @@ -3,7 +3,7 @@ import { useSortable } from '@dnd-kit/sortable' import { GitCompareArrows, Eye, ShieldAlert, Pin, ListChecks } from 'lucide-react' import { Input } from '@/components/ui/input' import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip' -import { basename, normalizeRelativePath } from '@/lib/path' +import { basename } from '@/lib/path' import { getEditorDisplayLabel } from '@/components/editor/editor-labels' import { renameFileOnDisk } from '@/lib/rename-file' import { isImeCompositionKeyDown } from '@/lib/ime-composition-keyboard-event' @@ -40,7 +40,7 @@ export default function EditorFileTab({ hasTabsToRight, hasTabsToLeft, tabCount, - statusByRelativePath, + gitStatus: tabStatus, onActivate, onClose, onCloseOthers, @@ -59,7 +59,7 @@ export default function EditorFileTab({ hasTabsToRight: boolean hasTabsToLeft: boolean tabCount: number - statusByRelativePath: Map + gitStatus: GitFileStatus | null onActivate: () => void onClose: () => void onCloseOthers: () => void @@ -196,10 +196,6 @@ export default function EditorFileTab({ [file.filePath] ) - const tabStatus = - file.relativePath === 'All Changes' - ? null - : (statusByRelativePath.get(normalizeRelativePath(file.relativePath)) ?? null) const tabStatusColor = tabStatus ? STATUS_COLORS[tabStatus] : undefined const tabLabel = getEditorDisplayLabel(file) diff --git a/src/renderer/src/components/tab-bar/TabBar.client-hosted-row-active-state.test.ts b/src/renderer/src/components/tab-bar/TabBar.client-hosted-row-active-state.test.ts index c856727e289..41bd5d796af 100644 --- a/src/renderer/src/components/tab-bar/TabBar.client-hosted-row-active-state.test.ts +++ b/src/renderer/src/components/tab-bar/TabBar.client-hosted-row-active-state.test.ts @@ -107,7 +107,7 @@ async function renderTerminalStrip(): Promise | null> { onSetTabColor: () => {}, onTogglePaneExpand: () => {} }), - 'SortableTab' + 'TabBarItemRow' ) } diff --git a/src/renderer/src/components/tab-bar/TabBar.context-menu.test.ts b/src/renderer/src/components/tab-bar/TabBar.context-menu.test.ts index 8b96951efc4..312dccfafed 100644 --- a/src/renderer/src/components/tab-bar/TabBar.context-menu.test.ts +++ b/src/renderer/src/components/tab-bar/TabBar.context-menu.test.ts @@ -53,6 +53,11 @@ const useAppStoreMock = vi.fn( vi.mock('react', async () => await stubHeadlessReact()) vi.mock('zustand/react/shallow', () => stubShallowSelector()) +// The headless React stub has no dispatcher for the hook each tab row subscribes to language changes with. +vi.mock('react-i18next', async () => ({ + ...(await vi.importActual>('react-i18next')), + useTranslation: () => ({}) +})) vi.mock('lucide-react', async () => (await import('./lucide-icon-stub-fixture')).stubEveryIcon()) diff --git a/src/renderer/src/components/tab-bar/TabBar.tsx b/src/renderer/src/components/tab-bar/TabBar.tsx index 923afdf7ba7..99e22b3f29e 100644 --- a/src/renderer/src/components/tab-bar/TabBar.tsx +++ b/src/renderer/src/components/tab-bar/TabBar.tsx @@ -7,6 +7,7 @@ import { useTabBarRuntimeModel } from './use-tab-bar-runtime-model' import { useTabBarCreateMenuController } from './use-tab-bar-create-menu-controller' import { useTabBarItemProjection } from './use-tab-bar-item-projection' import { renderTabBarSurface } from './tab-bar-surface' +import { useTabBarItemActions } from './use-tab-bar-item-actions' import { useActiveClientHostedBrowserRowId } from '@/lib/pane-manager/client-hosted-browser-row-state' function TabBarInner(props: TabBarProps): React.JSX.Element { @@ -64,6 +65,11 @@ function TabBarInner(props: TabBarProps): React.JSX.Element { } runtime.pinTab(item.unifiedTabId) } + const itemActions = useTabBarItemActions({ + props, + togglePinned, + toggleTabViewMode: runtime.toggleTabViewMode + }) // Read here, not just where the rows render: the real tabs have to know when a row took over. const activeClientHostedBrowserRowId = useActiveClientHostedBrowserRowId({ worktreeId, @@ -92,7 +98,7 @@ function TabBarInner(props: TabBarProps): React.JSX.Element { tabStripNavigation, tabStripDragScroll, activeClientHostedBrowserRowId, - togglePinned + itemActions }) } diff --git a/src/renderer/src/components/tab-bar/TabBarItemRow.test.tsx b/src/renderer/src/components/tab-bar/TabBarItemRow.test.tsx new file mode 100644 index 00000000000..a095d4e2e6f --- /dev/null +++ b/src/renderer/src/components/tab-bar/TabBarItemRow.test.tsx @@ -0,0 +1,379 @@ +// @vitest-environment happy-dom + +import { act, startTransition, Suspense, use } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { + clearClientHostedBrowserRowSelection, + getClientHostedBrowserRowSelection, + selectClientHostedBrowserRow +} from '@/lib/pane-manager/client-hosted-browser-row-state' +import { i18n } from '@/i18n/i18n' +import type { TabBarItem } from './tab-bar-item-model' +import type { WorkspaceVisibleTabType } from '../../../../shared/tab-types' +import { + renderTabBarItems, + type TabBarItemSurfaceProps, + type TabBarItemSurfaceRuntime +} from './tab-bar-item-surface' +import { useTabBarItemActions } from './use-tab-bar-item-actions' + +globalThis.IS_REACT_ACT_ENVIRONMENT = true + +type TabProps = { + tab?: { id: string; title: string } + isActive: boolean + isPinned: boolean + onActivate: (id: string) => void + onDuplicate?: () => void + gitStatus?: string | null +} + +// Every render of every tab, in order, keyed by the id the strip shows it under. +const tabRenders: { id: string; props: TabProps }[] = [] + +vi.mock('./SortableTab', () => ({ + default: (props: TabProps) => { + tabRenders.push({ id: props.tab!.id, props }) + return null + } +})) +vi.mock('./BrowserTab', () => ({ + default: (props: TabProps) => { + tabRenders.push({ id: props.tab!.id, props }) + return null + }, + getBrowserTabLabel: () => '' +})) +vi.mock('./EditorFileTab', () => ({ + default: (props: TabProps & { file: { id: string } }) => { + tabRenders.push({ id: props.file.id, props }) + return null + } +})) + +/** Rebuilt on every call, like the strip's projections: equal content, never the same objects. */ +function buildItems({ terminalPinned = false, browserTitle = 'Example' } = {}): TabBarItem[] { + return [ + { + type: 'terminal', + id: 'terminal-1', + unifiedTabId: 'unified-terminal-1', + isPinned: terminalPinned, + data: { + id: 'terminal-1', + ptyId: null, + worktreeId: 'wt-1', + title: 'zsh', + generatedTitle: 'Fix the login bug', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 0 + } + }, + { + type: 'browser', + id: 'browser-1', + unifiedTabId: 'unified-browser-1', + isPinned: false, + data: { + id: 'browser-1', + worktreeId: 'wt-1', + url: 'https://example.test', + title: browserTitle, + loading: false, + faviconUrl: null, + canGoBack: false, + canGoForward: false, + loadError: null, + createdAt: 0 + } + }, + { + type: 'editor', + id: 'file-1', + unifiedTabId: 'unified-file-1', + isPinned: false, + data: { + id: 'file-1', + filePath: '/repo/notes.md', + relativePath: 'notes.md', + worktreeId: 'wt-1', + language: 'markdown', + isPreview: false, + isDirty: false, + mode: 'edit' + } + }, + { + type: 'agent-session', + id: 'session-1', + unifiedTabId: 'session-1', + isPinned: false, + data: { + id: 'session-1', + entityId: 'session-1', + groupId: 'group-1', + worktreeId: 'wt-1', + contentType: 'agent-session', + label: 'Session', + customLabel: null, + color: null, + sortOrder: 3, + createdAt: 0 + } + } + ] +} + +type StripInputs = { + onActivate?: (id: string) => void + generatedTabTitlesEnabled?: boolean + managedBrowserCreationEnabled?: boolean + terminalPinned?: boolean + browserTitle?: string + activeTabType?: WorkspaceVisibleTabType + activeClientHostedBrowserRowId?: string | null + statusByRelativePath?: TabBarItemSurfaceRuntime['statusByRelativePath'] +} + +function Strip({ + onActivate = NOOP, + generatedTabTitlesEnabled = false, + managedBrowserCreationEnabled = false, + terminalPinned, + browserTitle, + activeTabType = 'terminal', + activeClientHostedBrowserRowId = null, + statusByRelativePath = STATUS_BY_RELATIVE_PATH +}: StripInputs): React.JSX.Element { + const props: TabBarItemSurfaceProps = { + worktreeId: 'wt-1', + activeTabId: 'terminal-1', + activeFileId: 'file-1', + activeBrowserTabId: 'browser-1', + activeSimulatorTabId: null, + activeTabType, + expandedPaneByTabId: {} + } + const runtime: TabBarItemSurfaceRuntime = { + resolvedGroupId: 'group-1', + generatedTabTitlesEnabled, + unifiedTabByVisibleId: new Map(), + nativeChatEnabled: false, + tabAgentTypesByTabId: {}, + nativeChatTabWideFallbackUnsafeTabsById: {}, + nativeChatTranscriptIsLocalReadable: false, + managedBrowserCreationEnabled, + statusByRelativePath + } + const actions = useTabBarItemActions({ + props: { + onActivate, + onActivateFile: onActivate, + onActivateBrowserTab: onActivate, + onActivateAgentSession: onActivate, + onClose: NOOP, + onCloseOthers: NOOP, + onCloseToRight: NOOP, + onCloseToLeft: NOOP, + onSetCustomTitle: NOOP, + onSetTabColor: NOOP, + onTogglePaneExpand: NOOP + }, + togglePinned: NOOP, + toggleTabViewMode: NOOP + }) + return ( + <> + {renderTabBarItems({ + items: buildItems({ terminalPinned, browserTitle }), + props, + runtime, + actions, + dropIndicatorByVisibleId: new Map(), + includeTopTabBorder: true, + activeClientHostedBrowserRowId + })} + + ) +} + +const NOOP = (): void => {} +const STATUS_BY_RELATIVE_PATH: TabBarItemSurfaceRuntime['statusByRelativePath'] = new Map() +const TAB_IDS = ['terminal-1', 'browser-1', 'file-1', 'session-1'] +let root: Root | null = null + +const NEVER_RESOLVES = new Promise(() => {}) + +function Suspended(): null { + use(NEVER_RESOLVES) + return null +} + +function stripTree(inputs: StripInputs, suspended = false): React.JSX.Element { + return ( + + + {suspended ? : null} + + ) +} + +function renderStrip(inputs: StripInputs = {}): void { + if (!root) { + root = createRoot(document.createElement('div')) + } + act(() => root!.render(stripTree(inputs))) +} + +function lastRender(tabId: string): TabProps { + return tabRenders.findLast((render) => render.id === tabId)!.props +} + +afterEach(() => { + act(() => root?.unmount()) + root = null + tabRenders.length = 0 + clearClientHostedBrowserRowSelection() +}) + +describe('tab strip rows', () => { + it('skips every tab when the strip re-renders with equal tab content', () => { + renderStrip() + expect(tabRenders.map((render) => render.id)).toEqual(TAB_IDS) + + renderStrip({ onActivate: vi.fn() }) + + expect(tabRenders).toHaveLength(TAB_IDS.length) + }) + + it('calls the handler the strip has now, from a tab that skipped its render', () => { + const first = vi.fn() + const current = vi.fn() + renderStrip({ onActivate: first }) + renderStrip({ onActivate: current }) + + lastRender('terminal-1').onActivate('terminal-1') + + expect(current).toHaveBeenCalledWith('terminal-1') + expect(first).not.toHaveBeenCalled() + }) + + it('keeps the committed handler when React abandons a render', async () => { + const committed = vi.fn() + const abandoned = vi.fn() + renderStrip({ onActivate: committed }) + await act(async () => + startTransition(() => root!.render(stripTree({ onActivate: abandoned }, true))) + ) + + lastRender('terminal-1').onActivate('terminal-1') + + expect(committed).toHaveBeenCalledWith('terminal-1') + expect(abandoned).not.toHaveBeenCalled() + }) + + it('re-renders every tab when the language is re-applied, as a language-pack reload does', async () => { + renderStrip() + + await act(() => i18n.changeLanguage(i18n.language)) + + expect(tabRenders.map((render) => render.id)).toEqual([...TAB_IDS, ...TAB_IDS]) + }) + + it('re-renders a terminal tab when the generated-titles setting changes its title', () => { + renderStrip() + expect(lastRender('terminal-1').tab?.title).toBe('zsh') + + renderStrip({ generatedTabTitlesEnabled: true }) + + expect(lastRender('terminal-1').tab?.title).toBe('Fix the login bug') + }) + + it('re-renders a browser tab when duplicating becomes available', () => { + renderStrip() + expect(lastRender('browser-1').onDuplicate).toBeUndefined() + + renderStrip({ managedBrowserCreationEnabled: true }) + + expect(lastRender('browser-1').onDuplicate).toBeTypeOf('function') + }) + + it('re-renders a tab when it is pinned', () => { + renderStrip() + expect(lastRender('terminal-1').isPinned).toBe(false) + + renderStrip({ terminalPinned: true }) + + expect(lastRender('terminal-1').isPinned).toBe(true) + }) + + it('re-renders only the two tabs a switch moves the active state between', () => { + renderStrip({ activeTabType: 'terminal' }) + tabRenders.length = 0 + + renderStrip({ activeTabType: 'browser' }) + + expect(tabRenders.map((render) => render.id)).toEqual(['terminal-1', 'browser-1']) + }) + + it('re-renders a tab when its own data changes', () => { + renderStrip() + + renderStrip({ browserTitle: 'Renamed page' }) + + expect(lastRender('browser-1').tab?.title).toBe('Renamed page') + expect(tabRenders).toHaveLength(TAB_IDS.length + 1) + }) + + it('re-renders an editor tab only when a git status write changes its own status', () => { + renderStrip() + + renderStrip({ statusByRelativePath: new Map([['other.md', 'modified']]) }) + expect(tabRenders).toHaveLength(TAB_IDS.length) + + renderStrip({ statusByRelativePath: new Map([['notes.md', 'modified']]) }) + expect(lastRender('file-1').gitStatus).toBe('modified') + expect(tabRenders).toHaveLength(TAB_IDS.length + 1) + }) + + it.each(TAB_IDS)('retires a client-hosted row selection when %s is activated', (tabId) => { + renderStrip() + selectClientHostedBrowserRow({ + worktreeId: 'wt-1', + browserPageId: 'page-1', + groupId: 'group-1', + groupActiveTabIdAtSelection: 'unified-terminal-1' + }) + + lastRender(tabId).onActivate(tabId) + + expect(getClientHostedBrowserRowSelection()).toBeNull() + }) +}) + +/** + * A client-hosted row covers the pane without moving the group's `activeTabId`, so the strip's two + * halves would each keep painting an underline — the reported double highlight. + */ +describe('real tabs while a client-hosted row is selected', () => { + const activeFlags = (): boolean[] => TAB_IDS.map((tabId) => lastRender(tabId).isActive) + + it.each(['terminal', 'browser', 'editor'])( + 'underlines the %s tab the group is actually showing when no row is selected', + (activeTabType) => { + renderStrip({ activeTabType }) + expect(activeFlags().filter(Boolean)).toHaveLength(1) + } + ) + + it.each(['terminal', 'browser', 'editor'])( + 'renders the %s tab inactive while a row owns the pane', + (activeTabType) => { + renderStrip({ activeTabType, activeClientHostedBrowserRowId: 'page-1' }) + expect(activeFlags()).toEqual([false, false, false, false]) + } + ) +}) diff --git a/src/renderer/src/components/tab-bar/TabBarItemRow.tsx b/src/renderer/src/components/tab-bar/TabBarItemRow.tsx new file mode 100644 index 00000000000..ead0645c7a5 --- /dev/null +++ b/src/renderer/src/components/tab-bar/TabBarItemRow.tsx @@ -0,0 +1,197 @@ +import { memo } from 'react' +import { useTranslation } from 'react-i18next' +import { shallow } from 'zustand/shallow' +import type { GitFileStatus } from '../../../../shared/git-status-types' +import type { TerminalTab } from '../../../../shared/terminal-tab-types' +import type { TuiAgent } from '../../../../shared/tui-agent' +import { isAgentSessionHandleProvider } from '../../../../shared/agent-session-provider-handle' +import type { OpenFile } from '../../store/slices/editor' +import SortableTab from './SortableTab' +import EditorFileTab from './EditorFileTab' +import BrowserTab from './BrowserTab' +import type { DropIndicator } from './drop-indicator' +import type { TabDragItemData } from '../tab-group/useTabDragSplit' +import { getTabDragLabel, resolveTerminalItemTab, type TabBarItem } from './tab-bar-item-model' +import type { TabBarItemActions } from './use-tab-bar-item-actions' + +// Why only values and `actions`: anything a tab draws must be a compared prop, or a skipped render shows it stale. +type TabBarItemRowProps = { + item: TabBarItem + actions: TabBarItemActions + worktreeId: string + groupId: string + generatedTabTitlesEnabled: boolean + tabCount: number + hasTabsToLeft: boolean + hasTabsToRight: boolean + isActive: boolean + isExpanded: boolean + dropIndicator: DropIndicator + includeTopTabBorder: boolean + canToggleViewMode: boolean + isChatView: boolean + /** Unified tab whose view mode the terminal tab toggles; absent when it has none. */ + viewModeTabId: string | undefined + canDuplicate: boolean + /** This editor tab's own status, so a git status write re-renders only the tabs it changed. */ + gitStatus: GitFileStatus | null +} + +function TabBarItemRow({ + item, + actions, + worktreeId, + groupId, + generatedTabTitlesEnabled, + tabCount, + hasTabsToLeft, + hasTabsToRight, + isActive, + isExpanded, + dropIndicator, + includeTopTabBorder, + canToggleViewMode, + isChatView, + viewModeTabId, + canDuplicate, + gitStatus +}: TabBarItemRowProps): React.JSX.Element { + // Why: the tabs' labels come from `translate()`, which a skipped render would leave in the old language. + useTranslation() + const dragData: TabDragItemData = { + kind: 'tab', + worktreeId, + groupId, + unifiedTabId: item.unifiedTabId, + visibleTabId: item.id, + tabType: item.type, + label: getTabDragLabel(item, generatedTabTitlesEnabled), + iconPath: item.type === 'editor' ? item.data.filePath : undefined, + color: item.type === 'terminal' ? (item.data.color ?? null) : null + } + const shared = { + isActive, + isPinned: item.isPinned, + hasTabsToRight, + hasTabsToLeft, + tabCount, + onTogglePin: () => actions.togglePinned(item), + dragData, + dropIndicator, + includeTopTabBorder + } + const sortableTabProps = { + ...shared, + unifiedTabId: item.unifiedTabId, + groupId, + onClose: actions.close, + onCloseOthers: actions.closeOthers, + onCloseToRight: actions.closeToRight, + onCloseToLeft: actions.closeToLeft, + onSetCustomTitle: actions.setCustomTitle, + onSetTabColor: actions.setTabColor + } + if (item.type === 'terminal') { + return ( + actions.toggleViewMode(viewModeTabId) : undefined} + isExpanded={isExpanded} + onActivate={actions.activateTerminal} + onToggleExpand={actions.togglePaneExpand} + /> + ) + } + if (item.type === 'agent-session') { + const structuredTab: TerminalTab = { + id: item.id, + ptyId: null, + worktreeId, + title: item.data.label, + customTitle: item.data.customLabel, + color: item.data.color, + sortOrder: item.data.sortOrder, + createdAt: item.data.createdAt, + ...(isAgentSessionHandleProvider(item.data.agentSessionAgent) + ? { launchAgent: item.data.agentSessionAgent as TuiAgent } + : {}) + } + return ( + {}} + canSplitTerminal={false} + /> + ) + } + const closeScope = { + onCloseOthers: () => actions.closeOthers(item.id), + onCloseToRight: () => actions.closeToRight(item.id), + onCloseToLeft: () => actions.closeToLeft(item.id) + } + if (item.type === 'browser') { + return ( + actions.activateBrowserTab(item.id)} + onClose={() => actions.closeBrowserTab(item.id)} + onDuplicate={ + canDuplicate ? () => actions.duplicateBrowserTab(item.id, item.unifiedTabId) : undefined + } + /> + ) + } + const fileTabProps = { + ...shared, + ...closeScope, + gitStatus, + onActivate: () => actions.activateFile(item.id), + onClose: () => actions.closeFile(item.id), + onCloseAll: actions.closeAllFiles + } + if (item.type === 'simulator') { + const simulatorLabel = item.data.label || 'Mobile Emulator' + const simulatorFile: OpenFile & { tabId: string } = { + id: item.id, + tabId: item.id, + filePath: simulatorLabel, + relativePath: simulatorLabel, + worktreeId, + language: 'simulator', + isPreview: false, + isDirty: false, + mode: 'edit' + } + return {}} /> + } + return ( + actions.makePreviewFilePermanent(item.data.id, item.data.tabId)} + /> + ) +} + +// Why field-by-field: the strip rebuilds every `item` and its `data` on any tab write, so identity never survives. +function sameTabBarItemRowProps(previous: TabBarItemRowProps, next: TabBarItemRowProps): boolean { + const { item: previousItem, ...previousRest } = previous + const { item: nextItem, ...nextRest } = next + const { data: previousData, ...previousIdentity } = previousItem + const { data: nextData, ...nextIdentity } = nextItem + return ( + shallow(previousRest, nextRest) && + shallow(previousIdentity, nextIdentity) && + shallow(previousData, nextData) + ) +} + +export default memo(TabBarItemRow, sameTabBarItemRowProps) diff --git a/src/renderer/src/components/tab-bar/tab-bar-item-model.ts b/src/renderer/src/components/tab-bar/tab-bar-item-model.ts index 3789d89b7de..6967197e7a9 100644 --- a/src/renderer/src/components/tab-bar/tab-bar-item-model.ts +++ b/src/renderer/src/components/tab-bar/tab-bar-item-model.ts @@ -1,4 +1,5 @@ import type { BrowserTab as BrowserTabState } from '../../../../shared/browser-workspace-types' +import type { GitFileStatus } from '../../../../shared/git-status-types' import type { Tab, WorkspaceVisibleTabType } from '../../../../shared/tab-types' import type { TerminalTab } from '../../../../shared/terminal-tab-types' import { resolveTerminalTabTitle } from '../../../../shared/tab-title-resolution' @@ -48,6 +49,24 @@ export type TabBarItem = data: Tab & { contentType: 'agent-session' } } +/** The terminal tab as the strip shows it: its title resolved against the generated-titles setting. */ +export function resolveTerminalItemTab( + tab: TerminalTab & { unifiedTabId?: string }, + generatedTitlesEnabled: boolean +): TerminalTab & { unifiedTabId?: string } { + return { ...tab, title: resolveTerminalTabTitle(tab, generatedTitlesEnabled, tab.title) } +} + +export function resolveEditorTabGitStatus( + relativePath: string, + statusByRelativePath: Map +): GitFileStatus | null { + if (relativePath === 'All Changes') { + return null + } + return statusByRelativePath.get(normalizeRelativePath(relativePath)) ?? null +} + export function getTabDragLabel(item: TabBarItem, generatedTitlesEnabled: boolean): string { if (item.type === 'terminal') { return resolveTerminalTabTitle(item.data, generatedTitlesEnabled, item.data.title) diff --git a/src/renderer/src/components/tab-bar/tab-bar-item-surface.client-hosted-active-state.test.tsx b/src/renderer/src/components/tab-bar/tab-bar-item-surface.client-hosted-active-state.test.tsx deleted file mode 100644 index a3f85044626..00000000000 --- a/src/renderer/src/components/tab-bar/tab-bar-item-surface.client-hosted-active-state.test.tsx +++ /dev/null @@ -1,166 +0,0 @@ -// @vitest-environment happy-dom - -import type React from 'react' -import { afterEach, describe, expect, it } from 'vitest' -import type { WorkspaceVisibleTabType } from '../../../../shared/tab-types' -import { - clearClientHostedBrowserRowSelection, - getClientHostedBrowserRowSelection, - selectClientHostedBrowserRow -} from '@/lib/pane-manager/client-hosted-browser-row-state' -import type { TabBarItem } from './tab-bar-item-model' -import type { TabBarProps } from './tab-bar-props' -import type { TabBarRuntimeModel } from './use-tab-bar-runtime-model' -import { renderTabBarItems } from './tab-bar-item-surface' - -const ITEMS: TabBarItem[] = [ - { - type: 'terminal', - id: 'terminal-1', - unifiedTabId: 'unified-terminal-1', - isPinned: false, - data: { - id: 'terminal-1', - ptyId: null, - worktreeId: 'wt-1', - title: 'Setup', - customTitle: null, - color: null, - sortOrder: 0, - createdAt: 0 - } - }, - { - type: 'browser', - id: 'browser-1', - unifiedTabId: 'unified-browser-1', - isPinned: false, - data: { - id: 'browser-1', - worktreeId: 'wt-1', - url: 'https://example.test/', - title: 'Example Domain', - loading: false, - faviconUrl: null, - canGoBack: false, - canGoForward: false, - loadError: null, - createdAt: 0 - } - }, - { - type: 'editor', - id: 'file-1', - unifiedTabId: 'unified-file-1', - isPinned: false, - data: { - id: 'file-1', - filePath: '/repo/README.md', - relativePath: 'README.md', - worktreeId: 'wt-1', - language: 'markdown', - isPreview: false, - isDirty: false, - mode: 'edit' - } - } -] - -// Only the fields renderTabBarItems reads; nothing else reaches an isActive decision. -const RUNTIME = { - resolvedGroupId: 'group-1', - generatedTabTitlesEnabled: false, - unifiedTabByVisibleId: new Map(), - nativeChatEnabled: false, - tabAgentTypesByTabId: {}, - nativeChatTabWideFallbackUnsafeTabsById: {}, - nativeChatTranscriptIsLocalReadable: false, - managedBrowserCreationEnabled: false, - toggleTabViewMode: () => {}, - statusByRelativePath: new Map() -} as unknown as TabBarRuntimeModel - -function makeProps(activeTabType: WorkspaceVisibleTabType): TabBarProps { - return { - worktreeId: 'wt-1', - activeTabId: 'terminal-1', - activeFileId: 'file-1', - activeBrowserTabId: 'browser-1', - activeSimulatorTabId: null, - activeTabType, - groupActiveTabId: 'unified-terminal-1', - expandedPaneByTabId: {} - } as unknown as TabBarProps -} - -function activeFlags( - activeTabType: WorkspaceVisibleTabType, - activeClientHostedBrowserRowId: string | null -): boolean[] { - const rendered = renderTabBarItems({ - items: ITEMS, - props: makeProps(activeTabType), - runtime: RUNTIME, - dropIndicatorByVisibleId: new Map(), - includeTopTabBorder: true, - activeClientHostedBrowserRowId, - togglePinned: () => {} - }) - return rendered.map( - (node) => (node as React.ReactElement<{ isActive: boolean }>).props.isActive === true - ) -} - -afterEach(() => { - clearClientHostedBrowserRowSelection() -}) - -/** - * A client-hosted row covers the pane without moving the group's `activeTabId`, so the strip's two - * halves would each keep painting an underline — the reported double highlight. - */ -describe('real tabs while a client-hosted row is selected', () => { - it.each(['terminal', 'browser', 'editor'])( - 'underlines the %s tab the group is actually showing when no row is selected', - (activeTabType) => { - expect(activeFlags(activeTabType, null).filter(Boolean)).toHaveLength(1) - } - ) - - it.each(['terminal', 'browser', 'editor'])( - 'renders the %s tab inactive while a row owns the pane', - (activeTabType) => { - expect(activeFlags(activeTabType, 'page-1')).toEqual([false, false, false]) - } - ) -}) - -describe('client-hosted row while a real tab is activated', () => { - it.each(ITEMS.map((item, index) => [item.type, index] as const))( - 'retires the row selection when the %s tab is clicked', - (_type, index) => { - selectClientHostedBrowserRow({ - worktreeId: 'wt-1', - browserPageId: 'page-1', - groupId: 'group-1', - groupActiveTabIdAtSelection: 'unified-terminal-1' - }) - const rendered = renderTabBarItems({ - items: ITEMS, - props: makeProps('terminal'), - runtime: RUNTIME, - dropIndicatorByVisibleId: new Map(), - includeTopTabBorder: true, - activeClientHostedBrowserRowId: 'page-1', - togglePinned: () => {} - }) - - const clicked = rendered[index] as React.ReactElement<{ - onActivate: (id: string) => void - }> - clicked.props.onActivate('terminal-1') - - expect(getClientHostedBrowserRowSelection()).toBeNull() - } - ) -}) diff --git a/src/renderer/src/components/tab-bar/tab-bar-item-surface.tsx b/src/renderer/src/components/tab-bar/tab-bar-item-surface.tsx index 449b9628ca8..7cd3c69cd35 100644 --- a/src/renderer/src/components/tab-bar/tab-bar-item-surface.tsx +++ b/src/renderer/src/components/tab-bar/tab-bar-item-surface.tsx @@ -1,37 +1,57 @@ import React from 'react' -import { resolveTerminalTabTitle } from '../../../../shared/tab-title-resolution' -import type { TerminalTab } from '../../../../shared/terminal-tab-types' -import type { TuiAgent } from '../../../../shared/tui-agent' -import { isAgentSessionHandleProvider } from '../../../../shared/agent-session-provider-handle' -import type { OpenFile } from '../../store/slices/editor' import { canToggleNativeChat } from '../native-chat/native-chat-availability' import { resolveNativeChatTabAgentEvidence } from './native-chat-tab-agent-evidence' -import SortableTab from './SortableTab' -import EditorFileTab from './EditorFileTab' -import BrowserTab from './BrowserTab' import type { DropIndicator } from './drop-indicator' -import type { TabDragItemData } from '../tab-group/useTabDragSplit' -import { getTabDragLabel, type TabBarItem } from './tab-bar-item-model' +import { + resolveEditorTabGitStatus, + resolveTerminalItemTab, + type TabBarItem +} from './tab-bar-item-model' import type { TabBarProps } from './tab-bar-props' import type { TabBarRuntimeModel } from './use-tab-bar-runtime-model' -import { clearClientHostedBrowserRowSelection } from '@/lib/pane-manager/client-hosted-browser-row-state' +import type { TabBarItemActions } from './use-tab-bar-item-actions' +import TabBarItemRow from './TabBarItemRow' + +export type TabBarItemSurfaceProps = Pick< + TabBarProps, + | 'worktreeId' + | 'activeTabId' + | 'activeFileId' + | 'activeBrowserTabId' + | 'activeSimulatorTabId' + | 'activeTabType' + | 'expandedPaneByTabId' +> + +export type TabBarItemSurfaceRuntime = Pick< + TabBarRuntimeModel, + | 'resolvedGroupId' + | 'generatedTabTitlesEnabled' + | 'unifiedTabByVisibleId' + | 'nativeChatEnabled' + | 'tabAgentTypesByTabId' + | 'nativeChatTabWideFallbackUnsafeTabsById' + | 'nativeChatTranscriptIsLocalReadable' + | 'managedBrowserCreationEnabled' + | 'statusByRelativePath' +> export function renderTabBarItems({ items, props, runtime, + actions, dropIndicatorByVisibleId, includeTopTabBorder, - activeClientHostedBrowserRowId, - togglePinned + activeClientHostedBrowserRowId }: { items: TabBarItem[] - props: TabBarProps - runtime: TabBarRuntimeModel + props: TabBarItemSurfaceProps + runtime: TabBarItemSurfaceRuntime + actions: TabBarItemActions dropIndicatorByVisibleId: Map includeTopTabBorder: boolean activeClientHostedBrowserRowId: string | null - togglePinned: (item: TabBarItem) => void }): React.ReactNode[] { const { worktreeId, @@ -40,23 +60,7 @@ export function renderTabBarItems({ activeBrowserTabId, activeSimulatorTabId, activeTabType, - expandedPaneByTabId, - onActivate, - onClose, - onCloseOthers, - onCloseToRight, - onCloseToLeft, - onSetCustomTitle, - onSetTabColor, - onTogglePaneExpand, - onActivateFile, - onCloseFile, - onActivateBrowserTab, - onActivateAgentSession, - onCloseBrowserTab, - onDuplicateBrowserTab, - onCloseAllFiles, - onMakePreviewFilePermanent + expandedPaneByTabId } = props const { resolvedGroupId, @@ -66,7 +70,7 @@ export function renderTabBarItems({ tabAgentTypesByTabId, nativeChatTabWideFallbackUnsafeTabsById, nativeChatTranscriptIsLocalReadable, - toggleTabViewMode, + managedBrowserCreationEnabled, statusByRelativePath } = runtime @@ -74,40 +78,40 @@ export function renderTabBarItems({ // the group's own activeTabId never moves for it, and two underlines would show at once. const clientHostedRowOwnsActiveState = activeClientHostedBrowserRowId !== null - // Why: this is the strip's single activation fan-out, so retiring a client-hosted placeholder - // here covers every row kind — including re-clicking the tab that was already active, which the - // group's activeTabId never moves for. - function activateRealTab(activate: ((arg: TArg) => void) | undefined): (arg: TArg) => void { - return (arg) => { - clearClientHostedBrowserRowSelection() - activate?.(arg) + function isActiveItem(item: TabBarItem): boolean { + if (clientHostedRowOwnsActiveState) { + return false } + if (item.type === 'terminal') { + return ( + (activeTabType === 'terminal' || activeTabType === 'simulator') && item.id === activeTabId + ) + } + if (item.type === 'browser') { + return activeTabType === 'browser' && activeBrowserTabId === item.id + } + if (item.type === 'simulator') { + return activeTabType === 'simulator' && item.id === activeSimulatorTabId + } + if (item.type === 'agent-session') { + return activeTabType === 'agent-session' && item.id === activeTabId + } + return (activeTabType === 'editor' || activeTabType === 'simulator') && activeFileId === item.id } return items.map((item, index) => { - const dragData: TabDragItemData = { - kind: 'tab', - worktreeId, - groupId: resolvedGroupId, - unifiedTabId: item.unifiedTabId, - visibleTabId: item.id, - tabType: item.type, - label: getTabDragLabel(item, generatedTabTitlesEnabled), - iconPath: item.type === 'editor' ? item.data.filePath : undefined, - color: item.type === 'terminal' ? (item.data.color ?? null) : null - } + let canToggleViewMode = false + let isChatView = false + let viewModeTabId: string | undefined if (item.type === 'terminal') { - const terminalTab = { - ...item.data, - title: resolveTerminalTabTitle(item.data, generatedTabTitlesEnabled, item.data.title) - } + const terminalTab = resolveTerminalItemTab(item.data, generatedTabTitlesEnabled) const unifiedTabForItem = unifiedTabByVisibleId.get(item.id) // Carry the agent *identity* (not just "an agent exists") so the native-chat gate can reject agents like Grok. const resolvedAgent = resolveNativeChatTabAgentEvidence(terminalTab, unifiedTabForItem) // Key the live-agent lookup by the backing terminal tab id: agent-status pane keys use it, not the unified tab id. const detectedAgent = tabAgentTypesByTabId[terminalTab.id] ?? null const tabWideFallbackSafe = nativeChatTabWideFallbackUnsafeTabsById[terminalTab.id] !== true - const canToggleViewMode = + canToggleViewMode = unifiedTabForItem !== undefined && canToggleNativeChat({ experimentalNativeChatEnabled: nativeChatEnabled, @@ -118,185 +122,33 @@ export function renderTabBarItems({ nativeChatTranscriptIsLocalReadable, isChatViewMode: unifiedTabForItem.viewMode === 'chat' }) - return ( - toggleTabViewMode(unifiedTabForItem.id) : undefined - } - hasTabsToRight={index < items.length - 1} - hasTabsToLeft={index > 0} - isActive={ - !clientHostedRowOwnsActiveState && - (activeTabType === 'terminal' || activeTabType === 'simulator') && - item.id === activeTabId - } - isPinned={item.isPinned} - isExpanded={expandedPaneByTabId[item.id] === true} - onActivate={activateRealTab(onActivate)} - onClose={onClose} - onCloseOthers={onCloseOthers} - onCloseToRight={onCloseToRight} - onCloseToLeft={onCloseToLeft} - onSetCustomTitle={onSetCustomTitle} - onSetTabColor={onSetTabColor} - onTogglePin={() => togglePinned(item)} - onToggleExpand={onTogglePaneExpand} - dragData={dragData} - dropIndicator={dropIndicatorByVisibleId.get(item.id) ?? null} - includeTopTabBorder={includeTopTabBorder} - /> - ) - } - if (item.type === 'browser') { - return ( - 0} - tabCount={items.length} - onActivate={() => activateRealTab(onActivateBrowserTab)(item.id)} - onClose={() => onCloseBrowserTab?.(item.id)} - onCloseOthers={() => onCloseOthers(item.id)} - onCloseToRight={() => onCloseToRight(item.id)} - onCloseToLeft={() => onCloseToLeft(item.id)} - onDuplicate={ - runtime.managedBrowserCreationEnabled - ? () => onDuplicateBrowserTab?.(item.id, item.unifiedTabId) - : undefined - } - onTogglePin={() => togglePinned(item)} - dragData={dragData} - dropIndicator={dropIndicatorByVisibleId.get(item.id) ?? null} - includeTopTabBorder={includeTopTabBorder} - /> - ) - } - if (item.type === 'simulator') { - const simulatorLabel = item.data.label || 'Mobile Emulator' - const simulatorFile: OpenFile & { tabId: string } = { - id: item.id, - tabId: item.id, - filePath: simulatorLabel, - relativePath: simulatorLabel, - worktreeId, - language: 'simulator', - isPreview: false, - isDirty: false, - mode: 'edit' - } - return ( - 0} - tabCount={items.length} - statusByRelativePath={statusByRelativePath} - onActivate={() => activateRealTab(onActivateFile)(item.id)} - onClose={() => onCloseFile?.(item.id)} - onCloseOthers={() => onCloseOthers(item.id)} - onCloseToRight={() => onCloseToRight(item.id)} - onCloseToLeft={() => onCloseToLeft(item.id)} - onCloseAll={() => onCloseAllFiles?.()} - onMakePermanent={() => {}} - onTogglePin={() => togglePinned(item)} - dragData={dragData} - dropIndicator={dropIndicatorByVisibleId.get(item.id) ?? null} - includeTopTabBorder={includeTopTabBorder} - /> - ) - } - if (item.type === 'agent-session') { - const structuredTab: TerminalTab = { - id: item.id, - ptyId: null, - worktreeId, - title: item.data.label, - customTitle: item.data.customLabel, - color: item.data.color, - sortOrder: item.data.sortOrder, - createdAt: item.data.createdAt, - ...(isAgentSessionHandleProvider(item.data.agentSessionAgent) - ? { launchAgent: item.data.agentSessionAgent as TuiAgent } - : {}) - } - return ( - 0} - isActive={ - !clientHostedRowOwnsActiveState && - activeTabType === 'agent-session' && - item.id === activeTabId - } - isPinned={item.isPinned} - isExpanded={false} - onActivate={() => activateRealTab(onActivateAgentSession)(item.id)} - onClose={() => onClose(item.id)} - onCloseOthers={() => onCloseOthers(item.id)} - onCloseToRight={() => onCloseToRight(item.id)} - onCloseToLeft={() => onCloseToLeft(item.id)} - onSetCustomTitle={onSetCustomTitle} - onSetTabColor={onSetTabColor} - onTogglePin={() => togglePinned(item)} - onToggleExpand={() => {}} - canSplitTerminal={false} - dragData={dragData} - dropIndicator={dropIndicatorByVisibleId.get(item.id) ?? null} - includeTopTabBorder={includeTopTabBorder} - /> - ) + isChatView = nativeChatEnabled && unifiedTabForItem?.viewMode === 'chat' + viewModeTabId = unifiedTabForItem?.id } return ( - 0} + item={item} + actions={actions} + worktreeId={worktreeId} + groupId={resolvedGroupId} + generatedTabTitlesEnabled={generatedTabTitlesEnabled} tabCount={items.length} - statusByRelativePath={statusByRelativePath} - onActivate={() => activateRealTab(onActivateFile)(item.id)} - onClose={() => onCloseFile?.(item.id)} - onCloseOthers={() => onCloseOthers(item.id)} - onCloseToRight={() => onCloseToRight(item.id)} - onCloseToLeft={() => onCloseToLeft(item.id)} - onCloseAll={() => onCloseAllFiles?.()} - onMakePermanent={() => onMakePreviewFilePermanent?.(item.data.id, item.data.tabId)} - onTogglePin={() => togglePinned(item)} - dragData={dragData} + hasTabsToLeft={index > 0} + hasTabsToRight={index < items.length - 1} + isActive={isActiveItem(item)} + isExpanded={item.type === 'terminal' && expandedPaneByTabId[item.id] === true} dropIndicator={dropIndicatorByVisibleId.get(item.id) ?? null} includeTopTabBorder={includeTopTabBorder} + canToggleViewMode={canToggleViewMode} + isChatView={isChatView} + viewModeTabId={viewModeTabId} + canDuplicate={item.type === 'browser' && managedBrowserCreationEnabled} + gitStatus={ + item.type === 'editor' + ? resolveEditorTabGitStatus(item.data.relativePath, statusByRelativePath) + : null + } /> ) }) diff --git a/src/renderer/src/components/tab-bar/tab-bar-surface.tsx b/src/renderer/src/components/tab-bar/tab-bar-surface.tsx index 1ef24b61831..8a6ad5be964 100644 --- a/src/renderer/src/components/tab-bar/tab-bar-surface.tsx +++ b/src/renderer/src/components/tab-bar/tab-bar-surface.tsx @@ -20,7 +20,7 @@ import type { TabBarProps } from './tab-bar-props' import type { TabBarRuntimeModel } from './use-tab-bar-runtime-model' import type { TabBarCreateMenuController } from './use-tab-bar-create-menu-controller' import type { TabBarItemProjection } from './use-tab-bar-item-projection' -import type { TabBarItem } from './tab-bar-item-model' +import type { TabBarItemActions } from './use-tab-bar-item-actions' import { renderTabBarItems } from './tab-bar-item-surface' import { TabBarStaticCreateMenu } from './tab-bar-static-create-menu' import ClientHostedBrowserTabRows from './ClientHostedBrowserTabRows' @@ -36,7 +36,7 @@ export function renderTabBarSurface({ tabStripNavigation, tabStripDragScroll, activeClientHostedBrowserRowId, - togglePinned + itemActions }: { props: TabBarProps runtime: TabBarRuntimeModel @@ -45,7 +45,7 @@ export function renderTabBarSurface({ tabStripNavigation: ReturnType tabStripDragScroll: ReturnType activeClientHostedBrowserRowId: string | null - togglePinned: (item: TabBarItem) => void + itemActions: TabBarItemActions }): React.JSX.Element { const { worktreeId, @@ -95,10 +95,10 @@ export function renderTabBarSurface({ items: orderedItems, props, runtime, + actions: itemActions, dropIndicatorByVisibleId, includeTopTabBorder, - activeClientHostedBrowserRowId, - togglePinned + activeClientHostedBrowserRowId }) return ( diff --git a/src/renderer/src/components/tab-bar/tab-title-tooltip.test.tsx b/src/renderer/src/components/tab-bar/tab-title-tooltip.test.tsx index c5e863af5ca..445076b590b 100644 --- a/src/renderer/src/components/tab-bar/tab-title-tooltip.test.tsx +++ b/src/renderer/src/components/tab-bar/tab-title-tooltip.test.tsx @@ -2,7 +2,6 @@ import { cloneElement, isValidElement, type ReactElement, type ReactNode } from import { renderToStaticMarkup } from 'react-dom/server' import { beforeEach, describe, expect, it, vi } from 'vitest' import type { BrowserTab as BrowserTabState } from '../../../../shared/browser-workspace-types' -import type { GitFileStatus } from '../../../../shared/git-status-types' import type { TerminalTab } from '../../../../shared/terminal-tab-types' import type { TuiAgent } from '../../../../shared/tui-agent' import type { OpenFile } from '../../store/slices/editor' @@ -361,7 +360,7 @@ describe('tab title tooltips', () => { hasTabsToRight={false} hasTabsToLeft={false} tabCount={1} - statusByRelativePath={new Map()} + gitStatus={null} onActivate={vi.fn()} onClose={vi.fn()} onCloseOthers={vi.fn()} diff --git a/src/renderer/src/components/tab-bar/use-tab-bar-item-actions.ts b/src/renderer/src/components/tab-bar/use-tab-bar-item-actions.ts new file mode 100644 index 00000000000..c404a075461 --- /dev/null +++ b/src/renderer/src/components/tab-bar/use-tab-bar-item-actions.ts @@ -0,0 +1,92 @@ +import { useLayoutEffect, useMemo, useRef } from 'react' +import { clearClientHostedBrowserRowSelection } from '@/lib/pane-manager/client-hosted-browser-row-state' +import type { TabBarItem } from './tab-bar-item-model' +import type { TabBarProps } from './tab-bar-props' + +type TabBarItemActionSource = { + props: Pick< + TabBarProps, + | 'onActivate' + | 'onActivateFile' + | 'onActivateBrowserTab' + | 'onActivateAgentSession' + | 'onClose' + | 'onCloseFile' + | 'onCloseBrowserTab' + | 'onCloseOthers' + | 'onCloseToRight' + | 'onCloseToLeft' + | 'onCloseAllFiles' + | 'onSetCustomTitle' + | 'onSetTabColor' + | 'onTogglePaneExpand' + | 'onDuplicateBrowserTab' + | 'onMakePreviewFilePermanent' + > + togglePinned: (item: TabBarItem) => void + toggleTabViewMode: (tabId: string) => void +} + +export type TabBarItemActions = { + activateTerminal: (tabId: string) => void + activateFile: (fileId: string) => void + activateBrowserTab: (tabId: string) => void + activateAgentSession: (tabId: string) => void + close: (tabId: string) => void + closeFile: (fileId: string) => void + closeBrowserTab: (tabId: string) => void + closeOthers: (tabId: string) => void + closeToRight: (tabId: string) => void + closeToLeft: (tabId: string) => void + closeAllFiles: () => void + setCustomTitle: (tabId: string, title: string | null) => void + setTabColor: (tabId: string, color: string | null) => void + togglePaneExpand: (tabId: string) => void + duplicateBrowserTab: (tabId: string, unifiedTabId: string) => void + makePreviewFilePermanent: (fileId: string, tabId?: string) => void + togglePinned: (item: TabBarItem) => void + toggleViewMode: (tabId: string) => void +} + +/** Actions that read the strip's handlers when called, so their own identity never has to change. */ +export function useTabBarItemActions(source: TabBarItemActionSource): TabBarItemActions { + const latest = useRef(source) + // Why an effect: a render React abandons must not hand its handlers to the tabs already on screen. + useLayoutEffect(() => { + latest.current = source + }) + return useMemo(() => { + // Why: this is the strip's single activation fan-out, so retiring a client-hosted placeholder + // here covers every row kind — including re-clicking the tab that was already active, which the + // group's activeTabId never moves for. + function activateRealTab(activate: () => void): void { + clearClientHostedBrowserRowSelection() + activate() + } + return { + activateTerminal: (tabId) => activateRealTab(() => latest.current.props.onActivate(tabId)), + activateFile: (fileId) => + activateRealTab(() => latest.current.props.onActivateFile?.(fileId)), + activateBrowserTab: (tabId) => + activateRealTab(() => latest.current.props.onActivateBrowserTab?.(tabId)), + activateAgentSession: (tabId) => + activateRealTab(() => latest.current.props.onActivateAgentSession?.(tabId)), + close: (tabId) => latest.current.props.onClose(tabId), + closeFile: (fileId) => latest.current.props.onCloseFile?.(fileId), + closeBrowserTab: (tabId) => latest.current.props.onCloseBrowserTab?.(tabId), + closeOthers: (tabId) => latest.current.props.onCloseOthers(tabId), + closeToRight: (tabId) => latest.current.props.onCloseToRight(tabId), + closeToLeft: (tabId) => latest.current.props.onCloseToLeft(tabId), + closeAllFiles: () => latest.current.props.onCloseAllFiles?.(), + setCustomTitle: (tabId, title) => latest.current.props.onSetCustomTitle(tabId, title), + setTabColor: (tabId, color) => latest.current.props.onSetTabColor(tabId, color), + togglePaneExpand: (tabId) => latest.current.props.onTogglePaneExpand(tabId), + duplicateBrowserTab: (tabId, unifiedTabId) => + latest.current.props.onDuplicateBrowserTab?.(tabId, unifiedTabId), + makePreviewFilePermanent: (fileId, tabId) => + latest.current.props.onMakePreviewFilePermanent?.(fileId, tabId), + togglePinned: (item) => latest.current.togglePinned(item), + toggleViewMode: (tabId) => latest.current.toggleTabViewMode(tabId) + } + }, []) +} diff --git a/src/renderer/src/components/tab-bar/use-tab-bar-item-projection.test.tsx b/src/renderer/src/components/tab-bar/use-tab-bar-item-projection.test.tsx new file mode 100644 index 00000000000..332b13473b9 --- /dev/null +++ b/src/renderer/src/components/tab-bar/use-tab-bar-item-projection.test.tsx @@ -0,0 +1,95 @@ +// @vitest-environment happy-dom + +import { renderHook } from '@testing-library/react' +import { describe, expect, it } from 'vitest' +import type { TerminalTab } from '../../../../shared/terminal-tab-types' +import type { TabBarProps } from './tab-bar-props' +import { useTabBarItemProjection } from './use-tab-bar-item-projection' + +const NOOP = (): void => {} + +function terminalTab(id: string, sortOrder: number): TerminalTab & { unifiedTabId?: string } { + return { + id, + unifiedTabId: `unified-${id}`, + ptyId: null, + worktreeId: 'wt-1', + title: 'zsh', + customTitle: null, + color: null, + sortOrder, + createdAt: 0 + } +} + +/** Rebuilt per call, like the store's tab list: equal content, never the same objects. */ +function buildProps(ids: string[]): TabBarProps { + return { + tabs: ids.map((id, index) => terminalTab(id, index)), + tabBarOrder: [...ids], + activeTabId: ids[0] ?? null, + activeTabType: 'terminal', + worktreeId: 'wt-1', + expandedPaneByTabId: {}, + onActivate: NOOP, + onClose: NOOP, + onCloseOthers: NOOP, + onCloseToRight: NOOP, + onCloseToLeft: NOOP, + onNewTerminalTab: NOOP, + onNewBrowserTab: NOOP, + onSetCustomTitle: NOOP, + onSetTabColor: NOOP, + onTogglePaneExpand: NOOP + } +} + +function renderProjection(initialIds: string[]): { + sortableIds: () => string[] + rerenderWith: (ids: string[]) => void +} { + const { result, rerender } = renderHook( + (ids: string[]) => + useTabBarItemProjection({ + props: buildProps(ids), + resolvedGroupId: 'group-1', + unifiedTabs: [], + unifiedTabByVisibleId: new Map(), + generatedTabTitlesEnabled: false, + statusByRelativePath: new Map() + }), + { initialProps: initialIds } + ) + return { + sortableIds: () => result.current.sortableIds, + rerenderWith: (ids) => rerender(ids) + } +} + +describe('tab strip sortable ids', () => { + it('hands dnd-kit the same array while the ids are unchanged', () => { + const projection = renderProjection(['term-1', 'term-2']) + const first = projection.sortableIds() + + projection.rerenderWith(['term-1', 'term-2']) + + // A new array here re-renders every tab through dnd-kit's context, even with identical ids. + expect(projection.sortableIds()).toBe(first) + }) + + it('hands dnd-kit a new array when a tab is added', () => { + const projection = renderProjection(['term-1', 'term-2']) + + projection.rerenderWith(['term-1', 'term-2', 'term-3']) + + expect(projection.sortableIds()).toEqual(['term-1', 'term-2', 'term-3']) + }) + + it('hands dnd-kit a new array when the tabs are reordered', () => { + const projection = renderProjection(['term-1', 'term-2']) + + projection.rerenderWith(['term-2', 'term-1']) + + expect(projection.sortableIds()).toEqual(['term-2', 'term-1']) + }) +}) diff --git a/src/renderer/src/components/tab-bar/use-tab-bar-item-projection.ts b/src/renderer/src/components/tab-bar/use-tab-bar-item-projection.ts index f48c3ab14c3..8a2f5a58157 100644 --- a/src/renderer/src/components/tab-bar/use-tab-bar-item-projection.ts +++ b/src/renderer/src/components/tab-bar/use-tab-bar-item-projection.ts @@ -1,4 +1,4 @@ -import { useMemo } from 'react' +import { useMemo, useState } from 'react' import type { GitFileStatus } from '../../../../shared/git-status-types' import type { Tab } from '../../../../shared/tab-types' import type { TabBarProps } from './tab-bar-props' @@ -10,6 +10,7 @@ import { type TabBarItem } from './tab-bar-item-model' import type { DropIndicator } from './drop-indicator' +import { sameStringArray } from '@/runtime/web-session-tabs-sync/state-equality-core' export type TabBarItemProjection = { orderedItems: TabBarItem[] @@ -107,7 +108,12 @@ export function useTabBarItemProjection({ unifiedTabByVisibleId ] ) - const sortableIds = useMemo(() => orderedItems.map((item) => item.id), [orderedItems]) + const orderedIds = useMemo(() => orderedItems.map((item) => item.id), [orderedItems]) + // Why: dnd-kit re-renders every tab when this array's identity changes, and the items rebuild on any tab write. + const [sortableIds, setSortableIds] = useState(orderedIds) + if (!sameStringArray(sortableIds, orderedIds)) { + setSortableIds(orderedIds) + } const activeIndicator = hoveredTabInsertion?.groupId === resolvedGroupId ? hoveredTabInsertion : null const dropIndicatorByVisibleId = useMemo( diff --git a/src/renderer/src/components/tab-group/useTabDragSplit.test.ts b/src/renderer/src/components/tab-group/useTabDragSplit.test.ts index d9730fadbcc..a9f14d57bbc 100644 --- a/src/renderer/src/components/tab-group/useTabDragSplit.test.ts +++ b/src/renderer/src/components/tab-group/useTabDragSplit.test.ts @@ -271,6 +271,26 @@ describe('canDropTabIntoPaneBody', () => { }) describe('useTabDragSplit', () => { + it('keeps the drag sensors across re-renders until enablement changes', () => { + const sensorsByRender: ReturnType['sensors'][] = [] + function Probe({ enabled }: { enabled: boolean }): null { + sensorsByRender.push(useTabDragSplit({ worktreeId: WT, enabled }).sensors) + return null + } + const container = document.createElement('div') + document.body.appendChild(container) + const root = createRoot(container) + mounted.push({ container, root }) + + act(() => root.render(createElement(Probe, { enabled: true }))) + act(() => root.render(createElement(Probe, { enabled: true }))) + // Why: new sensors rebuild every tab's drag listeners, which re-renders every tab. + expect(sensorsByRender.at(-1)).toBe(sensorsByRender.at(-2)) + + act(() => root.render(createElement(Probe, { enabled: false }))) + expect(sensorsByRender.at(-1)).not.toBe(sensorsByRender.at(-2)) + }) + it.each(['split', 'insertion'])( 'does not restore a %s preview after blur cleared the drag', async (preview) => { diff --git a/src/renderer/src/components/tab-group/useTabDragSplit.ts b/src/renderer/src/components/tab-group/useTabDragSplit.ts index ca1724e2bf4..f67712622ab 100644 --- a/src/renderer/src/components/tab-group/useTabDragSplit.ts +++ b/src/renderer/src/components/tab-group/useTabDragSplit.ts @@ -1,4 +1,4 @@ -import { useCallback, useRef, useState, type RefObject } from 'react' +import { useCallback, useMemo, useRef, useState, type RefObject } from 'react' import { closestCenter, pointerWithin, @@ -131,9 +131,13 @@ export function useTabDragSplit({ // useSensors(ptr) / useSensors(), because dnd-kit internally spreads // the sensors array into a useEffect dependency list — changing its // length between renders violates React's rules of hooks. - const pointerSensor = useSensor(TabDragPointerSensor, { - activationConstraint: { distance: getTabDragActivationDistance(enabled) } - }) + const activationDistance = getTabDragActivationDistance(enabled) + // Why memoized: fresh options rebuild every tab's drag listeners and wake every tab through dnd-kit's context. + const pointerSensorOptions = useMemo( + () => ({ activationConstraint: { distance: activationDistance } }), + [activationDistance] + ) + const pointerSensor = useSensor(TabDragPointerSensor, pointerSensorOptions) const sensors = useSensors(pointerSensor) const clearDragState = useCallback(() => { diff --git a/src/renderer/src/runtime/web-session-tabs-sync-mirror-identity.test.ts b/src/renderer/src/runtime/web-session-tabs-sync-mirror-identity.test.ts index e8b8f8d7d43..c2210640c21 100644 --- a/src/renderer/src/runtime/web-session-tabs-sync-mirror-identity.test.ts +++ b/src/renderer/src/runtime/web-session-tabs-sync-mirror-identity.test.ts @@ -1,5 +1,6 @@ import { beforeEach, describe, expect, it } from 'vitest' import { createStore } from 'zustand/vanilla' +import { shallow } from 'zustand/shallow' import type { RuntimeMobileSessionTabsResult } from '../../../shared/runtime-types' import { toWebTerminalSurfaceTabId } from '../../../shared/terminal-surface-id' import type { @@ -304,6 +305,37 @@ describe('remote mirror resource identity', () => { expect(next.browserCertificateFailuresByPageId).toBe(state.browserCertificateFailuresByPageId) }) + it('keeps an unchanged browser tab record when a sibling browser tab changes', () => { + const browserTab = (index: number, title: string) => ({ + type: 'browser' as const, + id: `host-browser-tab-${index}`, + browserWorkspaceId: `host-browser-workspace-${index}`, + browserPageId: `host-browser-page-${index}`, + title, + url: `https://example.com/${index}`, + loading: false, + canGoBack: false, + canGoForward: false, + certificateFailure: null, + isActive: index === 1 + }) + const state = applySnapshot( + makeState(), + makeSnapshot(WORKTREE_A, [browserTab(1, 'One'), browserTab(2, 'Two')], 'browser') + ) + const next = applySnapshot( + state, + makeSnapshot(WORKTREE_A, [browserTab(1, 'One'), browserTab(2, 'Renamed')], 'browser'), + NOW + 1 + ) + + const [unchanged, renamed] = next.browserTabsByWorktree[WORKTREE_A]! + const [previous] = state.browserTabsByWorktree[WORKTREE_A]! + expect(renamed!.title).toBe('Renamed') + // Why: the tab strip compares each record field by field, so a rebuilt page list re-renders the tab. + expect(shallow(unchanged, previous)).toBe(true) + }) + it('clears a same-page certificate failure without replacing its page or handle', () => { const state = applySnapshot(makeState(), makeBrowserSnapshot()) const next = applySnapshot(state, makeBrowserSnapshot({ certificateFailure: null }), NOW + 1) diff --git a/src/renderer/src/runtime/web-session-tabs-sync/mirrored-browser-tabs.ts b/src/renderer/src/runtime/web-session-tabs-sync/mirrored-browser-tabs.ts index 8464ba504d7..c4a27812da9 100644 --- a/src/renderer/src/runtime/web-session-tabs-sync/mirrored-browser-tabs.ts +++ b/src/renderer/src/runtime/web-session-tabs-sync/mirrored-browser-tabs.ts @@ -8,6 +8,7 @@ import type { WebSessionTabsSyncState, MirroredBrowserTab } from './state' import { readBrowserClientHostId } from '../browser-client-host-identity' import { peekWebSessionBrowserPlacementGroup } from '../web-session-browser-placement' import { browserPageEqual } from './state-equality-tabs' +import { sameStringArray } from './state-equality-core' import { collectLayoutGroupIds } from './tab-group-layout-tree' import { buildBrowserUnifiedTab } from './tab-builders' import { isReadyBrowserTab } from './terminal-surfaces' @@ -201,13 +202,18 @@ export function buildMirroredBrowserTabs( // Why: reuse hinges on browserPageEqual comparing workspaceId — the removed-workspace // page-list cleanup gates on page.workspaceId matching this entry's workspace.id. const page = existing && browserPageEqual(existing.page, nextPage) ? existing.page : nextPage + const existingPageIds = existing?.workspace.pageIds const workspace: BrowserWorkspace = { id: workspaceId, worktreeId: snapshot.worktree, label: existing?.workspace.label, sessionProfileId: existing?.workspace.sessionProfileId ?? null, activePageId: page.id, - pageIds: [page.id], + // Why: the tab strip skips a tab only while every field keeps its identity. + pageIds: + existingPageIds && sameStringArray(existingPageIds, [page.id]) + ? existingPageIds + : [page.id], url: page.url, title: page.title, loading: page.loading, diff --git a/tests/e2e/helpers/runtime-types.ts b/tests/e2e/helpers/runtime-types.ts index 0d4b864ee64..207d4a4255b 100644 --- a/tests/e2e/helpers/runtime-types.ts +++ b/tests/e2e/helpers/runtime-types.ts @@ -13,6 +13,7 @@ import type { WorkspaceVisibleTabType } from '../../../src/shared/tab-types' import type { TerminalTab } from '../../../src/shared/terminal-tab-types' import type { Worktree } from '../../../src/shared/worktree/types' import type { DictationMeterState } from '../../../src/renderer/src/components/dictation/dictation-audio-meter' +import type { ReactCommitHook } from './tab-render-recorder' // Why: window.__store is the Zustand bound store itself, so specs get the whole StoreApi. export type AppStore = { @@ -68,6 +69,8 @@ declare global { __store?: AppStore __dictationMeterE2E?: { publish(meter: DictationMeterState): void } __paneManagers?: Map + __REACT_DEVTOOLS_GLOBAL_HOOK__?: ReactCommitHook + __tabsRenderedPerCommit?: number[] } } diff --git a/tests/e2e/helpers/tab-render-recorder.ts b/tests/e2e/helpers/tab-render-recorder.ts new file mode 100644 index 00000000000..83770b35aee --- /dev/null +++ b/tests/e2e/helpers/tab-render-recorder.ts @@ -0,0 +1,80 @@ +/** + * Counts how many tab-strip tabs each React commit re-renders, the way React DevTools highlights + * updates: through the commit hook the renderer always installs. + */ + +import type { Page } from '@stablyai/playwright-test' + +type Fiber = { + tag: number + flags: number + child: Fiber | null + sibling: Fiber | null + stateNode: unknown +} + +export type ReactCommitHook = { + onCommitFiberRoot?: (rendererId: unknown, root: { current: Fiber }, ...rest: unknown[]) => unknown +} + +/** Commits that render no tab (the sidebar, terminals starting up) are left out. */ +export async function startRecordingTabRenders(page: Page): Promise { + await page.evaluate(() => { + // Function, class, forwardRef and memo components; bit 1 is React's PerformedWork flag. + const componentTags = new Set([0, 1, 11, 14, 15]) + const performedWork = 1 + const hook = window.__REACT_DEVTOOLS_GLOBAL_HOOK__ + if (!hook) { + throw new Error('React commit hook is not installed') + } + const original = hook.onCommitFiberRoot + // A subtree React skipped keeps last commit's fiber objects, whose flags are stale. Per root: the renderer mounts several. + const previousFibersByRoot = new WeakMap>() + const recorded: number[] = [] + window.__tabsRenderedPerCommit = recorded + hook.onCommitFiberRoot = function (rendererId, root, ...rest) { + const previousFibers = previousFibersByRoot.get(root) + const fibers = new Set() + const renderedTabIds = new Set() + const stack: Fiber[] = [root.current] + while (stack.length > 0) { + const fiber = stack.pop()! + fibers.add(fiber) + if ( + componentTags.has(fiber.tag) && + (fiber.flags & performedWork) === performedWork && + !previousFibers?.has(fiber) + ) { + let host: Fiber | null = fiber + while (host && host.tag !== 5) { + host = host.child + } + const element = host?.stateNode + const tabId = + element instanceof Element + ? element.closest('[data-tab-strip-slot]')?.getAttribute('data-tab-strip-slot') + : undefined + if (tabId) { + renderedTabIds.add(tabId) + } + } + if (fiber.sibling) { + stack.push(fiber.sibling) + } + if (fiber.child) { + stack.push(fiber.child) + } + } + previousFibersByRoot.set(root, fibers) + if (renderedTabIds.size > 0) { + recorded.push(renderedTabIds.size) + } + return original?.call(this, rendererId, root, ...rest) + } + }) +} + +/** Tabs re-rendered by each commit since the last call. */ +export async function takeTabRenders(page: Page): Promise { + return page.evaluate(() => window.__tabsRenderedPerCommit?.splice(0) ?? []) +} diff --git a/tests/e2e/tab-strip-tab-render-isolation.spec.ts b/tests/e2e/tab-strip-tab-render-isolation.spec.ts new file mode 100644 index 00000000000..c1c41726d80 --- /dev/null +++ b/tests/e2e/tab-strip-tab-render-isolation.spec.ts @@ -0,0 +1,97 @@ +/** + * E2E test for a long tab strip: a change to one tab must re-render that tab, not the strip. + * + * Why E2E: only the whole app shows every React commit a tab change causes — the store write, the + * strip's projections, and the drag-and-drop context every tab reads. + */ + +import type { Page } from '@stablyai/playwright-test' +import { test, expect } from './helpers/orca-app' +import { waitForSessionReady, waitForActiveWorktree, ensureTerminalVisible } from './helpers/store' +import { waitForActivePanePtyId } from './helpers/terminal' +import { runNodeScriptInTerminal } from './helpers/run-node-script-in-terminal' +import { startRecordingTabRenders, takeTabRenders } from './helpers/tab-render-recorder' + +const BACKGROUND_TABS = 30 + +// New terminals retitle their tabs for a few seconds after opening; wait for the strip to go quiet. +async function waitForQuietStrip(page: Page): Promise { + await expect + .poll( + async () => { + await takeTabRenders(page) + await page.waitForTimeout(500) + return (await takeTabRenders(page)).length + }, + { timeout: 30_000 } + ) + .toBe(0) +} + +test.describe('Tab strip tab render isolation', () => { + test.beforeEach(async ({ orcaPage }) => { + await waitForSessionReady(orcaPage) + await waitForActiveWorktree(orcaPage) + await ensureTerminalVisible(orcaPage) + }) + + test('a title change or a tab switch re-renders only the tabs involved', async ({ orcaPage }) => { + const worktreeId = await waitForActiveWorktree(orcaPage) + const ptyId = await waitForActivePanePtyId(orcaPage) + const tabIds = await orcaPage.evaluate( + ({ wId, count }) => { + const ids: string[] = [] + for (let i = 0; i < count; i++) { + ids.push( + window.__store!.getState().createTab(wId, undefined, undefined, { activate: false }).id + ) + } + return ids + }, + { wId: worktreeId, count: BACKGROUND_TABS } + ) + const tab = (tabId: string) => + orcaPage.locator(`[data-testid="sortable-tab"][data-tab-id="${tabId}"]`) + await expect(tab(tabIds[BACKGROUND_TABS - 1])).toBeAttached() + await startRecordingTabRenders(orcaPage) + + await waitForQuietStrip(orcaPage) + await orcaPage.evaluate( + (tabId) => window.__store!.getState().updateTabTitle(tabId, 'retitled in background'), + tabIds[5] + ) + await expect(tab(tabIds[5])).toHaveAttribute('data-tab-title', 'retitled in background') + const backgroundRetitle = await takeTabRenders(orcaPage) + // Control: the recorder sees the one tab that did change. + expect(backgroundRetitle.length).toBeGreaterThan(0) + expect(Math.max(...backgroundRetitle)).toBe(1) + + await waitForQuietStrip(orcaPage) + // A node script, so the title is emitted the same way under PowerShell, cmd and POSIX shells. + // It stays alive until the title is read; a shell prompt would retitle the tab straight back. + const retitle = await runNodeScriptInTerminal( + orcaPage, + ptyId, + `process.stdout.write('\\x1b]0;retitled by its terminal\\x07'); setTimeout(() => {}, 3000)` + ) + try { + await expect( + orcaPage.locator('[data-testid="sortable-tab"][data-active="true"]') + ).toHaveAttribute('data-tab-title', 'retitled by its terminal') + const terminalRetitle = await takeTabRenders(orcaPage) + expect(terminalRetitle.length).toBeGreaterThan(0) + expect(Math.max(...terminalRetitle)).toBe(1) + } finally { + retitle.cleanup() + } + + await waitForQuietStrip(orcaPage) + await tab(tabIds[1]).click() + await expect(tab(tabIds[1])).toHaveAttribute('data-active', 'true') + await orcaPage.waitForTimeout(500) + const tabSwitch = await takeTabRenders(orcaPage) + expect(tabSwitch.length).toBeGreaterThan(0) + // The tab that lost the active state and the one that gained it. + expect(Math.max(...tabSwitch)).toBeLessThanOrEqual(2) + }) +})