fix(mobile): ask before closing other tabs, sharing desktop's confirm (#33704)

This commit is contained in:
Kelvin Amoaba
2026-10-26 20:15:24 +00:00
committed by GitHub
parent 9608420517
commit d6567867e5
11 changed files with 144 additions and 48 deletions
@@ -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}
/>
@@ -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)}
/>
<ConfirmModal {...bulkCloseConfirm} />
<TextInputModal
visible={renameTarget != null}
title="Rename Terminal"
@@ -0,0 +1,35 @@
import { describe, expect, it, vi } from 'vitest'
import { createBulkCloseSheetActions } from './mobile-bulk-close-sheet-actions'
import type { MobileSessionTab } from './mobile-session-route-types'
const terminal = (id: string): MobileSessionTab => ({
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'])
})
})
@@ -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<void>
confirmBulkClose: () => Promise<boolean>
}
// 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)
]
}
@@ -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) ?? [])
]
}
@@ -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<boolean>((resolve) => setAnswerBulkClose(() => resolve))
})
const closeWithBulkActions = createCloseWithBulkActions(handleCloseSessionTab, bulkCloseActions)
@@ -97,6 +115,7 @@ export function useMobileSessionBulkClose(scope: MobileSessionCloseActionsModel)
])
return {
bulkCloseActions,
bulkCloseConfirm,
closeWithBulkActions,
visibleTabs,
activeMarkdownTab,
@@ -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<boolean>>())
@@ -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,
@@ -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]
)
+12
View File
@@ -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": {
+30
View File
@@ -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<boolean>,
closeTabs: (tabIds: string[]) => void
): Promise<void> {
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)
}
}