mirror of
https://github.com/stablyai/orca.git
synced 2026-09-27 16:02:35 +00:00
Show close button for single terminal panes (#22770)
* Show close button for single terminal panes Single-pane terminals previously had no close affordance; now display "Close tab" button while multi-pane terminals show "Close Pane". Pinned tabs omit the close button. Refactored terminal-unified-tab-lookup to include tab pinned state alongside chat view fields. * Show close button for titled single terminal panes For split panes, the X button remains remove-title only. For single panes with titles (including agent terminals that acquire runtime titles), a close tab button is needed to close the pane.
This commit is contained in:
@@ -44,6 +44,7 @@ function renderOverlay({
|
||||
paneCount = 2,
|
||||
showAlwaysOnHeaders = true,
|
||||
showSplitButton = true,
|
||||
isTabPinned = false,
|
||||
onClosePane = vi.fn(),
|
||||
onRemoveTitle = vi.fn(),
|
||||
onRenameSubmit = vi.fn(),
|
||||
@@ -56,6 +57,7 @@ function renderOverlay({
|
||||
paneCount?: number
|
||||
showAlwaysOnHeaders?: boolean
|
||||
showSplitButton?: boolean
|
||||
isTabPinned?: boolean
|
||||
onClosePane?: ReturnType<typeof vi.fn>
|
||||
onRemoveTitle?: ReturnType<typeof vi.fn>
|
||||
onRenameSubmit?: ReturnType<typeof vi.fn>
|
||||
@@ -69,7 +71,7 @@ function renderOverlay({
|
||||
onRemoveTitle: ReturnType<typeof vi.fn>
|
||||
onRenameSubmit: ReturnType<typeof vi.fn>
|
||||
} {
|
||||
const panes = [makePane(1), makePane(2)]
|
||||
const panes = [makePane(1), makePane(2)].slice(0, paneCount)
|
||||
const container = document.createElement('div')
|
||||
document.body.appendChild(container)
|
||||
const root = createRoot(container)
|
||||
@@ -81,6 +83,7 @@ function renderOverlay({
|
||||
cwd={path.join(path.sep, 'tmp')}
|
||||
showAlwaysOnHeaders={showAlwaysOnHeaders}
|
||||
showSplitButton={showSplitButton}
|
||||
isTabPinned={isTabPinned}
|
||||
paneCount={paneCount}
|
||||
activePaneId={1}
|
||||
panes={panes}
|
||||
@@ -145,7 +148,7 @@ afterEach(() => {
|
||||
})
|
||||
|
||||
describe('TerminalPaneHeaderOverlay', () => {
|
||||
it('keeps the titled-pane close affordance as remove-title while headers are always on', () => {
|
||||
it('keeps the titled split-pane X as remove-title only', () => {
|
||||
const { container, onClosePane, onRemoveTitle } = renderOverlay({
|
||||
paneTitles: { 1: 'server', 2: '' }
|
||||
})
|
||||
@@ -154,15 +157,34 @@ describe('TerminalPaneHeaderOverlay', () => {
|
||||
'button[aria-label="Remove pane title: server"]'
|
||||
)
|
||||
expect(removeTitle).not.toBeNull()
|
||||
expect(
|
||||
container.querySelector('.pane-title-bar[data-active-pane] button[aria-label="Close Pane"]')
|
||||
).toBeNull()
|
||||
|
||||
act(() => removeTitle?.click())
|
||||
|
||||
expect(onRemoveTitle).toHaveBeenCalledWith(1)
|
||||
expect(onClosePane).not.toHaveBeenCalledWith(1)
|
||||
expect(onClosePane).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('offers close tab beside remove-title for a titled single pane', () => {
|
||||
const { container, onClosePane, onRemoveTitle } = renderOverlay({
|
||||
paneTitles: { 1: 'server' },
|
||||
paneCount: 1
|
||||
})
|
||||
|
||||
expect(container.querySelector('button[aria-label="Remove pane title: server"]')).not.toBeNull()
|
||||
const closeTab = container.querySelector<HTMLButtonElement>('button[aria-label="Close tab"]')
|
||||
expect(closeTab).not.toBeNull()
|
||||
|
||||
act(() => closeTab?.click())
|
||||
|
||||
expect(onClosePane).toHaveBeenCalledWith(1)
|
||||
expect(onRemoveTitle).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('keeps split and close-pane controls available for untitled split pane headers', () => {
|
||||
const { container, onClosePane, onRemoveTitle } = renderOverlay({
|
||||
const { container, onClosePane } = renderOverlay({
|
||||
paneTitles: { 1: '', 2: '' }
|
||||
})
|
||||
|
||||
@@ -174,7 +196,31 @@ describe('TerminalPaneHeaderOverlay', () => {
|
||||
act(() => closePane?.click())
|
||||
|
||||
expect(onClosePane).toHaveBeenCalledWith(1)
|
||||
expect(onRemoveTitle).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('offers close tab for an untitled single pane', () => {
|
||||
const { container, onClosePane } = renderOverlay({ paneTitles: { 1: '' }, paneCount: 1 })
|
||||
|
||||
const closeTab = container.querySelector<HTMLButtonElement>('button[aria-label="Close tab"]')
|
||||
expect(closeTab).not.toBeNull()
|
||||
expect(container.querySelector('button[aria-label="Close Pane"]')).toBeNull()
|
||||
|
||||
act(() => closeTab?.click())
|
||||
|
||||
expect(onClosePane).toHaveBeenCalledWith(1)
|
||||
})
|
||||
|
||||
it.each([
|
||||
{ label: 'untitled', title: '' },
|
||||
{ label: 'titled', title: 'server' }
|
||||
])('keeps a pinned $label single-pane tab without a close button', ({ title }) => {
|
||||
const { container } = renderOverlay({
|
||||
paneTitles: { 1: title },
|
||||
paneCount: 1,
|
||||
isTabPinned: true
|
||||
})
|
||||
|
||||
expect(container.querySelector('button[aria-label="Close tab"]')).toBeNull()
|
||||
})
|
||||
|
||||
it('omits the split control when the header affordance is hidden', () => {
|
||||
@@ -185,6 +231,7 @@ describe('TerminalPaneHeaderOverlay', () => {
|
||||
})
|
||||
|
||||
expect(container.querySelector('button[aria-label="Split Terminal Right"]')).toBeNull()
|
||||
expect(container.querySelector('button[aria-label="Close tab"]')).toBeNull()
|
||||
})
|
||||
|
||||
it('ignores IME composition Enter before submitting a pane title rename', () => {
|
||||
|
||||
@@ -28,6 +28,7 @@ type TerminalPaneHeaderOverlayProps = {
|
||||
showAlwaysOnHeaders: boolean
|
||||
/** Used by ephemeral one-off command terminals that omit the header affordance. */
|
||||
showSplitButton?: boolean
|
||||
isTabPinned: boolean
|
||||
paneCount: number
|
||||
activePaneId: number | null | undefined
|
||||
panes: readonly ManagedPane[]
|
||||
@@ -73,6 +74,7 @@ export default function TerminalPaneHeaderOverlay({
|
||||
cwd,
|
||||
showAlwaysOnHeaders,
|
||||
showSplitButton = true,
|
||||
isTabPinned,
|
||||
paneCount,
|
||||
activePaneId,
|
||||
panes,
|
||||
@@ -126,6 +128,17 @@ export default function TerminalPaneHeaderOverlay({
|
||||
const isActivePane = activePaneId === pane.id
|
||||
const isChromeless = showAlwaysOnHeaders && !title && !isEditing
|
||||
const showHeader = overlayRect && (showAlwaysOnHeaders || Boolean(title) || isEditing)
|
||||
const closeLabel =
|
||||
paneCount > 1
|
||||
? translate(
|
||||
'auto.components.terminal.pane.TerminalContextMenu.8c17d6786d',
|
||||
'Close Pane'
|
||||
)
|
||||
: translate('auto.components.tab.bar.SortableTab.95db5f2f7d', 'Close tab')
|
||||
// Why: a titled split pane keeps its X as remove-title, but a titled single
|
||||
// pane (agent terminals get runtime titles) still needs a close control.
|
||||
const showCloseButton =
|
||||
showAlwaysOnHeaders && (paneCount > 1 ? !title : showSplitButton && !isTabPinned)
|
||||
if (!showHeader || !overlayRect) {
|
||||
return null
|
||||
}
|
||||
@@ -368,7 +381,8 @@ export default function TerminalPaneHeaderOverlay({
|
||||
)}
|
||||
</TooltipContent>
|
||||
</Tooltip>
|
||||
) : paneCount > 1 && showAlwaysOnHeaders ? (
|
||||
) : null}
|
||||
{showCloseButton ? (
|
||||
<Tooltip>
|
||||
<TooltipTrigger asChild>
|
||||
<Button
|
||||
@@ -380,19 +394,13 @@ export default function TerminalPaneHeaderOverlay({
|
||||
event.stopPropagation()
|
||||
onClosePane(pane.id)
|
||||
}}
|
||||
aria-label={translate(
|
||||
'auto.components.terminal.pane.TerminalContextMenu.8c17d6786d',
|
||||
'Close Pane'
|
||||
)}
|
||||
aria-label={closeLabel}
|
||||
>
|
||||
<X className="size-3" />
|
||||
</Button>
|
||||
</TooltipTrigger>
|
||||
<TooltipContent side="bottom" sideOffset={4}>
|
||||
{translate(
|
||||
'auto.components.terminal.pane.TerminalContextMenu.8c17d6786d',
|
||||
'Close Pane'
|
||||
)}
|
||||
{closeLabel}
|
||||
</TooltipContent>
|
||||
</Tooltip>
|
||||
) : null}
|
||||
|
||||
@@ -63,6 +63,7 @@ export function TerminalPaneSurface({
|
||||
handleToggleNativeChat,
|
||||
hiddenStartupStyle,
|
||||
isActive,
|
||||
isTabPinned,
|
||||
keybindings,
|
||||
managedPanes,
|
||||
managerRef,
|
||||
@@ -302,6 +303,7 @@ export function TerminalPaneSurface({
|
||||
cwd={cwd ?? ''}
|
||||
showAlwaysOnHeaders={isActive && terminalContentVisible}
|
||||
showSplitButton={showSplitButton}
|
||||
isTabPinned={isTabPinned}
|
||||
paneCount={paneCount}
|
||||
activePaneId={activePane?.id}
|
||||
panes={managedPanes}
|
||||
|
||||
@@ -2,7 +2,8 @@ import { describe, expect, it, vi } from 'vitest'
|
||||
import type { Tab } from '../../../../shared/tab-types'
|
||||
import {
|
||||
getCachedTerminalGroupIdForWorktree,
|
||||
getCachedUnifiedTerminalTabForWorktree
|
||||
getCachedUnifiedTerminalTabForWorktree,
|
||||
selectUnifiedTerminalTabFields
|
||||
} from './terminal-unified-tab-lookup'
|
||||
|
||||
function makeTerminalTab(entityId: string, groupId: string): Tab {
|
||||
@@ -95,4 +96,15 @@ describe('terminal unified tab lookup', () => {
|
||||
expect(first.iterator).toHaveBeenCalledTimes(1)
|
||||
expect(second.iterator).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('reads pinned state from the selected terminal tab after it changes', () => {
|
||||
const tab = { ...makeTerminalTab('terminal-1', 'group-a'), id: 'unified-1' }
|
||||
expect(
|
||||
selectUnifiedTerminalTabFields({ 'wt-1': [tab] }, 'wt-1', tab.entityId).isTabPinned
|
||||
).toBe(false)
|
||||
expect(
|
||||
selectUnifiedTerminalTabFields({ 'wt-1': [{ ...tab, isPinned: true }] }, 'wt-1', tab.entityId)
|
||||
.isTabPinned
|
||||
).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,9 +1,10 @@
|
||||
import type { Tab } from '../../../../shared/tab-types'
|
||||
|
||||
export type UnifiedTerminalTabChatFields = {
|
||||
export type UnifiedTerminalTabFields = {
|
||||
unifiedTabId: string | undefined
|
||||
isChatViewMode: boolean
|
||||
unifiedTabLabel: string | undefined
|
||||
isTabPinned: boolean
|
||||
}
|
||||
|
||||
const terminalTabLookupByUnifiedTabs = new WeakMap<readonly Tab[], Map<string, Tab>>()
|
||||
@@ -46,16 +47,16 @@ export function getCachedTerminalGroupIdForWorktree(
|
||||
}
|
||||
|
||||
/**
|
||||
* The unified-tab fields TerminalPane's chat state reads.
|
||||
* The unified-tab fields TerminalPane reads.
|
||||
*
|
||||
* Why bundled: they used to be five `useAppStore` calls, so one publication paid
|
||||
* the lookup five times and held five listener slots for every mounted tab.
|
||||
*/
|
||||
export function selectUnifiedTerminalTabChatFields(
|
||||
export function selectUnifiedTerminalTabFields(
|
||||
unifiedTabsByWorktree: Record<string, Tab[]>,
|
||||
worktreeId: string,
|
||||
terminalTabId: string
|
||||
): UnifiedTerminalTabChatFields {
|
||||
): UnifiedTerminalTabFields {
|
||||
const tab = getCachedUnifiedTerminalTabForWorktree(
|
||||
unifiedTabsByWorktree,
|
||||
worktreeId,
|
||||
@@ -64,6 +65,7 @@ export function selectUnifiedTerminalTabChatFields(
|
||||
return {
|
||||
unifiedTabId: tab?.id,
|
||||
isChatViewMode: tab?.viewMode === 'chat',
|
||||
unifiedTabLabel: tab?.label
|
||||
unifiedTabLabel: tab?.label,
|
||||
isTabPinned: tab?.isPinned === true
|
||||
}
|
||||
}
|
||||
|
||||
@@ -7,7 +7,7 @@ import { collectLeafIdsInOrder, EMPTY_LAYOUT } from './layout-serialization'
|
||||
import { sanitizeTerminalLayoutPaneTitles } from '@/lib/terminal-pane-title-sanitization'
|
||||
import { resolveNativeChatLeafTitleAgent } from './native-chat-leaf-title-agent'
|
||||
import { useTerminalPaneStoreActions } from './use-terminal-pane-store-actions'
|
||||
import { selectUnifiedTerminalTabChatFields } from './terminal-unified-tab-lookup'
|
||||
import { selectUnifiedTerminalTabFields } from './terminal-unified-tab-lookup'
|
||||
import { canToggleNativeChat } from '../native-chat/native-chat-availability'
|
||||
import {
|
||||
nativeChatLaunchAgentForLeaf,
|
||||
@@ -41,9 +41,9 @@ export function useTerminalPaneChatState(controller: TerminalPaneTitleController
|
||||
const pendingCodexPaneRestartIds = useAppStore((store) => store.pendingCodexPaneRestartIds)
|
||||
// Why one selector: five separate subscriptions each re-read the same unified
|
||||
// tab, so one publication paid the lookup five times per mounted tab.
|
||||
const { unifiedTabId, isChatViewMode, unifiedTabLabel } = useAppStore(
|
||||
const { unifiedTabId, isChatViewMode, unifiedTabLabel, isTabPinned } = useAppStore(
|
||||
useShallow((store) =>
|
||||
selectUnifiedTerminalTabChatFields(store.unifiedTabsByWorktree, worktreeId, tabId)
|
||||
selectUnifiedTerminalTabFields(store.unifiedTabsByWorktree, worktreeId, tabId)
|
||||
)
|
||||
)
|
||||
const nativeChatEnabled = useAppStore((store) => store.settings?.experimentalNativeChat === true)
|
||||
@@ -243,6 +243,7 @@ export function useTerminalPaneChatState(controller: TerminalPaneTitleController
|
||||
nativeChatEnabled,
|
||||
effectiveChatViewMode,
|
||||
unifiedTabLabel,
|
||||
isTabPinned,
|
||||
runtimePaneTitlesByPaneId,
|
||||
tabAgentTypeByLeaf,
|
||||
setTabViewMode,
|
||||
|
||||
Reference in New Issue
Block a user