From 1dda14feb5fabcccae6d6ea3eeab7fc4ac93daa5 Mon Sep 17 00:00:00 2001 From: brennanb2025 Date: Tue, 23 Jun 2026 19:39:37 -0700 Subject: [PATCH] Show mobile source-control diffs after file taps --- .../app/h/[hostId]/session/[worktreeId].tsx | 37 +++++ mobile/src/session/SessionDockColumn.tsx | 9 +- .../session/opened-mobile-session-tab.test.ts | 143 ++++++++++++++++++ .../src/session/opened-mobile-session-tab.ts | 76 ++++++++-- .../MobileSourceControlPanel.tsx | 7 +- .../use-mobile-source-control-openers.ts | 4 + .../use-mobile-source-control-state.ts | 4 +- 7 files changed, 266 insertions(+), 14 deletions(-) diff --git a/mobile/app/h/[hostId]/session/[worktreeId].tsx b/mobile/app/h/[hostId]/session/[worktreeId].tsx index 542dbf53a47..34824d50f26 100644 --- a/mobile/app/h/[hostId]/session/[worktreeId].tsx +++ b/mobile/app/h/[hostId]/session/[worktreeId].tsx @@ -192,6 +192,7 @@ import { } from '../../../../src/session/mobile-session-route-helpers' import { resolveMarkdownFloatingActionsBottom } from '../../../../src/session/markdown-floating-actions-layout' import { resolveTabStripScrollOffset } from '../../../../src/session/tab-strip-scroll' +import { activateOpenedSourceControlDiffTab } from '../../../../src/session/opened-mobile-session-tab' import { createMobileSessionCreateWarningState, dismissMobileSessionCreateWarningState, @@ -3140,6 +3141,41 @@ export default function SessionScreen() { [client, worktreeId, scheduleDelayedAction, fetchSessionTabs] ) + const handleOpenedFileDiffActivationSeqRef = useRef(0) + const handleOpenedFileDiff = useCallback( + (relativePath: string) => { + const activationSeq = ++handleOpenedFileDiffActivationSeqRef.current + const activeTabIdAtTap = activeSessionTabIdRef.current + + let activated = false + const activateOpenedTab = async (): Promise => { + // Route matching through the shared helper so the deterministic repro + // test exercises the same logic production runs. + const settled = await activateOpenedSourceControlDiffTab({ + relativePath, + activeTabIdAtTap, + fetchSessionTabs, + getTabs: () => sessionTabsRef.current, + getActiveTabId: () => activeSessionTabIdRef.current, + getActivationState: () => ({ + activated, + activationSeq, + latestActivationSeq: handleOpenedFileDiffActivationSeqRef.current + }), + switchSessionTab: (tab) => switchSessionTabRef.current?.(tab) + }) + if (settled) { + activated = true + } + } + + scheduleDelayedAction(() => void activateOpenedTab(), 300) + scheduleDelayedAction(() => void activateOpenedTab(), 900) + scheduleDelayedAction(() => void activateOpenedTab(), 1800) + }, + [fetchSessionTabs, scheduleDelayedAction] + ) + const handleTerminalOpenUrl = useCallback( (handle: string, url: string) => { if (handle !== activeHandleRef.current) { @@ -5044,6 +5080,7 @@ export default function SessionScreen() { branchContextLoaded={prContextLoaded && prRepoContextLoaded} availableWidth={sessionContentRowWidth} onRequestClose={() => setActivePanel(null)} + onOpenedFileDiff={handleOpenedFileDiff} /> )} diff --git a/mobile/src/session/SessionDockColumn.tsx b/mobile/src/session/SessionDockColumn.tsx index 9b02b97580a..e8e2432b01d 100644 --- a/mobile/src/session/SessionDockColumn.tsx +++ b/mobile/src/session/SessionDockColumn.tsx @@ -22,6 +22,7 @@ type Props = { branchContextLoaded: boolean availableWidth: number onRequestClose: () => void + onOpenedFileDiff?: (relativePath: string) => void } type DockPanelContentProps = Omit @@ -43,7 +44,8 @@ export function SessionDockColumn({ isGithubRepo, branchContextLoaded, availableWidth, - onRequestClose + onRequestClose, + onOpenedFileDiff }: Props) { const { dockWidth, panHandlers } = useMobileDockResize(availableWidth) return ( @@ -63,6 +65,7 @@ export function SessionDockColumn({ isGithubRepo={isGithubRepo} branchContextLoaded={branchContextLoaded} onRequestClose={onRequestClose} + onOpenedFileDiff={onOpenedFileDiff} /> ) @@ -81,7 +84,8 @@ const DockPanelContent = memo(function DockPanelContent({ headSha, isGithubRepo, branchContextLoaded, - onRequestClose + onRequestClose, + onOpenedFileDiff }: DockPanelContentProps) { if (activePanel === 'sourceControl') { return ( @@ -92,6 +96,7 @@ const DockPanelContent = memo(function DockPanelContent({ origin="session" embedded onRequestClose={onRequestClose} + onOpenedFileDiff={onOpenedFileDiff} /> ) } diff --git a/mobile/src/session/opened-mobile-session-tab.test.ts b/mobile/src/session/opened-mobile-session-tab.test.ts index 104ee312b1e..988a3ea313f 100644 --- a/mobile/src/session/opened-mobile-session-tab.test.ts +++ b/mobile/src/session/opened-mobile-session-tab.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from 'vitest' import { activateOpenedMobileSessionTab, + activateOpenedSourceControlDiffTab, findOpenedMobileSessionTab, refreshOpenedMobileSessionTabs, shouldActivateOpenedMobileSessionTab, @@ -43,6 +44,15 @@ describe('findOpenedMobileSessionTab', () => { expect(findOpenedMobileSessionTab(tabs, 'assets/logo.png')?.id).toBe('image-1') }) + it('matches a diff tab opened from mobile source control', () => { + const tabs: OpenedMobileSessionTabCandidate[] = [ + { type: 'terminal', id: 'term' }, + { type: 'file', id: 'diff-1', mode: 'diff', relativePath: 'src/app.ts' } + ] + + expect(findOpenedMobileSessionTab(tabs, 'src/app.ts')?.id).toBe('diff-1') + }) + it('skips diff tabs when an edit tab has the same relative path', () => { const tabs: OpenedMobileSessionTabCandidate[] = [ { type: 'file', id: 'diff-1', mode: 'diff', relativePath: 'src/app.ts' }, @@ -51,6 +61,15 @@ describe('findOpenedMobileSessionTab', () => { expect(findOpenedMobileSessionTab(tabs, 'src/app.ts')?.id).toBe('edit-1') }) + + it('can prefer a diff tab when source control just opened one', () => { + const tabs: OpenedMobileSessionTabCandidate[] = [ + { type: 'markdown', id: 'edit-1', relativePath: 'README.md' }, + { type: 'file', id: 'diff-1', mode: 'diff', relativePath: 'README.md' } + ] + + expect(findOpenedMobileSessionTab(tabs, 'README.md', { preferMode: 'diff' })?.id).toBe('diff-1') + }) }) describe('refreshOpenedMobileSessionTabs', () => { @@ -188,6 +207,130 @@ describe('activateOpenedMobileSessionTab', () => { }) }) +describe('activateOpenedSourceControlDiffTab', () => { + const diffTab = { type: 'file', id: 'diff-1', mode: 'diff', relativePath: 'src/app.ts' } + + function createSourceControlHarness() { + let tabs: OpenedMobileSessionTabCandidate[] = [] + let activeTabId: string | null = 'terminal-1' + let activated = false + const activationSeq = 1 + let latestActivationSeq = 1 + const switched: string[] = [] + return { + setTabs(nextTabs: OpenedMobileSessionTabCandidate[]) { + tabs = nextTabs + }, + setActiveTabId(id: string | null) { + activeTabId = id + }, + markActivated() { + activated = true + }, + supersedeActivation() { + latestActivationSeq += 1 + }, + options(fetchSessionTabs: () => Promise) { + return { + relativePath: 'src/app.ts', + activeTabIdAtTap: 'terminal-1' as string | null, + fetchSessionTabs, + getTabs: () => tabs, + getActiveTabId: () => activeTabId, + getActivationState: () => ({ activated, activationSeq, latestActivationSeq }), + switchSessionTab: (tab: OpenedMobileSessionTabCandidate) => { + activeTabId = tab.id + switched.push(tab.id) + } + } + }, + switched + } + } + + it('refreshes tabs and switches to the opened diff tab', async () => { + const harness = createSourceControlHarness() + + const settled = await activateOpenedSourceControlDiffTab( + harness.options(async () => { + harness.setTabs([diffTab]) + }) + ) + + expect(settled).toBe(true) + expect(harness.switched).toEqual(['diff-1']) + }) + + it('prefers the opened diff over an existing edit tab for the same path', async () => { + const harness = createSourceControlHarness() + + const settled = await activateOpenedSourceControlDiffTab( + harness.options(async () => { + harness.setTabs([{ type: 'markdown', id: 'edit-1', relativePath: 'src/app.ts' }, diffTab]) + }) + ) + + expect(settled).toBe(true) + expect(harness.switched).toEqual(['diff-1']) + }) + + it('settles without switching when the snapshot already made the diff active', async () => { + const harness = createSourceControlHarness() + + const settled = await activateOpenedSourceControlDiffTab( + harness.options(async () => { + harness.setTabs([diffTab]) + harness.setActiveTabId('diff-1') + }) + ) + + expect(settled).toBe(true) + expect(harness.switched).toEqual([]) + }) + + it('does not steal focus when the user moved to a different tab after the tap', async () => { + const harness = createSourceControlHarness() + + const settled = await activateOpenedSourceControlDiffTab( + harness.options(async () => { + harness.setTabs([diffTab]) + harness.setActiveTabId('terminal-2') + }) + ) + + expect(settled).toBe(false) + expect(harness.switched).toEqual([]) + }) + + it('does not switch when a newer tap supersedes this attempt during refresh', async () => { + const harness = createSourceControlHarness() + + const settled = await activateOpenedSourceControlDiffTab( + harness.options(async () => { + harness.setTabs([diffTab]) + harness.supersedeActivation() + }) + ) + + expect(settled).toBe(false) + expect(harness.switched).toEqual([]) + }) + + it('stops retrying after an earlier attempt already activated the tab', async () => { + const harness = createSourceControlHarness() + harness.markActivated() + + const settled = await activateOpenedSourceControlDiffTab( + harness.options(async () => { + harness.setTabs([diffTab]) + }) + ) + + expect(settled).toBe(false) + expect(harness.switched).toEqual([]) + }) +}) + describe('shouldActivateOpenedMobileSessionTab', () => { const currentTerminalState = { activationSeq: 2, diff --git a/mobile/src/session/opened-mobile-session-tab.ts b/mobile/src/session/opened-mobile-session-tab.ts index 16cecb0f3ab..17c8cff9848 100644 --- a/mobile/src/session/opened-mobile-session-tab.ts +++ b/mobile/src/session/opened-mobile-session-tab.ts @@ -40,17 +40,75 @@ export async function refreshOpenedMobileSessionTabs( export function findOpenedMobileSessionTab( tabs: readonly T[], - relativePath: string + relativePath: string, + options: { preferMode?: 'diff' | 'edit' } = {} ): T | null { - return ( - tabs.find( - (tab) => - tab.type !== 'browser' && - tab.type !== 'terminal' && - tab.mode !== 'diff' && - tab.relativePath === relativePath - ) ?? null + const matches = tabs.filter( + (tab) => tab.type !== 'browser' && tab.type !== 'terminal' && tab.relativePath === relativePath ) + if (matches.length === 0) { + return null + } + if (options.preferMode === 'diff') { + return matches.find((tab) => tab.mode === 'diff') ?? matches[0] ?? null + } + return matches.find((tab) => tab.mode !== 'diff') ?? matches[0] ?? null +} + +export type OpenedSourceControlDiffActivationState = { + activated: boolean + activationSeq: number + latestActivationSeq: number +} + +export type ActivateOpenedSourceControlDiffTabOptions = { + relativePath: string + activeTabIdAtTap: string | null + fetchSessionTabs: () => Promise + getTabs: () => readonly T[] + getActiveTabId: () => string | null + getActivationState: () => OpenedSourceControlDiffActivationState + switchSessionTab: (tab: T) => void +} + +// Activation for a diff tab opened by tapping a changed file in the docked +// source-control panel. Unlike the terminal path there is no "source terminal" +// to guard against; instead we cancel if the user moved to a different tab +// after the tap (no focus steal) and treat the diff already being active as +// success. Returns true once activation is settled so retries can stop. +export async function activateOpenedSourceControlDiffTab( + options: ActivateOpenedSourceControlDiffTabOptions +): Promise { + const isSuperseded = (): boolean => { + const state = options.getActivationState() + return state.activated || state.activationSeq !== state.latestActivationSeq + } + if (isSuperseded()) { + return false + } + await options.fetchSessionTabs() + if (isSuperseded()) { + return false + } + const opened = findOpenedMobileSessionTab(options.getTabs(), options.relativePath, { + preferMode: 'diff' + }) + if (!opened) { + return false + } + const activeTabId = options.getActiveTabId() + // The post-open snapshot may already show the opened diff as active; that is + // success, not a focus steal, so settle without re-activating. + if (activeTabId === opened.id) { + return true + } + // No focus steal: if the user moved to a different tab after the tap, leave + // them there instead of yanking focus back to the diff. + if (activeTabId !== options.activeTabIdAtTap) { + return false + } + options.switchSessionTab(opened) + return true } export function shouldActivateOpenedMobileSessionTab( diff --git a/mobile/src/source-control/MobileSourceControlPanel.tsx b/mobile/src/source-control/MobileSourceControlPanel.tsx index 4def372b3b1..52d31f7578b 100644 --- a/mobile/src/source-control/MobileSourceControlPanel.tsx +++ b/mobile/src/source-control/MobileSourceControlPanel.tsx @@ -16,6 +16,7 @@ export type MobileSourceControlPanelProps = { origin?: string embedded?: boolean onRequestClose?: () => void + onOpenedFileDiff?: (relativePath: string) => void } export function MobileSourceControlPanel({ @@ -24,7 +25,8 @@ export function MobileSourceControlPanel({ name = '', origin = '', embedded = false, - onRequestClose + onRequestClose, + onOpenedFileDiff }: MobileSourceControlPanelProps) { const state = useMobileSourceControlState({ hostId, @@ -32,7 +34,8 @@ export function MobileSourceControlPanel({ name, origin, embedded, - onRequestClose + onRequestClose, + onOpenedFileDiff }) const actionSheetActions = useMobileSourceControlActionSheet(state) const { diff --git a/mobile/src/source-control/use-mobile-source-control-openers.ts b/mobile/src/source-control/use-mobile-source-control-openers.ts index f0857b89582..f463c677f9f 100644 --- a/mobile/src/source-control/use-mobile-source-control-openers.ts +++ b/mobile/src/source-control/use-mobile-source-control-openers.ts @@ -28,6 +28,7 @@ type Params = { origin: string embedded: boolean onRequestClose?: () => void + onOpenedFileDiff?: (relativePath: string) => void branchCompareState: MobileBranchCompareState mountedRef: MutableRefObject busyActionRef: MutableRefObject @@ -46,6 +47,7 @@ export function useMobileSourceControlOpeners(params: Params) { origin, embedded, onRequestClose, + onOpenedFileDiff, branchCompareState, mountedRef, busyActionRef, @@ -98,6 +100,7 @@ export function useMobileSourceControlOpeners(params: Params) { } triggerSelection() if (origin === 'session') { + 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 @@ -140,6 +143,7 @@ export function useMobileSourceControlOpeners(params: Params) { hostId, mountedRef, name, + onOpenedFileDiff, onRequestClose, origin, router, diff --git a/mobile/src/source-control/use-mobile-source-control-state.ts b/mobile/src/source-control/use-mobile-source-control-state.ts index 875715ad9b0..21f302e753e 100644 --- a/mobile/src/source-control/use-mobile-source-control-state.ts +++ b/mobile/src/source-control/use-mobile-source-control-state.ts @@ -39,10 +39,11 @@ export type MobileSourceControlStateParams = { origin: string embedded: boolean onRequestClose?: () => void + onOpenedFileDiff?: (relativePath: string) => void } export function useMobileSourceControlState(params: MobileSourceControlStateParams) { - const { hostId, worktreeId, name, origin, embedded, onRequestClose } = params + const { hostId, worktreeId, name, origin, embedded, onRequestClose, onOpenedFileDiff } = params const insets = useSafeAreaInsets() const { client, state: connState } = useHostClient(hostId) const forceReconnect = useForceReconnect() @@ -89,6 +90,7 @@ export function useMobileSourceControlState(params: MobileSourceControlStatePara origin, embedded, onRequestClose, + onOpenedFileDiff, branchCompareState, mountedRef, busyActionRef,