From fce3f174d22bf0ca4e1e94b1b25aa8d6a268e1e6 Mon Sep 17 00:00:00 2001 From: Bohdan Triapitsyn Date: Wed, 9 Sep 2026 14:58:42 +0300 Subject: [PATCH] feat(mobile): add branch and commit comparison views Mobile Changes only exposed working-tree edits. Share comparison loading and source pickers with desktop, with phone sheets, tablet popovers, scoped file navigation and retry states. Keep checkout, Sync and commit controls under Changes. Compact the section and source triggers, center menu items, and preserve repeated file-open requests from chat. Validated with UI type-check and lint, comparison and navigation tests, web/mobile asset builds, and maintainer app testing. Co-authored-by: gaojunran --- packages/mobile/README.md | 1 + .../ui/src/apps/MobileChangesSurface.test.tsx | 194 ++++++++++ packages/ui/src/apps/MobileChangesSurface.tsx | 348 +++++++++++++++--- .../ui/src/apps/MobileWorkspaceDrawer.tsx | 6 +- .../src/components/ui/MobileOverlayPanel.tsx | 1 + .../ui/src/components/ui/dropdown-trigger.ts | 2 + packages/ui/src/components/views/DiffView.tsx | 113 ++---- .../components/views/branchDiffScope.test.ts | 15 - .../src/components/views/branchDiffScope.ts | 9 - .../git/BranchComparisonSelector.test.tsx | 20 +- .../views/git/BranchComparisonSelector.tsx | 130 ++++--- .../git/CommitComparisonSelector.test.tsx | 17 +- .../views/git/CommitComparisonSelector.tsx | 119 +++--- .../ui/src/hooks/useGitComparison.test.tsx | 135 +++++++ packages/ui/src/hooks/useGitComparison.ts | 79 ++++ packages/ui/src/lib/diff/patchFileDiff.ts | 3 + packages/ui/src/stores/DOCUMENTATION.md | 13 +- packages/ui/src/styles/mobile.css | 6 + packages/web/server/lib/git/DOCUMENTATION.md | 2 +- 19 files changed, 928 insertions(+), 285 deletions(-) create mode 100644 packages/ui/src/apps/MobileChangesSurface.test.tsx create mode 100644 packages/ui/src/hooks/useGitComparison.test.tsx create mode 100644 packages/ui/src/hooks/useGitComparison.ts diff --git a/packages/mobile/README.md b/packages/mobile/README.md index a06a96d8..da33229b 100644 --- a/packages/mobile/README.md +++ b/packages/mobile/README.md @@ -14,6 +14,7 @@ The mobile package reuses the web build, then rewrites `mobile.html` to `index.h - The tablet layout is a live size class (`useTabletLayout`), not a device check: any surface whose short side is at least 600px gets it, and the workspace only becomes a side panel where the width can host the sidebar, the panel and a readable chat at once. Book foldables therefore pick it up when unfolded, keep the portrait layout in both orientations (their long side is barely wider than a tablet's short one), and drop back to the phone layout when folded shut. The Android activity declares the matching `configChanges`, so folding resizes the WebView instead of recreating it. - Password-protected OpenChamber servers can be unlocked from the mobile app. The app stores the issued client token with the saved connection. - The Terminal workspace surface runs its PTY on the active OpenChamber server over the shared authenticated runtime transport; it never opens a local shell on the phone or tablet. Closing the surface detaches the renderer while the server session remains available for reattachment. On touch devices, dragging scrolls the buffer while long-pressing and dragging selects terminal text. +- The Changes workspace has a top-level Changes / Branch / Commit selector. Checkout, Sync, staging, and commit controls appear only under Changes; Branch and Commit show a read-only file list that opens one diff at a time. Their shared source pickers use bottom sheets on phones and anchored popovers on tablets. Commit lists the latest 50 commits of the checked-out branch. Closing the workspace preserves the current detail and suspends comparison reads; changing repository or instance resets navigation, and a new file link from chat opens the working-tree diff. ## Commands diff --git a/packages/ui/src/apps/MobileChangesSurface.test.tsx b/packages/ui/src/apps/MobileChangesSurface.test.tsx new file mode 100644 index 00000000..cc7bdd94 --- /dev/null +++ b/packages/ui/src/apps/MobileChangesSurface.test.tsx @@ -0,0 +1,194 @@ +import React, { act } from 'react'; +import { expect, test } from 'bun:test'; +import { Window } from 'happy-dom'; +import type { GitLogEntry, GitStatus } from '@/lib/api/types'; + +test('mobile comparisons drill into files, retry, resume, change source, and yield to external working diffs', async () => { + const dom = new Window({ url: 'http://localhost' }); + dom.happyDOM.setWindowSize({ width: 390, height: 844 }); + const originals = new Map(); + const globals = { + window: dom, document: dom.document, navigator: dom.navigator, location: dom.location, localStorage: dom.localStorage, + Element: dom.Element, HTMLElement: dom.HTMLElement, HTMLInputElement: dom.HTMLInputElement, Node: dom.Node, + customElements: dom.customElements, CSSStyleSheet: dom.CSSStyleSheet, + Event: dom.Event, CustomEvent: dom.CustomEvent, KeyboardEvent: dom.KeyboardEvent, MouseEvent: dom.MouseEvent, + MutationObserver: dom.MutationObserver, ResizeObserver: dom.ResizeObserver, + getComputedStyle: dom.getComputedStyle.bind(dom), requestAnimationFrame: dom.requestAnimationFrame.bind(dom), + cancelAnimationFrame: dom.cancelAnimationFrame.bind(dom), IS_REACT_ACT_ENVIRONMENT: true, + }; + for (const [name, value] of Object.entries(globals)) { + originals.set(name, Object.getOwnPropertyDescriptor(globalThis, name)); + Object.defineProperty(globalThis, name, { configurable: true, writable: true, value }); + } + const commits: GitLogEntry[] = ['a', 'b'].map((letter) => ({ + hash: letter.repeat(40), date: '2026-09-09T09:22:00Z', message: `Commit ${letter}`, + refs: '', body: '', author_name: 'Test Author', author_email: 'test@example.com', + filesChanged: 1, insertions: 0, deletions: 0, parents: [], + })); + const requests: URL[] = []; + const originalFetch = globalThis.fetch; + let failBranchDiff = true; + globalThis.fetch = Object.assign(async (input: RequestInfo | URL) => { + const url = new URL(input instanceof Request ? input.url : String(input), 'http://localhost'); + if (url.pathname === '/api/fs/home' || url.pathname === '/api/session-folders') return new Promise(() => {}); + requests.push(url); + switch (url.pathname) { + case '/api/git/remotes': return Response.json([]); + case '/api/git/remote-url': return Response.json({ url: null }); + case '/api/git/branch-base': return Response.json({ base: null }); + case '/api/git/range-files': return Response.json({ files: [{ path: url.searchParams.get('base') === 'refs/heads/parent' ? 'parent.png' : 'branch.png', status: 'M' }] }); + case '/api/git/range-diff': + return failBranchDiff + ? Response.json({ error: 'Branch diff failed' }, { status: 500 }) + : Response.json({ diff: 'Binary files a/branch.png and b/branch.png differ' }); + case '/api/git/log': return Response.json({ all: commits, latest: commits[0], total: commits.length }); + case '/api/git/commit-files': return Response.json({ files: [{ path: `commit-${url.searchParams.get('hash')?.[0]}.png`, previousPath: 'old.png', changeType: 'R', insertions: 0, deletions: 0, isBinary: true }] }); + case '/api/git/commit-diff': return Response.json({ diff: 'Binary files a/old.png and b/commit.png differ' }); + case '/api/git/file-diff': return Response.json({ path: 'working.png', original: '', modified: '', isBinary: true }); + default: throw new Error(`Unexpected request ${url.pathname}`); + } + }, originalFetch); + + const { createRoot } = await import('react-dom/client'); + const { I18nProvider } = await import('@/lib/i18n'); + const { RuntimeAPIContext } = await import('@/contexts/runtimeAPIContext'); + const { createWebAPIs } = await import('../../../web/src/api/index'); + const { useGitStore } = await import('@/stores/useGitStore'); + const { MobileChangesPane } = await import('./MobileChangesSurface'); + const apis = createWebAPIs(); + const status: GitStatus = { current: 'feature', tracking: null, ahead: 0, behind: 0, files: [], isClean: true, diffStats: {} }; + const seed = (directory: string, nextStatus = status) => { + useGitStore.getState().setActiveDirectory(directory); + const previous = useGitStore.getState().getDirectoryState(directory); + if (!previous) throw new Error('Missing repository state'); + const now = Date.now(); + const directories = new Map(useGitStore.getState().directories); + directories.set(directory, { + ...previous, status: nextStatus, isGitRepo: true, + branches: { all: ['feature', 'main', 'parent', 'remotes/origin/main'], current: 'feature', branches: {}, defaultBranches: { origin: 'main' } }, + log: { all: commits, latest: commits[0], total: 2 }, identity: { userName: 'Test Author', userEmail: 'test@example.com', sshCommand: null }, + lastStatusFetch: now, lastBranchesFetch: now, lastLogFetch: now, lastIdentityFetch: now, lastRepoCheckAt: now, + }); + useGitStore.setState({ directories }); + }; + seed('/repo'); + let directory = '/repo'; + let visible = true; + let initialDiff: { path: string; staged: boolean } | null = null; + const container = document.createElement('div'); + document.body.append(container); + const root = createRoot(container); + const render = () => act(async () => { + root.render( + + ); + }); + const click = async (selector: string) => { + const element = document.querySelector(selector); + if (!element) throw new Error(`Missing ${selector}`); + await act(async () => { element.click(); }); + }; + const chooseMode = async (label: string) => { + await click('[aria-label="Select change mode"]'); + const option = [...document.querySelectorAll('[role="menuitemradio"]')].find((element) => element.textContent === label); + if (!option) throw new Error(`Missing mode ${label}`); + await act(async () => { option.click(); }); + }; + const openFile = async (path: string) => { + const button = container.querySelector(`[title="${path}"]`)?.closest('button'); + if (!button) throw new Error(`Missing file ${path}`); + await act(async () => { button.click(); }); + }; + const comparisonRequests = () => requests.filter((url) => /\/(range-files|range-diff|commit-files|commit-diff)$/.test(url.pathname)); + const checkoutControl = () => [...container.querySelectorAll('button')].find((button) => button.textContent?.trim() === 'feature'); + + try { + await render(); + const modeTrigger = container.querySelector('[aria-label="Select change mode"]'); + const syncButton = container.querySelector('[aria-label="Sync Changes"]'); + if (!modeTrigger || !syncButton) throw new Error('Missing Changes controls'); + expect(modeTrigger?.textContent).toBe('Changes'); + expect(container.querySelector('h2')).toBeNull(); + expect(checkoutControl()).toBeDefined(); + expect(syncButton).not.toBeNull(); + expect(modeTrigger?.closest('header')?.contains(syncButton)).toBe(false); + expect(modeTrigger && syncButton && (modeTrigger.compareDocumentPosition(syncButton) & Node.DOCUMENT_POSITION_FOLLOWING)).toBe(Node.DOCUMENT_POSITION_FOLLOWING); + await chooseMode('Branch'); + expect(container.querySelector('[aria-label="Sync Changes"]')).toBeNull(); + expect(checkoutControl()).toBeUndefined(); + expect(container.textContent).toContain('Select a base branch'); + await click('[aria-label="Base branch"]'); + await click('[data-value="refs/heads/main"]'); + expect(container.querySelector('[title="branch.png"]')).not.toBeNull(); + await openFile('branch.png'); + expect(container.textContent).toContain('Branch diff failed'); + expect(requests.filter((url) => url.pathname === '/api/git/file-diff')).toHaveLength(0); + failBranchDiff = false; + const retry = [...container.querySelectorAll('button')].find((button) => button.textContent === 'Retry'); + if (!retry) throw new Error('Missing diff retry'); + await act(async () => { retry.click(); }); + expect(container.textContent).toContain('Content of this file cannot be viewed.'); + expect(container.textContent).toContain('Branch · main'); + + const beforeHide = comparisonRequests().length; + visible = false; + await render(); + await act(async () => { seed('/repo'); }); + expect(comparisonRequests()).toHaveLength(beforeHide); + visible = true; + await render(); + expect(container.querySelector('h2')?.textContent).toBe('branch.png'); + await click('[aria-label="Back"]'); + await click('[aria-label="Base branch"]'); + await click('[data-value="refs/heads/parent"]'); + expect(container.querySelector('[title="branch.png"]')).toBeNull(); + expect(container.querySelector('[title="parent.png"]')).not.toBeNull(); + + await chooseMode('Commit'); + expect(container.querySelector('[aria-label="Sync Changes"]')).toBeNull(); + expect(checkoutControl()).toBeUndefined(); + expect(container.querySelector('[title="commit-a.png"]')).not.toBeNull(); + await click('[aria-label="Select commit"]'); + await click(`[data-value="${'b'.repeat(40)}"]`); + expect(container.querySelector('[title="commit-a.png"]')).toBeNull(); + await openFile('commit-b.png'); + expect(container.textContent).toContain('Commit · bbbbbbbb'); + const commitRequest = [...requests].reverse().find((url) => url.pathname === '/api/git/commit-diff'); + expect(commitRequest?.searchParams.get('hash')).toBe('b'.repeat(40)); + expect(commitRequest?.searchParams.get('previousPath')).toBe('old.png'); + expect(requests.filter((url) => url.pathname === '/api/git/range-diff').every((url) => url.searchParams.get('includeWorkingTree') === 'true')).toBe(true); + + await act(async () => { seed('/repo', { ...status, isClean: false, files: [{ path: 'working.png', index: 'M', working_dir: ' ' }] }); }); + initialDiff = { path: 'working.png', staged: true }; + await render(); + expect(container.querySelector('h2')?.textContent).toBe('working.png'); + const workingRequest = [...requests].reverse().find((url) => url.pathname === '/api/git/file-diff'); + expect(workingRequest?.searchParams.get('staged')).toBe('true'); + await click('[aria-label="Back"]'); + expect(container.querySelector('[aria-label="Select change mode"]')?.textContent).toBe('Changes'); + expect(container.querySelector('[aria-label="Sync Changes"]')).not.toBeNull(); + expect(checkoutControl()).toBeDefined(); + + await chooseMode('Branch'); + initialDiff = { path: 'working.png', staged: true }; + await render(); + expect(container.querySelector('h2')?.textContent).toBe('working.png'); + + await act(async () => { seed('/repo-two'); }); + directory = '/repo-two'; + await render(); + expect(container.querySelector('[aria-label="Select change mode"]')?.textContent).toBe('Changes'); + expect(container.querySelector('h2')).toBeNull(); + expect(requests.some((url) => url.pathname === '/api/git/file-diff' && url.searchParams.get('directory') === '/repo-two')).toBe(false); + } finally { + await act(async () => root.unmount()); + globalThis.fetch = originalFetch; + await dom.happyDOM.abort(); + for (const [name, descriptor] of originals) { + if (descriptor) Object.defineProperty(globalThis, name, descriptor); + else Reflect.deleteProperty(globalThis, name); + } + } +}); diff --git a/packages/ui/src/apps/MobileChangesSurface.tsx b/packages/ui/src/apps/MobileChangesSurface.tsx index 50c9c7b3..62e6037b 100644 --- a/packages/ui/src/apps/MobileChangesSurface.tsx +++ b/packages/ui/src/apps/MobileChangesSurface.tsx @@ -3,9 +3,15 @@ import { Icon } from '@/components/icon/Icon'; import { toast } from '@/components/ui'; import { Button } from '@/components/ui/button'; +import { DropdownMenu, DropdownMenuContent, DropdownMenuRadioGroup, DropdownMenuRadioItem, DropdownMenuTrigger } from '@/components/ui/dropdown-menu'; +import { dropdownTriggerVariants } from '@/components/ui/dropdown-trigger'; import { ScrollShadow } from '@/components/ui/ScrollShadow'; import { ChangesPanel, type ChangesGroupConfig } from '@/components/views/git/ChangesPanel'; import { BranchSelector } from '@/components/views/git/BranchSelector'; +import { BranchComparisonSelector } from '@/components/views/git/BranchComparisonSelector'; +import { CommitComparisonSelector } from '@/components/views/git/CommitComparisonSelector'; +import { branchRefLabel } from '@/components/views/git/baseBranch'; +import { isBranchScopeAvailable, isBranchScopeDefinitelyUnavailable, useRangeKeyedCache } from '@/components/views/branchDiffScope'; import { CommitSection } from '@/components/views/git/CommitSection'; import { DirtyBranchSwitchDialog } from '@/components/views/git/DirtyBranchSwitchDialog'; import { SyncActions } from '@/components/views/git/SyncActions'; @@ -13,6 +19,13 @@ import { PierreDiffViewer } from '@/components/views/PierreDiffViewer'; import { useRuntimeAPIs } from '@/hooks/useRuntimeAPIs'; import { useEffectiveDirectory } from '@/hooks/useEffectiveDirectory'; import { useNestedGitDirectory } from '@/hooks/useNestedGitDirectory'; +import { useBranchComparisonBase } from '@/hooks/useBranchComparisonBase'; +import { useCommitComparison } from '@/hooks/useCommitComparison'; +import { useGitComparison, type GitComparisonFile, type GitComparisonSource } from '@/hooks/useGitComparison'; +import { useGitBaseBranchStore } from '@/stores/useGitBaseBranchStore'; +import { FileTypeIcon } from '@/components/icons/FileTypeIcon'; +import { fileDiffFromPatch, isBinaryPatch } from '@/lib/diff/patchFileDiff'; +import type { FileDiffMetadata } from '@pierre/diffs'; import type { GitStatus } from '@/lib/api/types'; import { useI18n } from '@/lib/i18n'; import { generateCommitMessage, stageGitFile, stageGitFiles, unstageGitFile, unstageGitFiles } from '@/lib/gitApi'; @@ -32,6 +45,29 @@ import { getRuntimeKey } from '@/lib/runtime-switch'; type SyncAction = 'fetch' | 'pull' | 'push' | 'sync' | null; type CommitAction = 'commit' | 'commitAndPush' | null; +type ChangesMode = 'working' | 'branch' | 'commit'; +type ChangesRoute = + | { type: 'list' } + | { type: 'diff'; path: string; staged: boolean } + | { type: 'comparison'; path: string; sourceKey: string }; +interface ChangesNavigation { + ownerKey: string; + mode: ChangesMode; + route: ChangesRoute; +} +interface MobileDiffData { + original: string; + modified: string; + isBinary?: boolean; + fileDiff?: FileDiffMetadata; +} +type ComparisonDiff = + | { status: 'loading' } + | { status: 'ready'; diff: MobileDiffData } + | { status: 'error'; message: string }; +const LOADING_COMPARISON_DIFF: ComparisonDiff = { status: 'loading' }; +const LIST_ROUTE: ChangesRoute = { type: 'list' }; + const normalizePath = (value?: string | null): string => (value || '').replace(/\\/g, '/').replace(/\/+$/g, ''); const isStagedStatusFile = (file: GitStatus['files'][number]): boolean => { @@ -51,21 +87,32 @@ type MobileChangesSurfaceProps = { /** When provided, the list header gets a close X that calls this. */ onClose?: () => void; /** - * When set (and non-null), the surface opens directly into the per-file diff view for this - * relative path. Updating it (incl. setting it to a different path while open) routes the - * surface to that diff. Setting it back to null leaves the user on the current internal route. + * A new request object opens its working-tree diff, including repeated requests + * for the same path. Reopening the drawer keeps the same object and navigation. */ - initialDiffPath?: string | null; - initialDiffStaged?: boolean; + initialDiff?: { path: string; staged: boolean } | null; + /** The workspace drawer keeps visited panes mounted while hidden. */ + visible?: boolean; }; -export const MobileChangesSurface: React.FC = ({ onClose, initialDiffPath, initialDiffStaged = false }) => { +export const MobileChangesSurface: React.FC = (props) => { + const rootDirectory = normalizePath(useEffectiveDirectory() ?? null); + const repository = useNestedGitDirectory(rootDirectory || null, { enabled: props.visible ?? true }); + return ; +}; + +interface MobileChangesPaneProps extends MobileChangesSurfaceProps { + rootDirectory: string; + repository: ReturnType; +} + +/** Repository-scoped navigation and actions, separate from session directory resolution. */ +export const MobileChangesPane: React.FC = ({ rootDirectory, repository, onClose, initialDiff, visible = true }) => { const { t } = useI18n(); const { git } = useRuntimeAPIs(); - const rootDirectory = normalizePath(useEffectiveDirectory() ?? null); // When the root is not itself a repository, changes come from the resolved // nested repository instead. - const { rootIsGitRepo, gitDirectory, nestedRepos } = useNestedGitDirectory(rootDirectory || null); + const { rootIsGitRepo, gitDirectory, nestedRepos } = repository; const currentDirectory = gitDirectory ?? rootDirectory; const status = useGitStatus(currentDirectory || null); const branches = useGitBranches(currentDirectory || null); @@ -81,21 +128,41 @@ export const MobileChangesSurface: React.FC = ({ onCl const getDiff = useGitStore((state) => state.getDiff); const setDiff = useGitStore((state) => state.setDiff); - const [route, setRoute] = React.useState<{ type: 'list' } | { type: 'diff'; path: string; staged: boolean }>( - () => (initialDiffPath ? { type: 'diff', path: initialDiffPath, staged: initialDiffStaged } : { type: 'list' }), - ); + const runtimeKey = useGitStore((state) => state.runtimeKey); + const ownerKey = JSON.stringify([runtimeKey, currentDirectory]); + const ownerKeyRef = React.useRef(ownerKey); + ownerKeyRef.current = ownerKey; + const [navigation, setNavigation] = React.useState(() => ({ + ownerKey, + mode: 'working', + route: initialDiff?.path ? { type: 'diff', path: initialDiff.path, staged: initialDiff.staged } : LIST_ROUTE, + })); + const mode = navigation.ownerKey === ownerKey ? navigation.mode : 'working'; + const route = navigation.ownerKey === ownerKey ? navigation.route : LIST_ROUTE; + const [modeMenuOpen, setModeMenuOpen] = React.useState(false); + const changeMode = React.useCallback((nextMode: ChangesMode) => { + setNavigation({ ownerKey, mode: nextMode, route: LIST_ROUTE }); + setModeMenuOpen(false); + }, [ownerKey]); + const setRoute = React.useCallback((nextRoute: ChangesRoute) => { + setNavigation((current) => ({ ownerKey, mode: current.ownerKey === ownerKey ? current.mode : 'working', route: nextRoute })); + }, [ownerKey]); + + React.useEffect(() => { + setNavigation((current) => current.ownerKey === ownerKey ? current : { ownerKey, mode: 'working', route: LIST_ROUTE }); + setModeMenuOpen(false); + }, [ownerKey]); + React.useEffect(() => { if (!visible) setModeMenuOpen(false); }, [visible]); // Allow the host (MobileApp) to push us into a specific diff when the surface // is reopened or when an external trigger (e.g. PendingChangesBar tap) requests // a different file mid-session. React.useEffect(() => { - if (!initialDiffPath) return; - setRoute((current) => ( - current.type === 'diff' && current.path === initialDiffPath && current.staged === initialDiffStaged - ? current - : { type: 'diff', path: initialDiffPath, staged: initialDiffStaged } - )); - }, [initialDiffPath, initialDiffStaged]); + if (!initialDiff?.path) return; + // A new external target is a working-tree diff, regardless of the last + // comparison mode. Changing directories alone must not replay this target. + setNavigation({ ownerKey: ownerKeyRef.current, mode: 'working', route: { type: 'diff', path: initialDiff.path, staged: initialDiff.staged } }); + }, [initialDiff]); const [syncAction, setSyncAction] = React.useState(null); const [commitAction, setCommitAction] = React.useState(null); const [commitMessage, setCommitMessage] = React.useState(''); @@ -110,6 +177,54 @@ export const MobileChangesSurface: React.FC = ({ onCl const [diffRetryNonce, setDiffRetryNonce] = React.useState(0); const [pendingDirtySwitchBranch, setPendingDirtySwitchBranch] = React.useState(null); + const currentBranch = status?.current ?? null; + const trackingRemote = status?.tracking?.trim().split('/')[0]; + const defaultBranch = (trackingRemote && branches?.defaultBranches?.[trackingRemote]) ?? branches?.defaultBranches?.origin ?? null; + const showBranchOption = isBranchScopeAvailable(currentBranch, defaultBranch); + const branchUnavailable = isGitRepo === false || isBranchScopeDefinitelyUnavailable(currentBranch, defaultBranch, status !== null, branches !== null); + const setBaseOverride = useGitBaseBranchStore((state) => state.setOverride); + const branchComparison = useBranchComparisonBase(currentDirectory || null, currentBranch, visible && mode === 'branch' && showBranchOption); + const commitComparison = useCommitComparison(currentDirectory || null, currentBranch, visible && mode === 'commit' && isGitRepo === true); + const selectedCommitHash = commitComparison.selectedCommit?.hash ?? null; + const comparisonSource = React.useMemo(() => { + if (mode === 'branch' && currentBranch && branchComparison.base) return { kind: 'branch', baseRef: branchComparison.base, headRef: currentBranch }; + if (mode === 'commit' && selectedCommitHash) return { kind: 'commit', hash: selectedCommitHash }; + return null; + }, [branchComparison.base, currentBranch, mode, selectedCommitHash]); + const comparisonRevision = mode === 'branch' ? branchComparison.revision : ''; + const comparison = useGitComparison(currentDirectory || null, comparisonSource, visible && isGitRepo === true, comparisonRevision); + const { fetchDiff: loadComparisonDiff } = comparison; + const comparisonFiles = React.useMemo(() => comparison.files ? [...comparison.files].sort((a, b) => a.path.localeCompare(b.path)) : null, [comparison.files]); + const activeComparisonPath = route.type === 'comparison' && route.sourceKey === comparison.key ? route.path : null; + const [comparisonRetry, setComparisonRetry] = React.useState(0); + const fetchComparisonDiff = React.useCallback(async (path: string): Promise => { + try { + const { diff: patch } = await loadComparisonDiff(path); + const diff: MobileDiffData = { original: '', modified: '', isBinary: isBinaryPatch(patch) }; + if (!diff.isBinary) diff.fileDiff = fileDiffFromPatch(path, patch); + return { status: 'ready', diff }; + } catch (error) { + return { status: 'error', message: error instanceof Error ? error.message : t('diffView.state.failedToLoadDiff') }; + } + }, [loadComparisonDiff, t]); + const comparisonDiffs = useRangeKeyedCache( + comparison.files ? comparison.key : null, + visible && activeComparisonPath ? activeComparisonPath : '', + visible ? fetchComparisonDiff : null, + LOADING_COMPARISON_DIFF, + JSON.stringify([comparisonRevision, comparisonRetry]), + ); + const activeComparisonDiff = activeComparisonPath ? comparisonDiffs.get(activeComparisonPath) ?? LOADING_COMPARISON_DIFF : null; + + React.useEffect(() => { + if (mode === 'branch' && branchUnavailable) changeMode('working'); + }, [branchUnavailable, changeMode, mode]); + React.useEffect(() => { + setNavigation((current) => current.ownerKey === ownerKey && current.route.type === 'comparison' && current.route.sourceKey !== comparison.key + ? { ...current, route: LIST_ROUTE } + : current); + }, [comparison.key, ownerKey]); + const changeEntries = React.useMemo(() => { const files = status?.files ?? []; const unique = new Map(); @@ -199,7 +314,7 @@ export const MobileChangesSurface: React.FC = ({ onCl const handleCreateBranch = React.useCallback(async (branch: string, remote?: GitRemote) => { if (!currentDirectory) return; try { - await git.createBranch(currentDirectory, branch, status?.current ?? 'HEAD'); + await git.createBranch(currentDirectory, branch, currentBranch ?? 'HEAD'); await git.checkoutBranch(currentDirectory, branch); if (remote) { await git.gitPush(currentDirectory, { remote: remote.name, branch, options: ['--set-upstream'] }); @@ -209,7 +324,7 @@ export const MobileChangesSurface: React.FC = ({ onCl toast.error(error instanceof Error ? error.message : t('gitView.toast.createBranchFailed')); throw error; } - }, [currentDirectory, git, refreshStatusAndBranches, status?.current, t]); + }, [currentBranch, currentDirectory, git, refreshStatusAndBranches, t]); const refreshRemotes = React.useCallback(async () => { if (!currentDirectory) { @@ -231,17 +346,17 @@ export const MobileChangesSurface: React.FC = ({ onCl }, [currentDirectory, git]); React.useEffect(() => { - if (!currentDirectory) return; + if (!currentDirectory || !visible) return; setActiveDirectory(currentDirectory); void ensureAll(currentDirectory, git); - }, [currentDirectory, ensureAll, git, setActiveDirectory]); + }, [currentDirectory, ensureAll, git, setActiveDirectory, visible]); React.useEffect(() => { - void refreshRemotes(); - }, [refreshRemotes]); + if (visible) void refreshRemotes(); + }, [refreshRemotes, visible]); React.useEffect(() => { - if (!currentDirectory || changeEntries.length === 0) return; + if (!visible || mode !== 'working' || !currentDirectory || changeEntries.length === 0) return; const orderedPaths = Array.from(new Set([ ...stagedChangeEntries.map((entry) => entry.path), ...visibleChangePaths, @@ -252,9 +367,10 @@ export const MobileChangesSurface: React.FC = ({ onCl void prefetchDiffs(currentDirectory, git, orderedPaths, { maxFiles: 40 }); }, 120); return () => window.clearTimeout(timeoutId); - }, [changeEntries, currentDirectory, git, prefetchDiffs, stagedChangeEntries, visibleChangePaths]); + }, [changeEntries, currentDirectory, git, mode, prefetchDiffs, stagedChangeEntries, visibleChangePaths, visible]); React.useEffect(() => { + if (!visible) return; if (route.type !== 'diff') { setDiffLoadError(null); return; @@ -285,7 +401,7 @@ export const MobileChangesSurface: React.FC = ({ onCl return () => { cancelled = true; }; - }, [currentDirectory, diffRetryNonce, getDiff, git, route, setDiff]); + }, [currentDirectory, diffRetryNonce, getDiff, git, route, setDiff, visible]); const handleSyncAction = async (action: Exclude, remote?: GitRemote) => { if (!currentDirectory) return; @@ -349,7 +465,7 @@ export const MobileChangesSurface: React.FC = ({ onCl const handleViewChangeDiff = React.useCallback((path: string, staged = false) => { setRoute({ type: 'diff', path, staged }); - }, []); + }, [setRoute]); const handleRevertFile = React.useCallback(async (filePath: string) => { if (!currentDirectory) return; @@ -580,9 +696,62 @@ export const MobileChangesSurface: React.FC = ({ onCl ); } + const modeLabel = mode === 'branch' ? t('diffView.scope.branch') : mode === 'commit' ? t('commitComparison.mode') : t('mobile.nav.changes'); + const sourceLabel = mode === 'branch' && branchComparison.base + ? branchRefLabel(branchComparison.base) + : mode === 'commit' ? selectedCommitHash?.slice(0, 8) : null; + if (activeComparisonPath && activeComparisonDiff) { + return ( + file.path === activeComparisonPath)} + error={comparison.error ?? (activeComparisonDiff.status === 'error' ? activeComparisonDiff.message : null)} + onBack={() => setRoute(LIST_ROUTE)} + onRetry={() => { + if (comparison.error) void comparison.refresh(); + setComparisonRetry((value) => value + 1); + }} + /> + ); + } + + const renderComparison = () => { + if (mode === 'branch' && !branchComparison.base) { + return ; + } + if (mode === 'commit' && !selectedCommitHash) { + return
+ {commitComparison.loading && } +

{commitComparison.loading ? t('diffView.state.loadingChanges') : commitComparison.error ?? t('commitComparison.noCommits')}

+ {commitComparison.error && } +
; + } + if (comparison.error) { + return
+

{t('diffView.state.failedToLoadDiff')}

+

{comparison.error}

+ +
; + } + if (!comparisonFiles) return ; + if (comparisonFiles.length === 0) { + return ; + } + return { + if (comparison.key) setRoute({ type: 'comparison', path, sourceKey: comparison.key }); + }} />; + }; + return (
-
+
{onClose ? ( ) : null} -
-

{t('mobile.nav.changes')}

- void handleCheckoutBranch(branch)} - onCreate={handleCreateBranch} + + + + + + { + if (value === 'working' || value === 'branch' || value === 'commit') changeMode(value); + }}> + {t('mobile.nav.changes')} + {showBranchOption && {t('diffView.scope.branch')}} + {t('commitComparison.mode')} + + + + {visible && mode === 'branch' && ( + { if (currentBranch) setBaseOverride(currentDirectory, currentBranch, base); }} /> + )} + {visible && mode === 'commit' && ( + void commitComparison.refresh()} /> + )} +
+ {mode === 'working' && ( +
+
+ void handleCheckoutBranch(branch)} + onCreate={handleCreateBranch} + remotes={effectiveRemotes} + disabled={isLoadingStatus} + directory={currentDirectory} + switchBlockedNotice={(status?.files?.length ?? 0) > 0 ? t('gitView.branch.switchBlockedNotice') : null} + /> +
+ 0 ? t('gitView.branch.switchBlockedNotice') : null} + onFetch={(remote) => void handleSyncAction('fetch', remote)} + onSync={(remote) => void handleSyncAction('sync', remote)} + disabled={commitAction !== null || isLoadingStatus} + aheadCount={status?.ahead ?? 0} + behindCount={status?.behind ?? 0} + trackingRemoteName={status?.tracking?.split('/')[0]} + hasUncommittedChanges={changeEntries.length > 0} />
- void handleSyncAction('fetch', remote)} - onSync={(remote) => void handleSyncAction('sync', remote)} - disabled={commitAction !== null || isLoadingStatus} - aheadCount={status?.ahead ?? 0} - behindCount={status?.behind ?? 0} - trackingRemoteName={status?.tracking?.split('/')[0]} - hasUncommittedChanges={changeEntries.length > 0} - /> -
- {changeEntries.length > 0 ? ( + )} + {mode !== 'working' ? ( +
{renderComparison()}
+ ) : changeEntries.length > 0 ? (
{/* File list scrolls inside ChangesPanel; the commit footer stays pinned. */}
@@ -735,12 +937,13 @@ const MobileChangesState: React.FC<{ const MobileDiffDetail: React.FC<{ path: string; - diff: { original: string; modified: string; isBinary?: boolean } | null; + subtitle?: string; + diff: MobileDiffData | null; fileExists: boolean; error: string | null; onBack: () => void; onRetry: () => void; -}> = ({ path, diff, fileExists, error, onBack, onRetry }) => { +}> = ({ path, subtitle, diff, fileExists, error, onBack, onRetry }) => { const { t } = useI18n(); const language = React.useMemo(() => getLanguageFromExtension(path) || 'text', [path]); @@ -757,6 +960,7 @@ const MobileDiffDetail: React.FC<{

{path}

+ {subtitle &&

{subtitle}

}
@@ -774,7 +978,7 @@ const MobileDiffDetail: React.FC<{ ) : diff.isBinary ? ( - ) : isImageFile(path) ? ( + ) : isImageFile(path) && !diff.fileDiff ? ( ) : ( ); }; + +const MobileComparisonFileList: React.FC<{ files: GitComparisonFile[]; onSelect: (path: string) => void }> = ({ files, onSelect }) => { + const { t } = useI18n(); + return ( + +
    + {files.map((file) => { + const statusLabel = file.status === 'A' ? t('diffView.change.new') + : file.status === 'D' ? t('diffView.change.deleted') + : file.status === 'R' ? t('diffView.change.renamed') + : file.status === 'C' ? t('diffView.change.copied') : t('diffView.change.modified'); + const statusColor = file.status === 'A' ? 'var(--status-success)' : file.status === 'D' ? 'var(--status-error)' + : file.status === 'R' || file.status === 'C' ? 'var(--status-info)' : 'var(--status-warning)'; + return
  • + +
    • ; + })} +
+ + ); +}; diff --git a/packages/ui/src/apps/MobileWorkspaceDrawer.tsx b/packages/ui/src/apps/MobileWorkspaceDrawer.tsx index 441e852b..c219629d 100644 --- a/packages/ui/src/apps/MobileWorkspaceDrawer.tsx +++ b/packages/ui/src/apps/MobileWorkspaceDrawer.tsx @@ -236,14 +236,14 @@ export const MobileWorkspaceDrawer: React.FC<{ {visitedTabs.has('changes') ? (
diff --git a/packages/ui/src/components/ui/MobileOverlayPanel.tsx b/packages/ui/src/components/ui/MobileOverlayPanel.tsx index 56ded022..a59d0e20 100644 --- a/packages/ui/src/components/ui/MobileOverlayPanel.tsx +++ b/packages/ui/src/components/ui/MobileOverlayPanel.tsx @@ -117,6 +117,7 @@ export const MobileOverlayPanel: React.FC = ({ entered ? 'opacity-100' : 'opacity-0', )} role="dialog" + aria-label={title} aria-modal="true" onClick={onClose} // The panel centers over the CHAT column, not the whole app: on a tablet diff --git a/packages/ui/src/components/ui/dropdown-trigger.ts b/packages/ui/src/components/ui/dropdown-trigger.ts index 8761c546..074ed75b 100644 --- a/packages/ui/src/components/ui/dropdown-trigger.ts +++ b/packages/ui/src/components/ui/dropdown-trigger.ts @@ -12,6 +12,7 @@ import { cva } from 'class-variance-authority'; * Sizes: * - `sm` — dense surfaces: chat composer, toolbars, list rows (h-6). * - `default` — forms, dialogs, and settings pages (h-8). + * - `touch` — mobile value pickers (h-11). */ export const dropdownTriggerVariants = cva( [ @@ -27,6 +28,7 @@ export const dropdownTriggerVariants = cva( size: { sm: "h-6 min-h-6 px-2 [&_svg:not([class*='size-'])]:size-3.5", default: "h-8 min-h-8 px-3 [&_svg:not([class*='size-'])]:size-4", + touch: "h-11 min-h-11 px-3 [&_svg:not([class*='size-'])]:size-4", }, }, defaultVariants: { diff --git a/packages/ui/src/components/views/DiffView.tsx b/packages/ui/src/components/views/DiffView.tsx index 826bdc7e..1ad21e8d 100644 --- a/packages/ui/src/components/views/DiffView.tsx +++ b/packages/ui/src/components/views/DiffView.tsx @@ -2,6 +2,7 @@ import React from 'react'; import { useUIStore, type PendingDiffScope } from '@/stores/useUIStore'; import { useCommitComparison } from '@/hooks/useCommitComparison'; +import { useGitComparison, type GitComparisonSource } from '@/hooks/useGitComparison'; import { CommitComparisonSelector } from '@/components/views/git/CommitComparisonSelector'; import { useEffectiveDirectory } from '@/hooks/useEffectiveDirectory'; import { useNestedGitDirectory } from '@/hooks/useNestedGitDirectory'; @@ -11,11 +12,10 @@ import { branchRefLabel } from '@/components/views/git/baseBranch'; import { useGitStore, useGitStatus, useIsGitRepo, useGitLoadingStatus } from '@/stores/useGitStore'; import { useGitBaseBranchStore } from '@/stores/useGitBaseBranchStore'; import { useBranchComparisonBase } from '@/hooks/useBranchComparisonBase'; -import { coerceDiffScope, branchRangeKey, isBranchScopeAvailable, isBranchScopeDefinitelyUnavailable, useRangeKeyedCache, useBoundedDirectoryRetry } from './branchDiffScope'; -import { getGitRangeDiff, getGitRangeFiles, getCommitFiles, getGitCommitDiff } from '@/lib/gitApi'; +import { coerceDiffScope, isBranchScopeAvailable, isBranchScopeDefinitelyUnavailable, useRangeKeyedCache, useBoundedDirectoryRetry } from './branchDiffScope'; import { getRuntimeKey } from '@/lib/runtime-switch'; import { cn } from '@/lib/utils'; -import type { GitStatus, GitRangeFileEntry, CommitFileEntry } from '@/lib/api/types'; +import type { GitStatus } from '@/lib/api/types'; import { DropdownMenu, DropdownMenuContent, @@ -47,7 +47,7 @@ import { sessionEvents } from '@/lib/sessionEvents'; import { findDiffScrollAnchor, getRestoredDiffScrollTop, type DiffScrollAnchor } from './diffScrollAnchor'; import { useI18n } from '@/lib/i18n'; import type { I18nKey } from '@/lib/i18n/store'; -import { fileDiffFromPatch } from '@/lib/diff/patchFileDiff'; +import { fileDiffFromPatch, isBinaryPatch } from '@/lib/diff/patchFileDiff'; import { isVSCodeRuntime } from '@/lib/desktop'; import { startReviewFlow } from '@/lib/reviewFlow'; import { WALKTHROUGH_ACTION_CLASS } from '@/components/views/walkthrough/walkthroughAction'; @@ -196,9 +196,6 @@ const getFirstChangedModifiedLine = (original: string, modified: string): number return 1; }; -const isBinaryPatch = (patch: string): boolean => - /^Binary files .+ differ$/m.test(patch) || /^GIT binary patch$/m.test(patch); - const listTurnDiffs = (value: unknown): TurnSnapshotDiff[] => { if (!Array.isArray(value)) return []; return value.filter((diff): diff is TurnSnapshotDiff => { @@ -1163,29 +1160,6 @@ export const DiffView: React.FC = ({ const currentBranch = status?.current ?? null; const commitComparison = useCommitComparison(effectiveDirectory ?? null, currentBranch, activeDiffScope === 'commit' && !isVSCodeRuntime()); const selectedCommitHash = commitComparison.selectedCommit?.hash ?? null; - const commitQueryKey = activeDiffScope === 'commit' && effectiveDirectory && selectedCommitHash - ? JSON.stringify([getRuntimeKey(), effectiveDirectory, selectedCommitHash]) - : null; - const [commitFilesResult, setCommitFilesResult] = React.useState< - { key: string; files: CommitFileEntry[] } | { key: string; error: string } | null - >(null); - const [commitFilesRetry, setCommitFilesRetry] = React.useState(0); - const currentCommitFiles = commitFilesResult?.key === commitQueryKey ? commitFilesResult : null; - const commitFiles = currentCommitFiles && 'files' in currentCommitFiles ? currentCommitFiles.files : null; - const commitFilesError = currentCommitFiles && 'error' in currentCommitFiles ? currentCommitFiles.error : null; - const commitFilesByPath = React.useMemo(() => new Map((commitFiles ?? []).map((file) => [file.path, file])), [commitFiles]); - React.useEffect(() => { - if (!commitQueryKey || !effectiveDirectory || !selectedCommitHash) return; - let cancelled = false; - setCommitFilesResult(null); - getCommitFiles(effectiveDirectory, selectedCommitHash) - .then(({ files }) => { if (!cancelled) setCommitFilesResult({ key: commitQueryKey, files }); }) - .catch((error) => { - if (!cancelled) setCommitFilesResult({ key: commitQueryKey, error: error instanceof Error ? error.message : t('diffView.state.failedToLoadDiff') }); - }); - return () => { cancelled = true; }; - }, [commitFilesRetry, commitQueryKey, effectiveDirectory, selectedCommitHash, t]); - React.useEffect(() => { if (activeDiffScope === 'commit' && isVSCodeRuntime()) { setActiveDiffScope('working'); @@ -1271,50 +1245,23 @@ export const DiffView: React.FC = ({ } }, [activeDiffScope, branchScopeDefinitelyUnavailable, onDiffScopeChange]); - const branchQueryKey = effectiveDirectory && currentBranch && branchBase - ? JSON.stringify([getRuntimeKey(), effectiveDirectory, branchBase, currentBranch]) - : null; - const [branchFilesResult, setBranchFilesResult] = React.useState< - { key: string; files: GitRangeFileEntry[] } | { key: string; error: string } | null - >(null); - const currentBranchFilesResult = branchFilesResult?.key === branchQueryKey ? branchFilesResult : null; - const branchFiles = currentBranchFilesResult && 'files' in currentBranchFilesResult ? currentBranchFilesResult.files : null; - const branchFilesError = currentBranchFilesResult && 'error' in currentBranchFilesResult ? currentBranchFilesResult.error : null; - - // Shared by the scope/base effect and the error-state Retry button; the - // fetch id discards completions from a superseded run (base or head - // changed, or an earlier retry is still in flight). - const branchFilesFetchIdRef = React.useRef(0); - const reloadBranchFiles = React.useCallback(() => { - if (!effectiveDirectory || !currentBranch || !branchBase || !branchQueryKey) return; - const fetchId = branchFilesFetchIdRef.current + 1; - branchFilesFetchIdRef.current = fetchId; - setBranchFilesResult((previous) => previous?.key === branchQueryKey && 'files' in previous ? previous : null); - getGitRangeFiles(effectiveDirectory, { base: branchBase, head: currentBranch, includeWorkingTree: true }) - .then((files) => { - if (branchFilesFetchIdRef.current === fetchId) setBranchFilesResult({ key: branchQueryKey, files }); - }) - .catch((error) => { - if (branchFilesFetchIdRef.current === fetchId) { - setBranchFilesResult({ key: branchQueryKey, error: error instanceof Error ? error.message : t('diffView.branch.loadError') }); - } - }); - }, [branchBase, branchQueryKey, currentBranch, effectiveDirectory, t]); - - React.useEffect(() => { - if (activeDiffScope === 'branch') { - reloadBranchFiles(); - } - return () => { branchFilesFetchIdRef.current += 1; }; - }, [activeDiffScope, branchRevision, reloadBranchFiles]); + const comparisonSource = React.useMemo(() => { + if (activeDiffScope === 'commit' && selectedCommitHash) return { kind: 'commit', hash: selectedCommitHash }; + if (activeDiffScope === 'branch' && branchBase && currentBranch) return { kind: 'branch', baseRef: branchBase, headRef: currentBranch }; + return null; + }, [activeDiffScope, branchBase, currentBranch, selectedCommitHash]); + const comparison = useGitComparison(effectiveDirectory ?? null, comparisonSource, !isVSCodeRuntime(), activeDiffScope === 'branch' ? branchRevision : ''); + const { fetchDiff: loadComparisonDiff } = comparison; + const commitFiles = activeDiffScope === 'commit' ? comparison.files : null; + const commitFilesError = activeDiffScope === 'commit' ? comparison.error : null; + const branchFiles = activeDiffScope === 'branch' ? comparison.files : null; + const branchFilesError = activeDiffScope === 'branch' ? comparison.error : null; // Range diffs are fetched per expanded file: unlike working/staged diffs // there is no per-file cache channel, so patch data lives in a range-keyed // local cache. Stale completions from a previous range cannot write into // the new range's cache (see useRangeKeyedCache). - const comparisonRangeKey = activeDiffScope === 'commit' ? (commitFiles ? commitQueryKey : null) : activeDiffScope === 'branch' && effectiveDirectory && currentBranch && branchBase - ? branchRangeKey(effectiveDirectory, branchBase, currentBranch) + getRuntimeKey() - : null; + const comparisonRangeKey = comparison.files ? comparison.key : null; const comparisonPathsKey = React.useMemo( () => (activeDiffScope === 'branch' || activeDiffScope === 'commit' ? Array.from(expandedFiles).sort().join('\0') : ''), [activeDiffScope, expandedFiles] @@ -1322,30 +1269,14 @@ export const DiffView: React.FC = ({ const fetchComparisonDiffEntry = React.useCallback( async (filePath: string): Promise => { - if (!effectiveDirectory) return EMPTY_COMPARISON_DIFF; try { - if (activeDiffScope === 'commit' && selectedCommitHash) { - const response = await getGitCommitDiff(effectiveDirectory, { - hash: selectedCommitHash, path: filePath, - previousPath: commitFilesByPath.get(filePath)?.previousPath, - contextLines: loadFullFiles ? FULL_CONTEXT_DIFF_LINES : DEFAULT_CONTEXT_DIFF_LINES, - }); - return { status: 'ready', data: createTextDiffDataFromPatch(filePath, response.diff, loadFullFiles ? 'full' : 'patch') }; - } - if (!branchBase || !currentBranch) return EMPTY_COMPARISON_DIFF; - const response = await getGitRangeDiff(effectiveDirectory, { - base: branchBase, - head: currentBranch, - path: filePath, - includeWorkingTree: true, - contextLines: loadFullFiles ? FULL_CONTEXT_DIFF_LINES : DEFAULT_CONTEXT_DIFF_LINES, - }); + const response = await loadComparisonDiff(filePath, loadFullFiles ? FULL_CONTEXT_DIFF_LINES : DEFAULT_CONTEXT_DIFF_LINES); return { status: 'ready', data: createTextDiffDataFromPatch(filePath, response.diff, loadFullFiles ? 'full' : 'patch') }; } catch (error) { return { status: 'error', message: error instanceof Error ? error.message : t('diffView.state.failedToLoadDiff') }; } }, - [activeDiffScope, branchBase, commitFilesByPath, currentBranch, effectiveDirectory, loadFullFiles, selectedCommitHash, t] + [loadComparisonDiff, loadFullFiles, t] ); const comparisonDiffData = useRangeKeyedCache( @@ -1361,8 +1292,8 @@ export const DiffView: React.FC = ({ const changedFiles: FileEntry[] = React.useMemo(() => { if (activeDiffScope === 'commit') { return (commitFiles ?? []).map((file) => ({ - path: file.path, index: '', working_dir: file.changeType, - insertions: file.insertions, deletions: file.deletions, isNew: file.changeType === 'A', + path: file.path, index: '', working_dir: file.status, + insertions: file.insertions, deletions: file.deletions, isNew: file.status === 'A', })); } if (activeDiffScope === 'branch') { @@ -2033,7 +1964,7 @@ export const DiffView: React.FC = ({ if (commitFilesError) { return

{commitFilesError}

- +
; } if (!selectedCommitHash && !commitComparison.loading) { @@ -2076,7 +2007,7 @@ export const DiffView: React.FC = ({ diff --git a/packages/ui/src/components/views/branchDiffScope.test.ts b/packages/ui/src/components/views/branchDiffScope.test.ts index d9d033a8..49904cba 100644 --- a/packages/ui/src/components/views/branchDiffScope.test.ts +++ b/packages/ui/src/components/views/branchDiffScope.test.ts @@ -3,7 +3,6 @@ import { createRoot, type Root } from 'react-dom/client'; import { describe, expect, test } from 'bun:test'; import { - branchRangeKey, coerceDiffScope, isBranchScopeAvailable, isBranchScopeDefinitelyUnavailable, @@ -130,20 +129,6 @@ describe('isBranchScopeDefinitelyUnavailable', () => { }); }); -describe('branchRangeKey', () => { - test('distinguishes bases, heads, and directories for the same path', () => { - // The same file path can carry different diff content per range; a cache - // keyed by path alone would leak a previous branch's patch. - const keys = [ - branchRangeKey('/repo', 'main', 'feature-a'), - branchRangeKey('/repo', 'develop', 'feature-a'), - branchRangeKey('/repo', 'main', 'feature-b'), - branchRangeKey('/other', 'main', 'feature-a'), - ]; - expect(new Set(keys).size).toBe(4); - }); -}); - // --------------------------------------------------------------------------- // useRangeKeyedCache // --------------------------------------------------------------------------- diff --git a/packages/ui/src/components/views/branchDiffScope.ts b/packages/ui/src/components/views/branchDiffScope.ts index 8f503d93..355b3b80 100644 --- a/packages/ui/src/components/views/branchDiffScope.ts +++ b/packages/ui/src/components/views/branchDiffScope.ts @@ -62,15 +62,6 @@ export const coerceDiffScope = ( branchScopeAvailable: boolean ): T | 'working' => (scope === 'branch' && !branchScopeAvailable ? 'working' : scope); -/** - * Identity of one `base...head` range in one repository. Range-cache entries - * are only valid within a single range: the same file path can carry different - * content under a different base or head, so a cache keyed by path alone leaks - * stale patches across branch and base switches. - */ -export const branchRangeKey = (directory: string, base: string, head: string): string => - JSON.stringify([directory, base, head]); - /** * Bounded per-directory retry for a request whose failure leaves no result and * no signal beyond the in-flight flag settling back to false. diff --git a/packages/ui/src/components/views/git/BranchComparisonSelector.test.tsx b/packages/ui/src/components/views/git/BranchComparisonSelector.test.tsx index f6c244cd..60a35ba0 100644 --- a/packages/ui/src/components/views/git/BranchComparisonSelector.test.tsx +++ b/packages/ui/src/components/views/git/BranchComparisonSelector.test.tsx @@ -2,8 +2,9 @@ import React, { act, useState } from 'react'; import { test, expect } from 'bun:test'; import { Window } from 'happy-dom'; -test('allows repeated base selection with search and keyboard navigation', async () => { +const checkBranchSelection = async (mobile: boolean, tablet = false) => { const dom = new Window({ url: 'http://localhost' }); + if (mobile && !tablet) dom.happyDOM.setWindowSize({ width: 390, height: 844 }); const originals = new Map(); const globals = { window: dom, @@ -38,6 +39,7 @@ test('allows repeated base selection with search and keyboard navigation', async function Harness() { const [base, setBase] = useState(null); return root.render()); await act(async () => trigger().click()); + if (mobile) { + if (tablet) expect(document.querySelector('[role="dialog"]')).toBeNull(); + else expect(document.querySelector('[role="dialog"]')).not.toBeNull(); + if (!tablet) expect(document.activeElement).not.toBe(document.querySelector('input')); + } expect(document.querySelector('[data-value="refs/heads/feature"]')).toBeNull(); await press('ArrowDown'); const afterDown = selectedRef(); @@ -95,6 +102,10 @@ test('allows repeated base selection with search and keyboard navigation', async trigger().dispatchEvent(new KeyboardEvent('keydown', { key: 'n', ctrlKey: true, bubbles: true })); }); expect(document.querySelector('input')).toBeNull(); + await act(async () => trigger().click()); + await press('Escape'); + expect(document.querySelector('input')).toBeNull(); + expect(choices).toHaveLength(3); } finally { await act(async () => root.unmount()); await dom.happyDOM.abort(); @@ -103,4 +114,9 @@ test('allows repeated base selection with search and keyboard navigation', async else Reflect.deleteProperty(globalThis, name); } } -}); +}; + +for (const mobile of [false, true]) { + test(`allows repeated base selection with search and keyboard navigation (mobile=${mobile})`, () => checkBranchSelection(mobile)); +} +test('keeps the mobile branch picker anchored on tablets', () => checkBranchSelection(true, true)); diff --git a/packages/ui/src/components/views/git/BranchComparisonSelector.tsx b/packages/ui/src/components/views/git/BranchComparisonSelector.tsx index eabbacac..40e31b8f 100644 --- a/packages/ui/src/components/views/git/BranchComparisonSelector.tsx +++ b/packages/ui/src/components/views/git/BranchComparisonSelector.tsx @@ -4,7 +4,9 @@ import { Button } from '@/components/ui/button'; import { Command, CommandEmpty, CommandGroup, CommandInput, CommandItem, CommandList } from '@/components/ui/command'; import { DropdownMenu, DropdownMenuContent, DropdownMenuTrigger } from '@/components/ui/dropdown-menu'; import { dropdownTriggerVariants } from '@/components/ui/dropdown-trigger'; +import { MobileOverlayPanel } from '@/components/ui/MobileOverlayPanel'; import { useI18n } from '@/lib/i18n'; +import { useTabletLayout } from '@/lib/device'; import { rankByQuery } from '@/lib/search/fuzzySearch'; import { cn } from '@/lib/utils'; import { branchRefLabel } from './baseBranch'; @@ -14,69 +16,89 @@ interface BranchComparisonSelectorProps { currentBranch: string | null; base: string | null; onSelect: (ref: string) => void; + mobile?: boolean; } -export function BranchComparisonSelector({ branches, currentBranch, base, onSelect }: BranchComparisonSelectorProps) { +export function BranchComparisonSelector({ branches, currentBranch, base, onSelect, mobile = false }: BranchComparisonSelectorProps) { const { t } = useI18n(); + const tabletLayout = useTabletLayout(); + const useSheet = mobile && !tabletLayout.enabled; const [open, setOpen] = useState(false); const [search, setSearch] = useState(''); const label = base ? branchRefLabel(base) : t('gitView.pr.field.baseBranch'); - return ( - { - setOpen(nextOpen); - if (!nextOpen) setSearch(''); + const changeOpen = (nextOpen: boolean) => { + setOpen(nextOpen); + if (!nextOpen) setSearch(''); + }; + const trigger = ( + + ); + const picker = ( + { + if (event.key !== 'Escape') event.stopPropagation(); }}> - - - + + + {t('gitView.branch.empty')} + + {open && rankByQuery( + [...new Set(branches)] + .filter((name) => name !== currentBranch) + .sort() + .map((name) => ({ + ref: name.startsWith('remotes/') ? `refs/${name}` : `refs/heads/${name}`, + label: branchRefLabel(name), + })), + search, + (branch) => [branch.label], + ).map((branch) => ( + { + onSelect(branch.ref); + changeOpen(false); + }}> + {branch.label} + {(branch.ref === base || branch.label === base) && } + + ))} + + + + ); + if (useSheet) { + return <> + {trigger} + changeOpen(false)}> + {picker} + + ; + } + return ( + + {trigger} - { - if (event.key !== 'Escape') event.stopPropagation(); - }}> - - - {t('gitView.branch.empty')} - - {open && rankByQuery( - [...new Set(branches)] - .filter((name) => name !== currentBranch) - .sort() - .map((name) => ({ - ref: name.startsWith('remotes/') ? `refs/${name}` : `refs/heads/${name}`, - label: branchRefLabel(name), - })), - search, - (branch) => [branch.label], - ).map((branch) => ( - { - onSelect(branch.ref); - setOpen(false); - setSearch(''); - }}> - {branch.label} - {(branch.ref === base || branch.label === base) && } - - ))} - - - + {picker} ); diff --git a/packages/ui/src/components/views/git/CommitComparisonSelector.test.tsx b/packages/ui/src/components/views/git/CommitComparisonSelector.test.tsx index 3e81363c..da73285d 100644 --- a/packages/ui/src/components/views/git/CommitComparisonSelector.test.tsx +++ b/packages/ui/src/components/views/git/CommitComparisonSelector.test.tsx @@ -3,8 +3,9 @@ import { expect, test } from 'bun:test'; import { Window } from 'happy-dom'; import type { GitLogEntry } from '@/lib/api/types'; -test('shows commit metadata and shares repeated searched selections between two pickers', async () => { +const checkCommitSelection = async (mobile: boolean, tablet = false) => { const dom = new Window({ url: 'http://localhost' }); + if (mobile && !tablet) dom.happyDOM.setWindowSize({ width: 390, height: 844 }); const originals = new Map(); const globals = { window: dom, document: dom.document, navigator: dom.navigator, location: dom.location, @@ -34,7 +35,7 @@ test('shows commit metadata and shares repeated searched selections between two function Harness() { const [hash, setHash] = useState(null); return <>{['changes', 'walkthrough'].map((name) =>
- { refreshes += 1; }} onSelect={(commit) => { selected.push(commit.hash); setHash(commit.hash); }} />
)}; @@ -55,6 +56,11 @@ test('shows commit metadata and shares repeated searched selections between two try { await act(async () => root.render()); await act(async () => trigger('changes').click()); + if (mobile) { + if (tablet) expect(document.querySelector('[role="dialog"]')).toBeNull(); + else expect(document.querySelector('[role="dialog"]')).not.toBeNull(); + if (!tablet) expect(document.activeElement).not.toBe(input()); + } const first = document.querySelector('[cmdk-item]'); expect(first?.textContent).toContain('fix: first commit'); expect(first?.textContent).toContain('Test Author'); @@ -85,4 +91,9 @@ test('shows commit metadata and shares repeated searched selections between two else Reflect.deleteProperty(globalThis, name); } } -}); +}; + +for (const mobile of [false, true]) { + test(`shows commit metadata and shares repeated searched selections between two pickers (mobile=${mobile})`, () => checkCommitSelection(mobile)); +} +test('keeps the mobile commit picker anchored on tablets', () => checkCommitSelection(true, true)); diff --git a/packages/ui/src/components/views/git/CommitComparisonSelector.tsx b/packages/ui/src/components/views/git/CommitComparisonSelector.tsx index 19f34470..e936abd6 100644 --- a/packages/ui/src/components/views/git/CommitComparisonSelector.tsx +++ b/packages/ui/src/components/views/git/CommitComparisonSelector.tsx @@ -5,7 +5,9 @@ import { Button } from '@/components/ui/button'; import { Command, CommandEmpty, CommandGroup, CommandInput, CommandItem, CommandList } from '@/components/ui/command'; import { DropdownMenu, DropdownMenuContent, DropdownMenuTrigger } from '@/components/ui/dropdown-menu'; import { dropdownTriggerVariants } from '@/components/ui/dropdown-trigger'; +import { MobileOverlayPanel } from '@/components/ui/MobileOverlayPanel'; import { useI18n } from '@/lib/i18n'; +import { useTabletLayout } from '@/lib/device'; import { rankByQuery } from '@/lib/search/fuzzySearch'; import { formatDateTimeForPreference } from '@/lib/timeFormat'; import { useUIStore } from '@/stores/useUIStore'; @@ -18,61 +20,84 @@ interface CommitComparisonSelectorProps { error: string | null; onSelect: (commit: GitLogEntry) => void; onRefresh: () => void; + mobile?: boolean; } -export function CommitComparisonSelector({ commits, selectedHash, loading, error, onSelect, onRefresh }: CommitComparisonSelectorProps) { +export function CommitComparisonSelector({ commits, selectedHash, loading, error, onSelect, onRefresh, mobile = false }: CommitComparisonSelectorProps) { const { t } = useI18n(); + const tabletLayout = useTabletLayout(); + const useSheet = mobile && !tabletLayout.enabled; const timeFormat = useUIStore((state) => state.timeFormatPreference); const [open, setOpen] = useState(false); const [search, setSearch] = useState(''); + const changeOpen = (value: boolean) => { + setOpen(value); + if (!value) setSearch(''); + else if (!loading) onRefresh(); + }; + const trigger = ( + + ); + const picker = ( + { if (event.key !== 'Escape') event.stopPropagation(); }}> + + {loading ? ( +
+ {t('diffView.state.loadingChanges')} +
+ ) : error ? ( +
+ {t('commitComparison.loadError')} + {error} + +
+ ) : ( + + {t('commitComparison.noCommits')} + + {open && rankByQuery(commits, search, (commit) => [commit.message, commit.author_name, commit.hash]).map((commit) => ( + { onSelect(commit); changeOpen(false); }}> +
+
{commit.message}
+
+ + {commit.author_name} · {Number.isNaN(new Date(commit.date).getTime()) ? commit.date : formatDateTimeForPreference(new Date(commit.date), timeFormat, { + year: 'numeric', month: 'short', day: 'numeric', hour: '2-digit', minute: '2-digit', + })} + + · {commit.hash.slice(0, 8)} +
+
+ {commit.hash === selectedHash && } + + ))} + + + )} + + ); + if (useSheet) { + return <> + {trigger} + changeOpen(false)}> + {picker} + + ; + } return ( - { - setOpen(value); - if (!value) setSearch(''); - else if (!loading) onRefresh(); - }}> - - - + + {trigger} - { if (event.key !== 'Escape') event.stopPropagation(); }}> - - {loading ? ( -
- {t('diffView.state.loadingChanges')} -
- ) : error ? ( -
- {t('commitComparison.loadError')} - {error} - -
- ) : ( - - {t('commitComparison.noCommits')} - - {open && rankByQuery(commits, search, (commit) => [commit.message, commit.author_name, commit.hash]).map((commit) => ( - { onSelect(commit); setOpen(false); setSearch(''); }}> -
-
{commit.message}
-
- {commit.author_name} · {Number.isNaN(new Date(commit.date).getTime()) ? commit.date : formatDateTimeForPreference(new Date(commit.date), timeFormat, { - year: 'numeric', month: 'short', day: 'numeric', hour: '2-digit', minute: '2-digit', - })} · {commit.hash.slice(0, 8)} -
-
- {commit.hash === selectedHash && } - - ))} - - - )} - + {picker} ); diff --git a/packages/ui/src/hooks/useGitComparison.test.tsx b/packages/ui/src/hooks/useGitComparison.test.tsx new file mode 100644 index 00000000..95e2ec55 --- /dev/null +++ b/packages/ui/src/hooks/useGitComparison.test.tsx @@ -0,0 +1,135 @@ +import React, { act } from 'react'; +import { expect, test } from 'bun:test'; +import { Window } from 'happy-dom'; +import type { GitComparisonSource } from './useGitComparison'; + +test('comparison reads preserve scope, report failures, retry, and stop while hidden', async () => { + const dom = new Window({ url: 'http://localhost' }); + const originals = new Map(); + const globals = { + window: dom, document: dom.document, navigator: dom.navigator, location: dom.location, + localStorage: dom.localStorage, + Element: dom.Element, HTMLElement: dom.HTMLElement, Node: dom.Node, + Event: dom.Event, CustomEvent: dom.CustomEvent, + requestAnimationFrame: dom.requestAnimationFrame.bind(dom), + cancelAnimationFrame: dom.cancelAnimationFrame.bind(dom), IS_REACT_ACT_ENVIRONMENT: true, + }; + for (const [name, value] of Object.entries(globals)) { + originals.set(name, Object.getOwnPropertyDescriptor(globalThis, name)); + Object.defineProperty(globalThis, name, { configurable: true, writable: true, value }); + } + const originalFetch = globalThis.fetch; + const requests: Array<{ url: URL; resolve: (response: Response) => void }> = []; + globalThis.fetch = Object.assign((input: RequestInfo | URL) => { + const url = new URL(input instanceof Request ? input.url : String(input), 'http://localhost'); + // Hold unrelated app-store bootstrap outside this fixture. Only explicitly + // resolved comparison requests should publish data during these transitions. + if (url.pathname === '/api/fs/home' || url.pathname === '/api/session-folders') { + return new Promise(() => {}); + } + return new Promise((resolve) => { requests.push({ url, resolve }); }); + }, originalFetch); + const { createRoot } = await import('react-dom/client'); + const { I18nProvider } = await import('@/lib/i18n'); + const { useGitComparison } = await import('./useGitComparison'); + type Capture = { current: ReturnType | null }; + const captured: Capture = { current: null }; + let directory = '/repo-a'; + let source: GitComparisonSource = { kind: 'branch', baseRef: 'refs/heads/main', headRef: 'feature' }; + let enabled = false; + let revision = '1'; + const container = document.createElement('div'); + document.body.append(container); + const root = createRoot(container); + function Harness() { + captured.current = useGitComparison(directory, source, enabled, revision); + return null; + } + const current = () => { + if (!captured.current) throw new Error('Comparison did not render'); + return captured.current; + }; + const render = () => act(async () => { root.render(); }); + const finish = (index: number, response: Response) => act(async () => { requests[index].resolve(response); }); + try { + await render(); + expect(requests.map(({ url }) => url.pathname)).toEqual([]); + enabled = true; + await render(); + expect(requests).toHaveLength(1); + expect(requests[0].url.searchParams.get('base')).toBe('refs/heads/main'); + expect(requests[0].url.searchParams.get('includeWorkingTree')).toBe('true'); + await finish(0, Response.json({ files: [{ path: 'a.ts', status: 'M' }] })); + expect(current().files?.map((file) => file.path)).toEqual(['a.ts']); + const oldRefresh = current().refresh; + const oldFetchDiff = current().fetchDiff; + + const patch = current().fetchDiff('a.ts'); + await act(async () => { await Promise.resolve(); }); + expect(requests[1].url.pathname).toBe('/api/git/range-diff'); + expect(requests[1].url.searchParams.get('includeWorkingTree')).toBe('true'); + await finish(1, Response.json({ diff: 'branch patch' })); + expect(await patch).toEqual({ diff: 'branch patch' }); + + revision = '2'; + await render(); + expect(current().files?.map((file) => file.path)).toEqual(['a.ts']); + await finish(2, Response.json({ error: 'snapshot failed' }, { status: 500 })); + expect(current().files).toBeNull(); + expect(current().error).toBe('snapshot failed'); + let retry: Promise | undefined; + await act(async () => { retry = current().refresh(); }); + await finish(3, Response.json({ files: [] })); + await retry; + expect(current().files).toEqual([]); + expect(current().error).toBeNull(); + + source = { kind: 'commit', hash: 'a'.repeat(40) }; + await render(); + expect(current().files).toBeNull(); + expect(requests[4].url.pathname).toBe('/api/git/commit-files'); + await finish(4, Response.json({ files: [{ path: 'new.ts', previousPath: 'old.ts', changeType: 'R', insertions: 1, deletions: 1, isBinary: false }] })); + const commitPatch = current().fetchDiff('new.ts', 20); + await act(async () => { await Promise.resolve(); }); + expect(requests[5].url.pathname).toBe('/api/git/commit-diff'); + expect(requests[5].url.searchParams.get('hash')).toBe('a'.repeat(40)); + expect(requests[5].url.searchParams.get('previousPath')).toBe('old.ts'); + expect(requests[5].url.searchParams.get('context')).toBe('20'); + await finish(5, Response.json({ diff: 'commit patch' })); + expect(await commitPatch).toEqual({ diff: 'commit patch' }); + await oldRefresh(); + await expect(oldFetchDiff('a.ts')).rejects.toThrow(); + expect(requests).toHaveLength(6); + + source = { kind: 'branch', baseRef: 'main', headRef: 'feature' }; + await render(); + directory = '/repo-b'; + await render(); + await finish(7, Response.json({ files: [{ path: 'b.ts', status: 'A' }] })); + await finish(6, Response.json({ files: [{ path: 'stale.ts', status: 'D' }] })); + expect(current().files?.map((file) => file.path)).toEqual(['b.ts']); + + const refreshBeforeHide = current().refresh; + const fetchBeforeHide = current().fetchDiff; + enabled = false; + revision = '3'; + await render(); + await current().refresh(); + await refreshBeforeHide(); + await expect(fetchBeforeHide('b.ts')).rejects.toThrow(); + expect(requests).toHaveLength(8); + expect(current().files?.map((file) => file.path)).toEqual(['b.ts']); + enabled = true; + await render(); + expect(requests).toHaveLength(9); + await finish(8, Response.json({ files: [{ path: 'b.ts', status: 'A' }] })); + } finally { + await act(async () => root.unmount()); + globalThis.fetch = originalFetch; + await dom.happyDOM.abort(); + for (const [name, descriptor] of originals) { + if (descriptor) Object.defineProperty(globalThis, name, descriptor); + else Reflect.deleteProperty(globalThis, name); + } + } +}); diff --git a/packages/ui/src/hooks/useGitComparison.ts b/packages/ui/src/hooks/useGitComparison.ts new file mode 100644 index 00000000..57268205 --- /dev/null +++ b/packages/ui/src/hooks/useGitComparison.ts @@ -0,0 +1,79 @@ +import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; +import type { GitDiffResponse } from '@/lib/api/types'; +import { getCommitFiles, getGitCommitDiff, getGitRangeDiff, getGitRangeFiles } from '@/lib/gitApi'; +import { useI18n } from '@/lib/i18n'; +import { getRuntimeKey } from '@/lib/runtime-switch'; +import type { WalkthroughSource } from '@/lib/walkthrough/types'; +import { useGitStore } from '@/stores/useGitStore'; + +export type GitComparisonSource = Extract; + +export interface GitComparisonFile { + path: string; + status: string; + previousPath?: string; + insertions: number; + deletions: number; +} + +type ComparisonFiles = + | { key: string; status: 'loading' } + | { key: string; status: 'ready'; files: GitComparisonFile[] } + | { key: string; status: 'error'; message: string }; + +/** File-list authority shared by the stacked desktop view and mobile drill-down. */ +export function useGitComparison(directory: string | null, source: GitComparisonSource | null, enabled = true, revision = '') { + const { t } = useI18n(); + const runtimeKey = useGitStore((state) => state.runtimeKey); + const key = directory && source ? JSON.stringify([runtimeKey, directory, source]) : null; + const sourceRef = useRef({ key, source, enabled }); + sourceRef.current = { key, source, enabled }; + const [result, setResult] = useState(null); + const generation = useRef(0); + + const refresh = useCallback(async () => { + const { key: targetKey, source: target, enabled: active } = sourceRef.current; + if (!enabled || !active || !key || targetKey !== key || !directory || !target) return; + const request = ++generation.current; + const runtime = getRuntimeKey(); + setResult((previous) => previous?.key === key && previous.status === 'ready' ? previous : { key, status: 'loading' }); + try { + const files: GitComparisonFile[] = target.kind === 'branch' + ? (await getGitRangeFiles(directory, { base: target.baseRef, head: target.headRef, includeWorkingTree: true })) + .map((file) => ({ ...file, insertions: 0, deletions: 0 })) + : (await getCommitFiles(directory, target.hash)).files + .map((file) => ({ path: file.path, status: file.changeType, previousPath: file.previousPath, insertions: file.insertions, deletions: file.deletions })); + if (generation.current !== request || getRuntimeKey() !== runtime) return; + setResult({ key, status: 'ready', files }); + } catch (error) { + if (generation.current !== request || getRuntimeKey() !== runtime) return; + setResult({ key, status: 'error', message: error instanceof Error ? error.message : t('diffView.state.failedToLoadDiff') }); + } + }, [directory, enabled, key, t]); + + useEffect(() => { + void refresh(); + return () => { generation.current += 1; }; + }, [refresh, revision]); + + const current = result?.key === key ? result : null; + const files = current?.status === 'ready' ? current.files : null; + const filesByPath = useMemo(() => new Map((files ?? []).map((file) => [file.path, file])), [files]); + const fetchDiff = useCallback(async (filePath: string, contextLines = 3): Promise => { + const { key: targetKey, source: target, enabled: active } = sourceRef.current; + const file = filesByPath.get(filePath); + if (!directory || targetKey !== key || !target || !file || !enabled || !active) throw new Error(t('diffView.state.failedToLoadDiff')); + return target.kind === 'branch' + ? getGitRangeDiff(directory, { base: target.baseRef, head: target.headRef, path: filePath, contextLines, includeWorkingTree: true }) + : getGitCommitDiff(directory, { hash: target.hash, path: filePath, previousPath: file.previousPath, contextLines }); + }, [directory, enabled, filesByPath, key, t]); + + return { + key, + files, + loading: Boolean(enabled && key && (!current || current.status === 'loading')), + error: current?.status === 'error' ? current.message : null, + refresh, + fetchDiff, + }; +} diff --git a/packages/ui/src/lib/diff/patchFileDiff.ts b/packages/ui/src/lib/diff/patchFileDiff.ts index 304d6f0a..7b2988d8 100644 --- a/packages/ui/src/lib/diff/patchFileDiff.ts +++ b/packages/ui/src/lib/diff/patchFileDiff.ts @@ -10,6 +10,9 @@ const PATCH_DIFF_CACHE_LIMIT = 64; const DEFAULT_PATCH_CONTEXT_LINES = 3; const patchFileDiffCache = new Map(); +export const isBinaryPatch = (patch: string): boolean => + /^Binary files .+ differ$/m.test(patch) || /^GIT binary patch$/m.test(patch); + export const fileDiffFromPatch = ( file: string, patch: string, diff --git a/packages/ui/src/stores/DOCUMENTATION.md b/packages/ui/src/stores/DOCUMENTATION.md index 47be2516..920d591d 100644 --- a/packages/ui/src/stores/DOCUMENTATION.md +++ b/packages/ui/src/stores/DOCUMENTATION.md @@ -42,8 +42,8 @@ refresh attempt per opening, so a failed first load cannot create a retry loop. ### UI state stores -`useCommitSelectionStore.ts` shares the selected commit between Changes and -walkthrough. Choices are session-only and keyed by runtime, directory, and +`useCommitSelectionStore.ts` shares the selected commit between desktop/mobile +Changes and walkthrough. Choices are session-only and keyed by runtime, directory, and checked-out branch, with at most 100 remembered choices. The picker history belongs to `useCommitComparison`, loads only while Commit mode is active, and is limited to the latest 50 commits. History failure stays distinct from an @@ -51,6 +51,15 @@ empty list; stale directory/runtime requests cannot replace current history or selection. A refreshed list preserves an explicit selection even when newer commits have pushed it beyond the latest 50. +`hooks/useGitComparison.ts` owns the local file-list state used by desktop and +mobile comparisons. Its key contains runtime, directory, and the complete +branch/commit source. A source change hides the old list immediately; failed +reads remain errors, and manual retries cannot publish into a superseded scope. +The hook also resolves per-file patch requests, including a commit rename's +previous path. Views own their lazy patch caches through `useRangeKeyedCache`. +Mobile requests only the active detail path and suspends reads while its +keep-alive workspace pane is hidden. + Examples: - `useUIStore.ts` diff --git a/packages/ui/src/styles/mobile.css b/packages/ui/src/styles/mobile.css index b798db77..b1fc8b31 100644 --- a/packages/ui/src/styles/mobile.css +++ b/packages/ui/src/styles/mobile.css @@ -56,6 +56,12 @@ min-width: 36px; } + /* The Changes section and source pickers take their height from dropdownTriggerVariants, + rather than the generic mobile button minimum. */ + :root.mobile-pointer:not(.desktop-runtime) button[data-mobile-comparison-trigger] { + min-height: 0; + } + /* Composer footer action buttons (sessions / attach / auto-accept): hug the icon so the group stays tight. The container only renders in the mobile JSX, so no pointer/runtime gating is needed; !important overrides both the diff --git a/packages/web/server/lib/git/DOCUMENTATION.md b/packages/web/server/lib/git/DOCUMENTATION.md index f67528df..1017c1bb 100644 --- a/packages/web/server/lib/git/DOCUMENTATION.md +++ b/packages/web/server/lib/git/DOCUMENTATION.md @@ -134,7 +134,7 @@ The following functions are internal helpers used by exported functions: ### Runtime availability of range diffs - `GET /api/git/range-diff` is served by the OpenChamber web server, so it is available to web, desktop, and mobile clients. The shared `GitAPI.getGitRangeDiff` is therefore optional: web supplies the HTTP implementation, and VS Code does not implement it because the extension host serves Git through its own bridge rather than these routes. Features built on range diffs (currently the AI diff walkthrough) are not offered in VS Code. -- Commit comparison uses the same server boundary through optional `GitAPI.getGitCommitDiff`. The shared Changes toolbar and walkthrough expose it on their existing desktop/tablet surfaces. The phone-specific Changes surface and the VS Code Git bridge keep their existing modes; Commit mode is not offered there. The HTTP operation is available to web, Electron, hosted mobile, and Capacitor clients. +- Commit comparison uses the same server boundary through optional `GitAPI.getGitCommitDiff`. Desktop Changes, mobile Changes, and the existing walkthrough surface share branch/commit comparison semantics. Mobile Changes uses the same selectors and `useGitComparison` file-list owner, with a read-only list-to-detail flow. VS Code keeps its existing modes because its Git bridge does not provide these comparison operations. The HTTP operations are available to web, Electron, hosted mobile, and Capacitor clients. ### Staged and unstaged change handling - `status.files` exposes both `index` and `working_dir` codes. Shared UI uses these as separate scopes: staged rows are derived from non-empty `index` statuses, while unstaged rows are derived from `working_dir` statuses and untracked files.