fix(mobile): activate Source Control diff tabs on phones (#12770)
* fix(mobile): activate source-control diff tabs on phones * fix(mobile): reveal legacy source control file tabs
This commit is contained in:
Brennan Benson
GitHub
parent
885afb55a9
commit
7f4570c9a6
|
|
@ -0,0 +1,233 @@
|
|||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { RpcClient } from '../transport/rpc-client'
|
||||
import type { RpcResponse } from '../transport/types'
|
||||
import { revealMobileSourceControlSessionDiff } from './reveal-mobile-source-control-session-diff'
|
||||
|
||||
function success(result: unknown): RpcResponse {
|
||||
return { id: 'rpc-1', ok: true, result, _meta: { runtimeId: 'runtime-1' } }
|
||||
}
|
||||
|
||||
function clientWith(sendRequest: RpcClient['sendRequest']): Pick<RpcClient, 'sendRequest'> {
|
||||
return { sendRequest }
|
||||
}
|
||||
|
||||
function options(sendRequest: RpcClient['sendRequest']) {
|
||||
return {
|
||||
client: clientWith(sendRequest),
|
||||
worktreeId: 'worktree-1',
|
||||
relativePath: 'src/target.ts',
|
||||
tabMode: 'diff' as const,
|
||||
staged: true
|
||||
}
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers()
|
||||
})
|
||||
|
||||
describe('revealMobileSourceControlSessionDiff', () => {
|
||||
it('activates the requested file when its diff tab is already open', async () => {
|
||||
const sendRequest = vi
|
||||
.fn<RpcClient['sendRequest']>()
|
||||
.mockResolvedValueOnce(
|
||||
success({
|
||||
activeTabId: 'agent-tab',
|
||||
tabs: [
|
||||
{ id: 'agent-tab', type: 'terminal' },
|
||||
{
|
||||
id: 'other-diff',
|
||||
type: 'file',
|
||||
mode: 'diff',
|
||||
diffSource: 'staged',
|
||||
relativePath: 'src/other.ts'
|
||||
},
|
||||
{
|
||||
id: 'target-diff',
|
||||
type: 'file',
|
||||
mode: 'diff',
|
||||
diffSource: 'staged',
|
||||
relativePath: 'src/target.ts'
|
||||
}
|
||||
]
|
||||
})
|
||||
)
|
||||
.mockResolvedValueOnce(success({ activeTabId: 'target-diff' }))
|
||||
|
||||
await expect(revealMobileSourceControlSessionDiff(options(sendRequest))).resolves.toBe(
|
||||
'revealed'
|
||||
)
|
||||
expect(sendRequest).toHaveBeenNthCalledWith(2, 'session.tabs.activate', {
|
||||
worktree: 'id:worktree-1',
|
||||
tabId: 'target-diff',
|
||||
notifyClients: false,
|
||||
navigation: 'caller',
|
||||
intent: 'user'
|
||||
})
|
||||
})
|
||||
|
||||
it('selects the staged diff when both versions of a file are open', async () => {
|
||||
const sendRequest = vi
|
||||
.fn<RpcClient['sendRequest']>()
|
||||
.mockResolvedValueOnce(
|
||||
success({
|
||||
tabs: [
|
||||
{
|
||||
id: 'unstaged-diff',
|
||||
type: 'file',
|
||||
mode: 'diff',
|
||||
diffSource: 'unstaged',
|
||||
relativePath: 'src/target.ts'
|
||||
},
|
||||
{
|
||||
id: 'staged-diff',
|
||||
type: 'file',
|
||||
mode: 'diff',
|
||||
diffSource: 'staged',
|
||||
relativePath: 'src/target.ts'
|
||||
}
|
||||
]
|
||||
})
|
||||
)
|
||||
.mockResolvedValueOnce(success({ activeTabId: 'staged-diff' }))
|
||||
|
||||
await expect(revealMobileSourceControlSessionDiff(options(sendRequest))).resolves.toBe(
|
||||
'revealed'
|
||||
)
|
||||
expect(sendRequest).toHaveBeenLastCalledWith(
|
||||
'session.tabs.activate',
|
||||
expect.objectContaining({ tabId: 'staged-diff' })
|
||||
)
|
||||
})
|
||||
|
||||
it('waits for the requested diff source instead of activating the other one', async () => {
|
||||
vi.useFakeTimers()
|
||||
const sendRequest = vi
|
||||
.fn<RpcClient['sendRequest']>()
|
||||
.mockResolvedValueOnce(
|
||||
success({
|
||||
tabs: [
|
||||
{
|
||||
id: 'unstaged-diff',
|
||||
type: 'file',
|
||||
mode: 'diff',
|
||||
diffSource: 'unstaged',
|
||||
relativePath: 'src/target.ts'
|
||||
}
|
||||
]
|
||||
})
|
||||
)
|
||||
.mockResolvedValueOnce(
|
||||
success({
|
||||
tabs: [
|
||||
{
|
||||
id: 'staged-diff',
|
||||
type: 'file',
|
||||
mode: 'diff',
|
||||
diffSource: 'staged',
|
||||
relativePath: 'src/target.ts'
|
||||
}
|
||||
]
|
||||
})
|
||||
)
|
||||
.mockResolvedValueOnce(success({ activeTabId: 'staged-diff' }))
|
||||
|
||||
const reveal = revealMobileSourceControlSessionDiff(options(sendRequest))
|
||||
await vi.advanceTimersByTimeAsync(300)
|
||||
|
||||
await expect(reveal).resolves.toBe('revealed')
|
||||
expect(sendRequest).toHaveBeenLastCalledWith(
|
||||
'session.tabs.activate',
|
||||
expect.objectContaining({ tabId: 'staged-diff' })
|
||||
)
|
||||
})
|
||||
|
||||
it('activates a legacy edit tab when opening a diff falls back to files.open', async () => {
|
||||
const sendRequest = vi
|
||||
.fn<RpcClient['sendRequest']>()
|
||||
.mockResolvedValueOnce(
|
||||
success({
|
||||
tabs: [
|
||||
{
|
||||
id: 'stale-diff',
|
||||
type: 'file',
|
||||
mode: 'diff',
|
||||
diffSource: 'staged',
|
||||
relativePath: 'src/target.ts'
|
||||
},
|
||||
{
|
||||
id: 'target-edit',
|
||||
type: 'file',
|
||||
relativePath: 'src/target.ts'
|
||||
}
|
||||
]
|
||||
})
|
||||
)
|
||||
.mockResolvedValueOnce(success({ activeTabId: 'target-edit' }))
|
||||
|
||||
await expect(
|
||||
revealMobileSourceControlSessionDiff({ ...options(sendRequest), tabMode: 'edit' })
|
||||
).resolves.toBe('revealed')
|
||||
expect(sendRequest).toHaveBeenLastCalledWith(
|
||||
'session.tabs.activate',
|
||||
expect.objectContaining({ tabId: 'target-edit' })
|
||||
)
|
||||
})
|
||||
|
||||
it('retries until the opened diff appears in the session snapshot', async () => {
|
||||
vi.useFakeTimers()
|
||||
const sendRequest = vi
|
||||
.fn<RpcClient['sendRequest']>()
|
||||
.mockResolvedValueOnce(success({ tabs: [{ id: 'agent-tab', type: 'terminal' }] }))
|
||||
.mockResolvedValueOnce(
|
||||
success({
|
||||
tabs: [
|
||||
{
|
||||
id: 'target-diff',
|
||||
type: 'file',
|
||||
mode: 'diff',
|
||||
diffSource: 'staged',
|
||||
relativePath: 'src/target.ts'
|
||||
}
|
||||
]
|
||||
})
|
||||
)
|
||||
.mockResolvedValueOnce(success({ activeTabId: 'target-diff' }))
|
||||
|
||||
const reveal = revealMobileSourceControlSessionDiff(options(sendRequest))
|
||||
await vi.advanceTimersByTimeAsync(300)
|
||||
|
||||
await expect(reveal).resolves.toBe('revealed')
|
||||
expect(sendRequest).toHaveBeenCalledTimes(3)
|
||||
})
|
||||
|
||||
it('delegates to the mounted session when the dock callback is available', async () => {
|
||||
const sendRequest = vi.fn<RpcClient['sendRequest']>()
|
||||
const onOpenedFileDiff = vi.fn()
|
||||
|
||||
await expect(
|
||||
revealMobileSourceControlSessionDiff({
|
||||
...options(sendRequest),
|
||||
onOpenedFileDiff
|
||||
})
|
||||
).resolves.toBe('revealed')
|
||||
|
||||
expect(onOpenedFileDiff).toHaveBeenCalledWith('src/target.ts')
|
||||
expect(sendRequest).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('cancels route-owned polling after the source-control screen unmounts', async () => {
|
||||
let current = true
|
||||
const sendRequest = vi.fn<RpcClient['sendRequest']>().mockImplementation(async () => {
|
||||
current = false
|
||||
return success({ tabs: [] })
|
||||
})
|
||||
|
||||
await expect(
|
||||
revealMobileSourceControlSessionDiff({
|
||||
...options(sendRequest),
|
||||
isCurrent: () => current
|
||||
})
|
||||
).resolves.toBe('cancelled')
|
||||
expect(sendRequest).toHaveBeenCalledOnce()
|
||||
})
|
||||
})
|
||||
|
|
@ -0,0 +1,143 @@
|
|||
import type { RpcClient } from '../transport/rpc-client'
|
||||
import { activateMobileSessionTab } from '../session/mobile-session-tab-activation'
|
||||
|
||||
type ActivationClient = Pick<RpcClient, 'sendRequest'>
|
||||
|
||||
type SessionFileTabCandidate = {
|
||||
id: string
|
||||
type: string
|
||||
mode?: unknown
|
||||
relativePath?: unknown
|
||||
diffSource?: unknown
|
||||
}
|
||||
|
||||
type Options = {
|
||||
client: ActivationClient
|
||||
worktreeId: string
|
||||
relativePath: string
|
||||
tabMode: 'diff' | 'edit'
|
||||
staged: boolean
|
||||
onOpenedFileDiff?: (relativePath: string) => void
|
||||
isCurrent?: () => boolean
|
||||
}
|
||||
|
||||
export type MobileSourceControlSessionDiffRevealResult = 'revealed' | 'cancelled' | 'timeout'
|
||||
|
||||
const TAB_POLL_DELAYS_MS = [0, 300, 600, 900] as const
|
||||
|
||||
export async function revealMobileSourceControlSessionDiff(
|
||||
options: Options
|
||||
): Promise<MobileSourceControlSessionDiffRevealResult> {
|
||||
if (options.onOpenedFileDiff) {
|
||||
options.onOpenedFileDiff(options.relativePath)
|
||||
return 'revealed'
|
||||
}
|
||||
|
||||
for (const delayMs of TAB_POLL_DELAYS_MS) {
|
||||
await waitForDelay(delayMs)
|
||||
if (options.isCurrent?.() === false) {
|
||||
return 'cancelled'
|
||||
}
|
||||
|
||||
const tab = await findOpenedSessionFileTab(options)
|
||||
if (options.isCurrent?.() === false) {
|
||||
return 'cancelled'
|
||||
}
|
||||
if (!tab) {
|
||||
continue
|
||||
}
|
||||
|
||||
const activated = await activateSessionFileTab(options, tab.id)
|
||||
if (options.isCurrent?.() === false) {
|
||||
return 'cancelled'
|
||||
}
|
||||
if (activated) {
|
||||
return 'revealed'
|
||||
}
|
||||
}
|
||||
|
||||
return 'timeout'
|
||||
}
|
||||
|
||||
async function findOpenedSessionFileTab(options: Options): Promise<SessionFileTabCandidate | null> {
|
||||
try {
|
||||
const response = await options.client.sendRequest('session.tabs.list', {
|
||||
worktree: `id:${options.worktreeId}`
|
||||
})
|
||||
if (!response.ok) {
|
||||
return null
|
||||
}
|
||||
const snapshot = readTabSnapshot(response.result)
|
||||
if (!snapshot) {
|
||||
return null
|
||||
}
|
||||
|
||||
const matches = snapshot.tabs.filter(
|
||||
(tab) =>
|
||||
tab.type !== 'browser' &&
|
||||
tab.type !== 'terminal' &&
|
||||
matchesTabMode(tab.mode, options.tabMode) &&
|
||||
tab.relativePath === options.relativePath
|
||||
)
|
||||
if (options.tabMode === 'edit') {
|
||||
return matches[0] ?? null
|
||||
}
|
||||
const source = options.staged ? 'staged' : 'unstaged'
|
||||
return (
|
||||
matches.find((tab) => tab.diffSource === source) ??
|
||||
matches.find((tab) => tab.diffSource == null) ??
|
||||
null
|
||||
)
|
||||
} catch {
|
||||
return null
|
||||
}
|
||||
}
|
||||
|
||||
function matchesTabMode(mode: unknown, expected: 'diff' | 'edit'): boolean {
|
||||
return expected === 'diff' ? mode === 'diff' : mode === 'edit' || mode == null
|
||||
}
|
||||
|
||||
async function activateSessionFileTab(options: Options, tabId: string): Promise<boolean> {
|
||||
try {
|
||||
const response = await activateMobileSessionTab(options.client, {
|
||||
worktree: `id:${options.worktreeId}`,
|
||||
tabId,
|
||||
notifyClients: false,
|
||||
navigation: 'caller',
|
||||
intent: 'user'
|
||||
})
|
||||
return response.ok && readActiveTabId(response.result) === tabId
|
||||
} catch {
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
function readTabSnapshot(value: unknown): { tabs: SessionFileTabCandidate[] } | null {
|
||||
if (
|
||||
!isRecord(value) ||
|
||||
!Array.isArray(value.tabs) ||
|
||||
!value.tabs.every(isSessionFileTabCandidate)
|
||||
) {
|
||||
return null
|
||||
}
|
||||
return { tabs: value.tabs }
|
||||
}
|
||||
|
||||
function isSessionFileTabCandidate(value: unknown): value is SessionFileTabCandidate {
|
||||
return isRecord(value) && typeof value.id === 'string' && typeof value.type === 'string'
|
||||
}
|
||||
|
||||
function readActiveTabId(value: unknown): string | null {
|
||||
return isRecord(value) && typeof value.activeTabId === 'string' ? value.activeTabId : null
|
||||
}
|
||||
|
||||
function isRecord(value: unknown): value is Record<string, unknown> {
|
||||
return typeof value === 'object' && value !== null
|
||||
}
|
||||
|
||||
async function waitForDelay(delayMs: number): Promise<void> {
|
||||
if (delayMs === 0) {
|
||||
return
|
||||
}
|
||||
await new Promise<void>((resolve) => setTimeout(resolve, delayMs))
|
||||
}
|
||||
|
|
@ -18,6 +18,7 @@ import {
|
|||
type MobileGitStatusEntry
|
||||
} from './mobile-git-status'
|
||||
import { buildMobileReviewFileRoute } from './mobile-review-route'
|
||||
import { revealMobileSourceControlSessionDiff } from './reveal-mobile-source-control-session-diff'
|
||||
import type {
|
||||
GitDiffTextResult,
|
||||
MobileBranchCompareState,
|
||||
|
|
@ -115,11 +116,13 @@ export function useMobileSourceControlOpeners(params: Params) {
|
|||
relativePath: entry.path,
|
||||
staged: entry.area === 'staged'
|
||||
})
|
||||
let openedTabMode: 'diff' | 'edit' = 'diff'
|
||||
if (!response.ok && isMobileGitUnavailable(response.error?.code, response.error?.message)) {
|
||||
response = await client.sendRequest('files.open', {
|
||||
worktree: `id:${worktreeId}`,
|
||||
relativePath: entry.path
|
||||
})
|
||||
openedTabMode = 'edit'
|
||||
}
|
||||
if (!response.ok) {
|
||||
throw new Error(response.error?.message || 'Unable to open diff')
|
||||
|
|
@ -127,8 +130,22 @@ export function useMobileSourceControlOpeners(params: Params) {
|
|||
if (!mountedRef.current) {
|
||||
return
|
||||
}
|
||||
const revealResult = await revealMobileSourceControlSessionDiff({
|
||||
client,
|
||||
worktreeId,
|
||||
relativePath: entry.path,
|
||||
tabMode: openedTabMode,
|
||||
staged: entry.area === 'staged',
|
||||
onOpenedFileDiff,
|
||||
isCurrent: () => mountedRef.current && openingPathRef.current === entry.path
|
||||
})
|
||||
if (revealResult === 'cancelled') {
|
||||
return
|
||||
}
|
||||
if (revealResult === 'timeout') {
|
||||
throw new Error("The file opened, but its tab isn't ready yet. Try again.")
|
||||
}
|
||||
triggerSelection()
|
||||
onOpenedFileDiff?.(entry.path)
|
||||
// Why: when launched from the session screen, opening a file dismisses
|
||||
// this surface back to the session. In embedded mode there is nothing
|
||||
// to pop (the panel docks beside the terminal), so close the dock
|
||||
|
|
|
|||
Loading…
Reference in New Issue