diff --git a/mobile/src/session/MobileBrowserTabActionSheet.tsx b/mobile/src/session/MobileBrowserTabActionSheet.tsx index 3c2c9544386..5ded7bcfa11 100644 --- a/mobile/src/session/MobileBrowserTabActionSheet.tsx +++ b/mobile/src/session/MobileBrowserTabActionSheet.tsx @@ -13,7 +13,7 @@ export function MobileBrowserTabActionSheet(props: { onCloseTab: (target: BrowserTab) => void /** Rendered after Close — receives the open tab's id so the session route's * bulk-close builder can resolve the anchor itself. */ - bulkCloseActions?: (anchorTabId: string | undefined, dismiss: () => void) => ActionSheetAction[] + bulkCloseActions?: (anchorTabId: string | undefined) => ActionSheetAction[] }): React.JSX.Element { const { target, onClose, onNavigate, onCloseTab, bulkCloseActions } = props return ( @@ -73,7 +73,7 @@ export function MobileBrowserTabActionSheet(props: { } } }, - ...(bulkCloseActions?.(target?.id, onClose) ?? []) + ...(bulkCloseActions?.(target?.id) ?? []) ]} onClose={onClose} /> diff --git a/mobile/src/session/MobileSessionSheets.tsx b/mobile/src/session/MobileSessionSheets.tsx index 39d2b1feb53..9c100e9e154 100644 --- a/mobile/src/session/MobileSessionSheets.tsx +++ b/mobile/src/session/MobileSessionSheets.tsx @@ -86,6 +86,7 @@ export function MobileSessionSheets({ controller }: { controller: MobileSessionC handleCloseTerminal, handleCloseSessionTab, bulkCloseActions, + bulkCloseConfirm, closeWithBulkActions, createTabAgentActions, sendDiffNotesAgentActions, @@ -333,6 +334,7 @@ export function MobileSessionSheets({ controller }: { controller: MobileSessionC onConfirm={confirmDiscardMarkdown} onCancel={() => setDiscardMarkdownTarget(null)} /> + ({ + type: 'terminal', + id, + title: id, + terminal: `term-${id}`, + isActive: false +}) + +describe('Close Other Tabs', () => { + it('closes the other tabs only after the user confirms', async () => { + let answer: (confirmed: boolean) => void = () => {} + const closeSessionTab = vi.fn((_tab: MobileSessionTab) => Promise.resolve()) + const [action] = createBulkCloseSheetActions({ + sessionTabsRef: { current: [terminal('a'), terminal('b'), terminal('c')] }, + markdownDocs: new Map(), + activeSessionTabIdRef: { current: 'a' }, + switchSessionTab: vi.fn(), + closeSessionTab, + confirmBulkClose: () => + new Promise((resolve) => { + answer = resolve + }) + })('a') + + action.onPress() + expect(closeSessionTab).not.toHaveBeenCalled() + answer(true) + await vi.waitFor(() => expect(closeSessionTab).toHaveBeenCalledTimes(2)) + expect(closeSessionTab.mock.calls.map(([tab]) => tab.id)).toEqual(['b', 'c']) + }) +}) diff --git a/mobile/src/session/mobile-bulk-close-sheet-actions.ts b/mobile/src/session/mobile-bulk-close-sheet-actions.ts index d8261ce03d0..8281a337337 100644 --- a/mobile/src/session/mobile-bulk-close-sheet-actions.ts +++ b/mobile/src/session/mobile-bulk-close-sheet-actions.ts @@ -1,6 +1,7 @@ import type { MarkdownDocState, MobileSessionTab } from './mobile-session-route-types' import type { ActionSheetAction } from '../components/ActionSheetModal' import { selectBulkCloseTabs } from './mobile-tab-close-selection' +import { closeTabsAfterConfirmation } from '../../../src/shared/bulk-tab-close-confirmation' type BulkCloseSheetDeps = { sessionTabsRef: { readonly current: readonly MobileSessionTab[] } @@ -8,6 +9,7 @@ type BulkCloseSheetDeps = { activeSessionTabIdRef: { readonly current: string | null } switchSessionTab: (tab: MobileSessionTab) => void closeSessionTab: (tab: MobileSessionTab) => Promise + confirmBulkClose: () => Promise } // Pass tab ids so long-press sheets share bulk-close orchestration without retaining full tabs. @@ -23,8 +25,8 @@ export function createBulkCloseSheetActions(deps: BulkCloseSheetDeps) { return !(doc?.status === 'ready' && doc.isDirty) }) - const bulkClose = async (anchor: MobileSessionTab) => { - const targets = selectClosable(anchor.id) + const bulkClose = async (anchor: MobileSessionTab, tabIds: readonly string[]) => { + const targets = deps.sessionTabsRef.current.filter((candidate) => tabIds.includes(candidate.id)) const activeWasTargeted = targets.some( (candidate) => candidate.id === deps.activeSessionTabIdRef.current ) @@ -38,7 +40,7 @@ export function createBulkCloseSheetActions(deps: BulkCloseSheetDeps) { } } - return (anchorTabId: string | null | undefined, dismiss: () => void): ActionSheetAction[] => { + return (anchorTabId: string | null | undefined): ActionSheetAction[] => { const anchor = anchorTabId == null ? undefined @@ -50,9 +52,14 @@ export function createBulkCloseSheetActions(deps: BulkCloseSheetDeps) { { label: 'Close Other Tabs', destructive: true, + // Why: the confirm is its own native Modal; wait for this sheet to unmount (#10331). + closeBeforePress: true, onPress: () => { - dismiss() - void bulkClose(anchor) + void closeTabsAfterConfirmation( + () => selectClosable(anchor.id).map((candidate) => candidate.id), + deps.confirmBulkClose, + (tabIds) => void bulkClose(anchor, tabIds) + ) } } ] @@ -74,6 +81,6 @@ export function createCloseWithBulkActions( } } }, - ...bulkActions(target?.id, dismiss) + ...bulkActions(target?.id) ] } diff --git a/mobile/src/session/mobile-terminal-action-sheet-actions.ts b/mobile/src/session/mobile-terminal-action-sheet-actions.ts index 1ff3be043f4..ee432676ce6 100644 --- a/mobile/src/session/mobile-terminal-action-sheet-actions.ts +++ b/mobile/src/session/mobile-terminal-action-sheet-actions.ts @@ -23,7 +23,7 @@ export function getMobileTerminalActionSheetActions< onClose: (target: Target) => void /** Preferred path runs host teardown and records the local tombstone. */ onCloseSessionTab: (tab: Tab) => void - bulkCloseActions?: (anchorTabId: string | undefined, dismiss: () => void) => ActionSheetAction[] + bulkCloseActions?: (anchorTabId: string | undefined) => ActionSheetAction[] }): ActionSheetAction[] { const { target } = args if (!target) { @@ -75,6 +75,6 @@ export function getMobileTerminalActionSheetActions< args.onClose(target) } }, - ...(args.bulkCloseActions?.(sessionTab?.id, args.onDismiss) ?? []) + ...(args.bulkCloseActions?.(sessionTab?.id) ?? []) ] } diff --git a/mobile/src/session/use-mobile-session-bulk-close.ts b/mobile/src/session/use-mobile-session-bulk-close.ts index bad0d56238d..c9d86788fe7 100644 --- a/mobile/src/session/use-mobile-session-bulk-close.ts +++ b/mobile/src/session/use-mobile-session-bulk-close.ts @@ -1,9 +1,10 @@ -import { useEffect } from 'react' +import { useEffect, useState } from 'react' import { createBulkCloseSheetActions, createCloseWithBulkActions } from './mobile-bulk-close-sheet-actions' import type { RpcSuccess } from '../transport/types' +import { BULK_TAB_CLOSE_COPY } from '../../../src/shared/bulk-tab-close-confirmation' import { activateMobileSessionTab } from './mobile-session-tab-activation' import type { MobileSessionTab, SessionTabsResult } from './mobile-session-route-types' import type { MobileSessionCloseActionsModel } from './use-mobile-session-close-actions' @@ -28,12 +29,29 @@ export function useMobileSessionBulkClose(scope: MobileSessionCloseActionsModel) parkedPendingTerminalContext, retryPendingTerminalRecovery } = scope + const [answerBulkClose, setAnswerBulkClose] = useState<((confirmed: boolean) => void) | null>( + null + ) + const resolveBulkCloseConfirm = (confirmed: boolean) => { + answerBulkClose?.(confirmed) + setAnswerBulkClose(null) + } + const bulkCloseConfirm = { + visible: answerBulkClose !== null, + title: BULK_TAB_CLOSE_COPY.title.others, + message: BULK_TAB_CLOSE_COPY.description, + confirmLabel: BULK_TAB_CLOSE_COPY.confirm, + destructive: true, + onConfirm: () => resolveBulkCloseConfirm(true), + onCancel: () => resolveBulkCloseConfirm(false) + } const bulkCloseActions = createBulkCloseSheetActions({ sessionTabsRef, markdownDocs, activeSessionTabIdRef, switchSessionTab, - closeSessionTab: handleCloseSessionTab + closeSessionTab: handleCloseSessionTab, + confirmBulkClose: () => new Promise((resolve) => setAnswerBulkClose(() => resolve)) }) const closeWithBulkActions = createCloseWithBulkActions(handleCloseSessionTab, bulkCloseActions) @@ -97,6 +115,7 @@ export function useMobileSessionBulkClose(scope: MobileSessionCloseActionsModel) ]) return { bulkCloseActions, + bulkCloseConfirm, closeWithBulkActions, visibleTabs, activeMarkdownTab, diff --git a/src/renderer/src/components/tab-group/useTabGroupCloseScopeCommands.test.tsx b/src/renderer/src/components/tab-group/useTabGroupCloseScopeCommands.test.tsx index 071cb884f08..467750a4f42 100644 --- a/src/renderer/src/components/tab-group/useTabGroupCloseScopeCommands.test.tsx +++ b/src/renderer/src/components/tab-group/useTabGroupCloseScopeCommands.test.tsx @@ -3,7 +3,7 @@ import { act, cleanup, renderHook } from '@testing-library/react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { useAppStore } from '@/store' import { makeTabGroup, makeUnifiedTab } from '../../store/slices/store-session-test-harness' -import type { BulkTabCloseScope } from '../use-bulk-tab-close-confirmation' +import type { BulkTabCloseScope } from '../../../../shared/bulk-tab-close-confirmation' import { useTabGroupCloseScopeCommands } from './useTabGroupCloseScopeCommands' const confirm = vi.hoisted(() => vi.fn<() => Promise>()) diff --git a/src/renderer/src/components/tab-group/useTabGroupCloseScopeCommands.ts b/src/renderer/src/components/tab-group/useTabGroupCloseScopeCommands.ts index 0f6615b040b..6f1d1448b49 100644 --- a/src/renderer/src/components/tab-group/useTabGroupCloseScopeCommands.ts +++ b/src/renderer/src/components/tab-group/useTabGroupCloseScopeCommands.ts @@ -2,10 +2,8 @@ import { useCallback } from 'react' import { useAppStore } from '../../store' import { captureWorkspaceEmptiedReaction } from './workspace-emptied-reaction' import { isEditorTabContentType } from '@/store/slices/editor/tabs/editor-tab-content-type' -import { - useBulkTabCloseConfirmation, - type BulkTabCloseScope -} from '../use-bulk-tab-close-confirmation' +import { useBulkTabCloseConfirmation } from '../use-bulk-tab-close-confirmation' +import type { BulkTabCloseScope } from '../../../../shared/bulk-tab-close-confirmation' export function useTabGroupCloseScopeCommands({ groupId, diff --git a/src/renderer/src/components/use-bulk-tab-close-confirmation.ts b/src/renderer/src/components/use-bulk-tab-close-confirmation.ts index bae7f8b71e3..15d152a4109 100644 --- a/src/renderer/src/components/use-bulk-tab-close-confirmation.ts +++ b/src/renderer/src/components/use-bulk-tab-close-confirmation.ts @@ -1,8 +1,11 @@ import { useCallback } from 'react' import { useConfirmationDialog } from './confirmation-dialog-context' import { translate } from '@/i18n/i18n' - -export type BulkTabCloseScope = 'others' | 'right' | 'left' | 'editors' +import { + BULK_TAB_CLOSE_COPY as COPY, + closeTabsAfterConfirmation, + type BulkTabCloseScope +} from '../../../shared/bulk-tab-close-confirmation' export function useBulkTabCloseConfirmation() { const confirm = useConfirmationDialog() @@ -12,35 +15,25 @@ export function useBulkTabCloseConfirmation() { selectTabIds: () => string[], closeTabs: (tabIds: string[]) => void ): void => { - const selectedIds = new Set(selectTabIds()) - if (selectedIds.size === 0) { - return - } - const titles = { - others: translate('components.tab.bar.bulkClose.others', 'Close other tabs?'), - right: translate('components.tab.bar.bulkClose.right', 'Close tabs to the right?'), - left: translate('components.tab.bar.bulkClose.left', 'Close tabs to the left?'), - editors: translate('components.tab.bar.bulkClose.editors', 'Close all editor tabs?') - } - void confirm({ - title: titles[scope], - description: translate( - 'components.tab.bar.bulkClose.description', - 'The selected tabs will be closed. Pinned tabs will stay open.' - ), - confirmLabel: translate('components.tab.bar.bulkClose.confirm', 'Close tabs'), - cancelVariant: 'ghost', - confirmVariant: 'destructive' - }).then((confirmed) => { - if (!confirmed) { - return - } - // Remote updates can pin, move, remove or add tabs while the dialog is open. - const remainingIds = selectTabIds().filter((id) => selectedIds.has(id)) - if (remainingIds.length > 0) { - closeTabs(remainingIds) - } - }) + void closeTabsAfterConfirmation( + selectTabIds, + () => { + const titles = { + others: translate('components.tab.bar.bulkClose.others', COPY.title.others), + right: translate('components.tab.bar.bulkClose.right', COPY.title.right), + left: translate('components.tab.bar.bulkClose.left', COPY.title.left), + editors: translate('components.tab.bar.bulkClose.editors', COPY.title.editors) + } + return confirm({ + title: titles[scope], + description: translate('components.tab.bar.bulkClose.description', COPY.description), + confirmLabel: translate('components.tab.bar.bulkClose.confirm', COPY.confirm), + cancelVariant: 'ghost', + confirmVariant: 'destructive' + }) + }, + closeTabs + ) }, [confirm] ) diff --git a/src/renderer/src/i18n/en-runtime-required.json b/src/renderer/src/i18n/en-runtime-required.json index a81ef5ae899..bc12057730e 100644 --- a/src/renderer/src/i18n/en-runtime-required.json +++ b/src/renderer/src/i18n/en-runtime-required.json @@ -3408,6 +3408,18 @@ } } }, + "tab": { + "bar": { + "bulkClose": { + "confirm": "Close tabs", + "description": "The selected tabs will be closed. Pinned tabs will stay open.", + "editors": "Close all editor tabs?", + "left": "Close tabs to the left?", + "others": "Close other tabs?", + "right": "Close tabs to the right?" + } + } + }, "workspace": { "cleanup": { "browse": { diff --git a/src/shared/bulk-tab-close-confirmation.ts b/src/shared/bulk-tab-close-confirmation.ts new file mode 100644 index 00000000000..f467f468697 --- /dev/null +++ b/src/shared/bulk-tab-close-confirmation.ts @@ -0,0 +1,30 @@ +// Desktop and mobile share when bulk tab close asks first and its English wording. + +export type BulkTabCloseScope = 'others' | 'right' | 'left' | 'editors' + +export const BULK_TAB_CLOSE_COPY = { + title: { + others: 'Close other tabs?', + right: 'Close tabs to the right?', + left: 'Close tabs to the left?', + editors: 'Close all editor tabs?' + }, + description: 'The selected tabs will be closed. Pinned tabs will stay open.', + confirm: 'Close tabs' +} as const + +export async function closeTabsAfterConfirmation( + selectTabIds: () => string[], + confirm: () => Promise, + closeTabs: (tabIds: string[]) => void +): Promise { + const selectedIds = new Set(selectTabIds()) + if (selectedIds.size === 0 || !(await confirm())) { + return + } + // Remote updates can pin, move, remove or add tabs while the dialog is open. + const remainingIds = selectTabIds().filter((id) => selectedIds.has(id)) + if (remainingIds.length > 0) { + closeTabs(remainingIds) + } +}