Show mobile source-control diffs after file taps

This commit is contained in:
brennanb2025
2026-06-23 19:39:37 -07:00
parent e03a1cf769
commit 1dda14feb5
7 changed files with 266 additions and 14 deletions
@@ -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<void> => {
// Route matching through the shared helper so the deterministic repro
// test exercises the same logic production runs.
const settled = await activateOpenedSourceControlDiffTab<MobileSessionTab>({
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}
/>
)}
</View>
+7 -2
View File
@@ -22,6 +22,7 @@ type Props = {
branchContextLoaded: boolean
availableWidth: number
onRequestClose: () => void
onOpenedFileDiff?: (relativePath: string) => void
}
type DockPanelContentProps = Omit<Props, 'availableWidth'>
@@ -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}
/>
</View>
)
@@ -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}
/>
)
}
@@ -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<void>) {
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,
@@ -40,17 +40,75 @@ export async function refreshOpenedMobileSessionTabs(
export function findOpenedMobileSessionTab<T extends OpenedMobileSessionTabCandidate>(
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<T extends OpenedMobileSessionTabCandidate> = {
relativePath: string
activeTabIdAtTap: string | null
fetchSessionTabs: () => Promise<void>
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<T extends OpenedMobileSessionTabCandidate>(
options: ActivateOpenedSourceControlDiffTabOptions<T>
): Promise<boolean> {
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(
@@ -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 {
@@ -28,6 +28,7 @@ type Params = {
origin: string
embedded: boolean
onRequestClose?: () => void
onOpenedFileDiff?: (relativePath: string) => void
branchCompareState: MobileBranchCompareState
mountedRef: MutableRefObject<boolean>
busyActionRef: MutableRefObject<string | null>
@@ -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,
@@ -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,