diff --git a/CLAUDE.md b/CLAUDE.md index 91607d5a..d20bf5da 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -24,7 +24,9 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co - **Settings**: `src/settings.ts` defines `GitLabFilesPushSettings` interface, `DEFAULT_SETTINGS` object, and `GitLabSyncSettingTab` for the Obsidian UI. - **Services**: `src/services/` abstracts the git provider behind `GitServiceInterface`, with `GitHubService` and `GitLabService` implementations sharing common logic via `BaseGitService`. - **Sync logic**: `src/logic/sync-manager.ts` handles push/pull, conflict detection, and rename detection; `src/logic/gitignore-manager.ts` merges local and remote `.gitignore` rules. -- **UI**: `src/ui/SyncStatusView.ts` renders the sync status side panel; `src/ui/components/` holds its sub-views. +- **UI**: the production Source Control surface is `SourceControlItemView` (`src/ui/source-control/SourceControlItemView.ts`), which renders `SourceControlView` (`src/ui/source-control/SourceControlView.ts`). User intent (push/pull/delete-remote/resolve-conflict) flows through `SourceControlActionService` (`src/logic/source-control/SourceControlActionService.ts`) into `SyncWorkspace` (`src/logic/sync/SyncWorkspace.ts`), which drives `SyncManager` and its executors (`PushExecutor`, `PullExecutor`, `RemoteDeleteExecutor`, etc. in `src/logic/sync/`). `src/ui/components/` holds shared diff/change presentation pieces used by this surface. + - Do not reintroduce `SyncStatusView` or `ui/sync-status/*` — that legacy presentation layer was replaced by the Source Control surface above and is blocked by an ESLint `no-restricted-imports` rule (`eslint.config.*`). The historical migration docs live in `docs/source-control-refactor/` and are marked as such; they are not current implementation guidance. + - `SOURCE_CONTROL_VIEW_TYPE` (`'sync-status-view'`) and the `open-sync-status` command id are intentionally kept as-is for pinned-leaf/workspace-layout compatibility — they resolve to the current `SourceControlItemView`, not a leftover of the old UI. Do not rename them as "cleanup." - **Bundling**: Uses `esbuild.config.mjs` for compilation from TypeScript to a single `main.js` file. - **Deployment**: Relies on `manifest.json` for plugin metadata and `versions.json` for version mapping/compatibility. diff --git a/docs/source-control-refactor/phase-1-viewmodel-foundation.md b/docs/source-control-refactor/phase-1-viewmodel-foundation.md index ca14c94d..cc23b501 100644 --- a/docs/source-control-refactor/phase-1-viewmodel-foundation.md +++ b/docs/source-control-refactor/phase-1-viewmodel-foundation.md @@ -1,5 +1,8 @@ # Phase 1 — Source Control ViewModel Foundation +> **Historical migration roadmap. Do not use as current implementation +> guidance.** See `docs/source-control.md` for the current architecture. + ## Goal 建立 Source Control UI 與 Sync domain 之間的 ViewModel layer。 diff --git a/docs/source-control-refactor/phase-2-action-unification.md b/docs/source-control-refactor/phase-2-action-unification.md index f9623bdd..11b09ad9 100644 --- a/docs/source-control-refactor/phase-2-action-unification.md +++ b/docs/source-control-refactor/phase-2-action-unification.md @@ -1,5 +1,8 @@ # Phase 2 — Sync Action Unification +> **Historical migration roadmap. Do not use as current implementation +> guidance.** See `docs/source-control.md` for the current architecture. + ## Goal 統一 Source Control、Context Menu、Single File 操作的 pipeline。 diff --git a/docs/source-control-refactor/phase-3-source-control-ui.md b/docs/source-control-refactor/phase-3-source-control-ui.md index f9299175..1f89e9ec 100644 --- a/docs/source-control-refactor/phase-3-source-control-ui.md +++ b/docs/source-control-refactor/phase-3-source-control-ui.md @@ -1,5 +1,8 @@ # Phase 3 — Source Control UI +> **Historical migration roadmap. Do not use as current implementation +> guidance.** See `docs/source-control.md` for the current architecture. + ## Goal 建立 VS Code style Source Control workflow。 diff --git a/docs/source-control-refactor/phase-4-legacy-cleanup.md b/docs/source-control-refactor/phase-4-legacy-cleanup.md index a1da7103..56b4db12 100644 --- a/docs/source-control-refactor/phase-4-legacy-cleanup.md +++ b/docs/source-control-refactor/phase-4-legacy-cleanup.md @@ -1,5 +1,8 @@ # Phase 4 — Legacy Cleanup +> **Historical migration roadmap. Do not use as current implementation +> guidance.** See `docs/source-control.md` for the current architecture. + ## Goal 移除舊 Source Control orchestration，保留同步核心能力。 diff --git a/docs/source-control-refactor/roadmap.md b/docs/source-control-refactor/roadmap.md index a928ff3e..d5cc3fd8 100644 --- a/docs/source-control-refactor/roadmap.md +++ b/docs/source-control-refactor/roadmap.md @@ -1,8 +1,12 @@ # Source Control Refactor — Roadmap (v2) +> **Historical migration roadmap. Do not use as current implementation +> guidance.** The migration this document tracked has landed on `main`; for +> the current architecture see `docs/source-control.md`. + > Supersedes `phase-1..4-*.md`. Those phase docs are kept only as historical -> design notes; this file is the authoritative current plan, grounded in the -> actual branch state as of 2026-08-22. +> design notes; this file was the authoritative current plan as of +> 2026-08-22, before the migration it tracked landed on `main`. ## Where we actually are diff --git a/docs/source-control.md b/docs/source-control.md new file mode 100644 index 00000000..93ed5fb4 --- /dev/null +++ b/docs/source-control.md @@ -0,0 +1,44 @@ +# Source Control — Current Architecture + +The Source Control side panel is the plugin's only sync UI. There is no +separate "sync status" view; `docs/source-control-refactor/` describes the +historical migration into this architecture and is not current guidance. + +## Call chain + +``` +SourceControlItemView (src/ui/source-control/SourceControlItemView.ts) + └─ SourceControlView (src/ui/source-control/SourceControlView.ts) + └─ SourceControlActionService (src/logic/source-control/SourceControlActionService.ts) + └─ SyncWorkspace (src/logic/sync/SyncWorkspace.ts) + └─ SyncManager + executors (src/logic/sync/, e.g. PushExecutor, + PullExecutor, RemoteDeleteExecutor) +``` + +- `SourceControlItemView` is the `ItemView` Obsidian mounts; it owns no + rendering logic itself and delegates to `SourceControlView`. +- `SourceControlView` renders the change tree, Sync Queue, and diff surfaces + (`src/ui/components/`, `src/ui/source-control/DiffTabView.ts`), and turns + clicks into calls on `SourceControlActionService`. +- `SourceControlActionService` converts Source Control intent (push / pull / + delete-remote / delete-local / resolve-conflict) into `SyncWorkspace` calls + and reports outcome via `OperationState`. It never talks to a git provider + directly. +- `SyncWorkspace` is the execution boundary: it drives the real `SyncManager` + and provider-mutating executors (`PushExecutor`, `PullExecutor`, + `RemoteDeleteExecutor`, etc.), which in turn call `GitServiceInterface` + (`src/services/`). + +## Compatibility identifiers (do not remove) + +- `SOURCE_CONTROL_VIEW_TYPE = 'sync-status-view'` — kept so pinned leaves and + saved workspace layouts from before the Source Control migration resolve to + the current `SourceControlItemView` instead of breaking. +- The `open-sync-status` command id — same reason; it already routes to + `activateSourceControlView()`. + +## Legacy surface (removed, do not reintroduce) + +`SyncStatusView` and `ui/sync-status/*` were the pre-migration UI and no +longer exist in `src/`. An ESLint `no-restricted-imports` rule +(`eslint.config.*`) blocks reintroducing imports from those paths. diff --git a/e2e-tests/provider/suites/sync-manager.e2e.test.ts b/e2e-tests/provider/suites/sync-manager.e2e.test.ts index 57f16cce..718c0688 100644 --- a/e2e-tests/provider/suites/sync-manager.e2e.test.ts +++ b/e2e-tests/provider/suites/sync-manager.e2e.test.ts @@ -4,12 +4,20 @@ import { SyncPlanModal, SyncPlanDirection } from '../../../src/ui/SyncPlanModal' import { BatchConflictResolutionModal } from '../../../src/ui/BatchConflictResolutionModal'; import { ObsidianSyncInteraction } from '../../../src/ui/ObsidianSyncInteraction'; import { describePushResult } from '../support/push-result-diagnostic'; +import { SyncManagerWorkspace } from '../../../src/logic/sync/SyncWorkspace'; +import { SourceControlActionService } from '../../../src/logic/source-control/SourceControlActionService'; +import { ChangeRepository } from '../../../src/logic/source-control/ChangeRepository'; +import { OperationState } from '../../../src/logic/source-control/OperationState'; +import { toChangeId } from '../../../src/logic/source-control/types'; // `import type` deliberately, not a value import: src/settings.ts also // exports settings-tab UI (GitLabSyncSettingTab -> FolderSuggest -> // AbstractInputSuggest etc.) which pulls in far more of `obsidian` than this // suite's minimal runtime shim provides. A type-only import is erased // entirely, so none of that module ever loads. import type { GitLabFilesPushSettings } from '../../../src/settings'; +import type { SyncStatusRefreshService } from '../../../src/logic/sync/SyncStatusRefreshService'; +import type { SyncDiffService } from '../../../src/logic/sync/SyncDiffService'; +import type { App } from 'obsidian'; import { TFile as ObsidianTFile } from 'obsidian'; import { GitVerifier } from '../support/git-verifier'; import { FakeVault, fakeApp, type TFileLike, type TFileCtor } from '../shim/fake-vault'; @@ -222,9 +230,14 @@ describe('SyncManager E2E', () => { expect(headAfterParent).toBe(headBefore); }); - it('deletes a file via the real service, verified independently', async () => { - // Deletion isn't a SyncManager method -- src/ui/SyncStatusView.ts calls - // gitService.deleteFile directly, so this reproduces that real path. + it('deletes a file via the current Source Control application path, verified independently', async () => { + // Deletion isn't a SyncManager method -- the production call chain is + // SourceControlActionService.deleteRemote() -> SyncWorkspace.deleteRemote() + // -> RemoteDeleteExecutor -> gitService.deleteFile(), not a direct + // provider call, so this exercises that full chain instead of + // bypassing it. `refreshService`/`diffService`/`app` are stubbed -- + // deleteRemote() never touches them -- the same pattern + // tests/logic/sync/SyncWorkspace.test.ts uses for its deleteRemote suite. const filePath = path('to-delete.md'); const vault = new FakeVault(TFile); vault.writeLocal(filePath, 'delete me'); @@ -235,9 +248,24 @@ describe('SyncManager E2E', () => { expect(initialPush.failed, describePushResult(initialPush)).toBe(0); expect(await verifier.fileMissing(filePath, branch)).toBe(false); - await service.deleteFile(filePath, branch, 'e2e: delete file'); - await manager.clearMetadata(filePath); + const changeId = toChangeId(filePath); + const repository = new ChangeRepository(); + repository.replace([{ id: changeId, path: filePath, kind: 'remote-only' }]); + const operations = new OperationState(); + const workspace = new SyncManagerWorkspace({ + manager: () => manager, + gitService: () => service, + settings: () => settings, + refreshService: {} as SyncStatusRefreshService, + diffService: {} as SyncDiffService, + normalizePath: p => p, + app: {} as App, + }); + const actionService = new SourceControlActionService(repository, operations, workspace); + + await actionService.deleteRemote([changeId]); + expect(operations.get(changeId)).toBe('success'); expect(await verifier.fileMissing(filePath, branch)).toBe(true); expect(settings.syncMetadata[filePath]).toBeUndefined(); }); diff --git a/src/ui/components/DiffViewer.ts b/src/ui/components/DiffViewer.ts index 1d8e5f4d..1f5587f7 100644 --- a/src/ui/components/DiffViewer.ts +++ b/src/ui/components/DiffViewer.ts @@ -35,43 +35,75 @@ export function resetDiffLayoutMemoryForTests(): void { } export interface DiffViewerOptions { - remote: string; - local: string; + /** + * Diff content. Omit both when the content isn't loaded yet (e.g. an + * async fetch is in flight) — the viewer renders just the empty body, + * and the caller fills it in later via the returned handle's + * `setContent`. Passing only one of the two is not supported. + */ + remote?: string; + local?: string; layout: DiffLayout; /** * Where the split/unified toggle renders — typically the fixed header * region of the enclosing surface, so it stays reachable while a long * diff scrolls below. When omitted, no toggle is rendered and the viewer - * shows the given layout statically. + * shows the given layout statically. Also suppressed on phones (see + * `renderDiffViewer` doc) regardless of this option. */ toggleHost?: HTMLElement; /** State-sync callback so the owning surface can persist the layout across its own re-renders. */ onLayoutChange?: (next: DiffLayout) => void; } +/** Handle to the diff body a `renderDiffViewer` call created, for filling in content that wasn't ready yet at render time. */ +export interface DiffViewerHandle { + /** Replaces the body's content. Safe to call once the initial (possibly content-less) render has happened. */ + setContent(remote: string, local: string): void; +} + /** * The shared diff-viewer composition: layout toggle + body layout class + * diff panel. Every diff surface (desktop diff tab, conflict modal, mobile * detail) renders through this instead of reassembling DiffLayoutToggle and * DiffPanel — and never rebuilds its own "apply layout class + re-render - * toggle" dance. + * toggle" dance. This is also the single owner of the diff body element: + * callers that load content asynchronously must go through the returned + * handle's `setContent` rather than reaching into the DOM and calling + * `renderDiffPanel` themselves, which would append a second copy alongside + * whatever this function already rendered. + * + * Phones (`Platform.isPhone`) always render unified with no toggle — a + * split view is unreadable at phone width, and offering a toggle that + * produces two ~150px columns is worse than not offering it. Tablets and + * desktop keep the caller's requested layout and toggle. */ -export function renderDiffViewer(container: HTMLElement, options: DiffViewerOptions): void { - const body = container.createDiv({ cls: `scv-diff-tab-body scv-diff-layout-${options.layout}` }); - renderDiffPanel(body, options.remote, options.local); +export function renderDiffViewer(container: HTMLElement, options: DiffViewerOptions): DiffViewerHandle { + const layout: DiffLayout = Platform.isPhone ? 'unified' : options.layout; + const body = container.createDiv({ cls: `scv-diff-tab-body scv-diff-layout-${layout}` }); + if (options.remote !== undefined && options.local !== undefined) { + renderDiffPanel(body, options.remote, options.local); + } const toggleHost = options.toggleHost; - if (!toggleHost) return; + if (toggleHost && !Platform.isPhone) { + const renderToggle = (l: DiffLayout): void => { + toggleHost.empty(); + renderDiffLayoutToggle(toggleHost, l, next => { + applyLayoutClass(body, next); + options.onLayoutChange?.(next); + renderToggle(next); + }); + }; + renderToggle(layout); + } - const renderToggle = (layout: DiffLayout): void => { - toggleHost.empty(); - renderDiffLayoutToggle(toggleHost, layout, next => { - applyLayoutClass(body, next); - options.onLayoutChange?.(next); - renderToggle(next); - }); + return { + setContent(remote: string, local: string): void { + body.empty(); + renderDiffPanel(body, remote, local); + }, }; - renderToggle(options.layout); } function applyLayoutClass(body: HTMLElement, layout: DiffLayout): void { diff --git a/src/ui/source-control/SourceControlView.ts b/src/ui/source-control/SourceControlView.ts index fffc04b9..5a6de09f 100644 --- a/src/ui/source-control/SourceControlView.ts +++ b/src/ui/source-control/SourceControlView.ts @@ -5,8 +5,7 @@ import { SourceControlViewModel, type SourceControlItem } from '../../logic/sour import type { ChangeId } from '../../logic/source-control/types'; import { defaultSyncAction } from '../../logic/source-control/ChangeActionPolicy'; import { ICONS } from '../components/icons'; -import { renderDiffViewer, currentDiffLayout, rememberDiffLayout } from '../components/DiffViewer'; -import { renderDiffPanel } from '../components/DiffPanel'; +import { renderDiffViewer, currentDiffLayout, rememberDiffLayout, type DiffViewerHandle } from '../components/DiffViewer'; import { renderChangeTree, renderChangeList, type ChangeTreeCallbacks } from './ChangeTree'; import { renderChangeItem } from './ChangeItem'; import { DiffStatProvider, type DiffStatLoadResult } from './DiffStatProvider'; @@ -569,12 +568,11 @@ export class SourceControlView { const toggleSlot = bar.createDiv({ cls: 'scv-detail-bar-toggle' }); // Shared DiffViewer renders an empty placeholder body; the async - // load below fills it (stale-guarded) once the diff content is ready. - // The viewer appends the body directly to `detail`, so the legacy - // .scv-detail-diff wrapper's CSS is kept by styling the body itself. - renderDiffViewer(detail, { - remote: '', - local: '', + // load below fills it in (stale-guarded) via the returned handle, + // once the diff content is ready. The viewer appends the body + // directly to `detail`, so the legacy .scv-detail-diff wrapper's CSS + // is kept by styling the body itself. + const viewer = renderDiffViewer(detail, { layout: currentDiffLayout(), toggleHost: toggleSlot, onLayoutChange: (next) => { @@ -584,12 +582,12 @@ export class SourceControlView { }); const diffBody = detail.querySelector('.scv-diff-tab-body'); diffBody?.addClass('scv-detail-diff'); - if (diffBody && this.selectedChangeId) { - void this.loadAndRenderDiff(diffBody, this.selectedChangeId); + if (this.selectedChangeId) { + void this.loadAndRenderDiff(viewer, this.selectedChangeId); } } - private async loadAndRenderDiff(container: HTMLElement, changeId: ChangeId): Promise { + private async loadAndRenderDiff(viewer: DiffViewerHandle, changeId: ChangeId): Promise { if (!this.callbacks.loadDiffContent) return; const item = this.viewModel.getState('all').items.find(i => i.id === changeId) ?? this.viewModel.getState('synced', true).items.find(i => i.id === changeId); @@ -598,7 +596,7 @@ export class SourceControlView { const content = await this.callbacks.loadDiffContent(item); // Stale response guard: the selection may have moved on while awaiting. if (!content || this.selectedChangeId !== changeId) return; - renderDiffPanel(container, content.remote, content.local); + viewer.setContent(content.remote, content.local); } private toggleFolder(path: string): void { diff --git a/styles.css b/styles.css index 2d782ee6..05ca5814 100644 --- a/styles.css +++ b/styles.css @@ -749,6 +749,8 @@ body.is-mobile .scv-view-toggle-label { display: none; } display: flex; flex-direction: column; min-height: 0; + min-width: 0; + max-width: 100%; } /* Let the diff fill all the space this full tab gives it, instead of the @@ -792,6 +794,8 @@ body.is-mobile .scv-view-toggle-label { display: none; } display: flex; flex-direction: column; height: 100%; + min-width: 0; + max-width: 100%; } .scv-detail-bar { @@ -891,11 +895,13 @@ body.is-mobile .scv-view-toggle-label { display: none; } .ssv-diff-grid { display: grid; - grid-template-columns: 1fr 1fr; + grid-template-columns: minmax(0, 1fr) minmax(0, 1fr); font-family: var(--font-monospace); font-size: 0.74em; max-height: 260px; - overflow-y: auto; + overflow: auto; + min-width: 0; + max-width: 100%; } .ssv-diff-hd { diff --git a/tests/ci-workflow.test.ts b/tests/ci-workflow.test.ts index ce85aea6..4d3975fe 100644 --- a/tests/ci-workflow.test.ts +++ b/tests/ci-workflow.test.ts @@ -7,6 +7,7 @@ const harness = readFileSync('scripts/e2e-harness.sh', 'utf8'); const runner = readFileSync('scripts/run-e2e.sh', 'utf8'); const vitestE2eConfig = readFileSync('vitest.e2e.config.ts', 'utf8'); const eslintConfig = readFileSync('eslint.config.mts', 'utf8'); +const syncManagerE2eSuite = readFileSync('e2e-tests/provider/suites/sync-manager.e2e.test.ts', 'utf8'); describe('CI workflow contracts', () => { it('retries transient provider failures three times', () => { @@ -131,4 +132,29 @@ describe('E2E scanner-boundary contracts (e2e-tests/provider, no runtime generat expect(eslintConfig).toContain('"e2e-tests/**/*.ts"'); expect(eslintConfig).not.toContain('"e2e/**/*.ts"'); }); + + it('still blocks src/ imports of the removed legacy sync-status presentation layer', () => { + // Architecture regression guard for the SyncStatusView -> Source + // Control migration (see docs/source-control.md): a future refactor + // must not silently drop this no-restricted-imports rule and let + // ui/sync-status or SyncStatusView get re-wired back in. + expect(eslintConfig).toContain('"**/ui/sync-status"'); + expect(eslintConfig).toContain('"**/ui/sync-status/*"'); + expect(eslintConfig).toContain('"**/SyncStatusView"'); + expect(eslintConfig).toContain('"**/ui/SyncStatusView"'); + expect(eslintConfig).toContain('no-restricted-imports'); + }); + + it('exercises remote delete through the Source Control application path, not a direct provider bypass', () => { + // The remote-delete E2E used to call `service.deleteFile()` directly, + // reproducing what the removed SyncStatusView UI used to do. The + // current production path is SourceControlActionService.deleteRemote() + // -> SyncWorkspace.deleteRemote() -> RemoteDeleteExecutor -> + // gitService.deleteFile() -- a future edit must keep exercising that + // chain instead of quietly reverting to the raw provider call. + expect(syncManagerE2eSuite).not.toMatch(/\bservice\.deleteFile\(/); + expect(syncManagerE2eSuite).toContain('actionService.deleteRemote('); + expect(syncManagerE2eSuite).toContain("from '../../../src/logic/source-control/SourceControlActionService'"); + expect(syncManagerE2eSuite).toContain("from '../../../src/logic/sync/SyncWorkspace'"); + }); }); diff --git a/tests/setup.ts b/tests/setup.ts index 6cb2ec0e..dd850fe9 100644 --- a/tests/setup.ts +++ b/tests/setup.ts @@ -297,7 +297,7 @@ export const debounce = (cb: (...args: T) => unknown, timeo return fn; }; // Mutable so tests can exercise both the desktop and mobile branches. -export const Platform = { isDesktopApp: true, isMobile: false }; +export const Platform = { isDesktopApp: true, isMobile: false, isPhone: false, isTablet: false }; export const FileSystemAdapter = class { getBasePath() { return '/mock/path'; } }; diff --git a/tests/ui/components/DiffViewer.test.ts b/tests/ui/components/DiffViewer.test.ts index c63b50bf..cc490923 100644 --- a/tests/ui/components/DiffViewer.test.ts +++ b/tests/ui/components/DiffViewer.test.ts @@ -1,4 +1,5 @@ -import { beforeAll, describe, expect, it } from 'vitest'; +import { afterEach, beforeAll, describe, expect, it } from 'vitest'; +import { Platform } from 'obsidian'; import { renderDiffViewer } from '../../../src/ui/components/DiffViewer'; import { setupObsidianDOM, createContainer } from '../setup-dom'; @@ -52,4 +53,53 @@ describe('renderDiffViewer', () => { expect(layouts).toEqual(['split', 'unified']); expect(body?.classList.contains('scv-diff-layout-unified')).toBe(true); }); + + describe('content-less initial render (async load in flight)', () => { + it('renders no diff panel when remote/local are omitted', () => { + const container = createContainer(); + + renderDiffViewer(container, { layout: 'split' }); + + expect(container.querySelector('.ssv-diff-split')).toBeNull(); + expect(container.querySelector('.ssv-diff-unified')).toBeNull(); + }); + + it('fills in the diff panel exactly once via the handle, replacing any prior content', () => { + const container = createContainer(); + + const viewer = renderDiffViewer(container, { layout: 'split' }); + viewer.setContent('remote text', 'local text'); + viewer.setContent('remote text 2', 'local text 2'); + + expect(container.querySelectorAll('.ssv-diff-split')).toHaveLength(1); + expect(container.querySelectorAll('.ssv-diff-unified')).toHaveLength(1); + expect(container.textContent).toContain('remote text 2'); + expect(container.textContent).not.toContain('remote text\n'); + }); + }); + + describe('phone layout policy', () => { + afterEach(() => { Platform.isPhone = false; }); + + it('forces unified and ignores the requested split layout', () => { + Platform.isPhone = true; + const container = createContainer(); + + renderDiffViewer(container, { remote: 'r', local: 'l', layout: 'split' }); + + const body = container.querySelector('.scv-diff-tab-body'); + expect(body?.classList.contains('scv-diff-layout-unified')).toBe(true); + expect(body?.classList.contains('scv-diff-layout-split')).toBe(false); + }); + + it('renders no layout toggle even when a toggleHost is given', () => { + Platform.isPhone = true; + const container = createContainer(); + const toggleHost = createContainer(); + + renderDiffViewer(container, { remote: 'r', local: 'l', layout: 'split', toggleHost }); + + expect(toggleHost.querySelector('.scv-diff-layout-toggle')).toBeNull(); + }); + }); }); \ No newline at end of file diff --git a/tests/ui/source-control/SourceControlView.test.ts b/tests/ui/source-control/SourceControlView.test.ts index 221e1c0a..b650df2f 100644 --- a/tests/ui/source-control/SourceControlView.test.ts +++ b/tests/ui/source-control/SourceControlView.test.ts @@ -645,7 +645,7 @@ describe('SourceControlView', () => { // onOpenDiff, and the host (SourceControlItemView) opens a main-area // tab. Only the mobile full-screen detail view still loads/renders // diff content inside SourceControlView itself. - afterEach(() => { Platform.isMobile = false; }); + afterEach(() => { Platform.isMobile = false; Platform.isPhone = false; }); it('loads and renders diff content in the mobile detail view for the clicked change', async () => { Platform.isMobile = true; @@ -666,6 +666,83 @@ describe('SourceControlView', () => { expect(container.querySelector('.ssv-diff-split')).not.toBeNull(); }); + it('renders no diff panel before the async load resolves, and exactly one once it does', async () => { + Platform.isMobile = true; + let resolveLoad: (value: { remote: string; local: string }) => void = () => {}; + const loadDiffContent = vi.fn().mockReturnValue(new Promise(resolve => { resolveLoad = resolve; })); + const { view } = buildView( + [{ id: toChangeId('c-1'), path: 'a.md', kind: 'local-modified' }], + { loadDiffContent }, + ); + view.render(container); + + (container.querySelector('.scv-change-item') as HTMLElement).click(); + await Promise.resolve(); + + expect(container.querySelector('.ssv-diff-split')).toBeNull(); + expect(container.querySelector('.ssv-diff-unified')).toBeNull(); + + resolveLoad({ remote: 'remote text', local: 'local text' }); + await Promise.resolve(); + await Promise.resolve(); + + expect(container.querySelectorAll('.ssv-diff-split')).toHaveLength(1); + expect(container.querySelectorAll('.ssv-diff-unified')).toHaveLength(1); + expect(container.querySelectorAll('.ssv-diff-hd')).toHaveLength(2); + }); + + it('does not let a stale async result render into a reopened detail view', async () => { + Platform.isMobile = true; + let resolveFirst: (value: { remote: string; local: string }) => void = () => {}; + const loadDiffContent = vi.fn() + .mockReturnValueOnce(new Promise(resolve => { resolveFirst = resolve; })) + .mockResolvedValueOnce({ remote: 'second remote', local: 'second local' }); + const { view } = buildView( + [ + { id: toChangeId('c-1'), path: 'a.md', kind: 'local-modified' }, + { id: toChangeId('c-2'), path: 'b.md', kind: 'local-modified' }, + ], + { loadDiffContent }, + ); + view.render(container); + + (container.querySelectorAll('.scv-change-item')[0] as HTMLElement).click(); + await Promise.resolve(); + (container.querySelector('.scv-detail-back') as HTMLElement).click(); + (container.querySelectorAll('.scv-change-item')[1] as HTMLElement).click(); + await Promise.resolve(); + await Promise.resolve(); + + resolveFirst({ remote: 'first remote', local: 'first local' }); + await Promise.resolve(); + await Promise.resolve(); + + expect(container.textContent).not.toContain('first remote'); + expect(container.textContent).toContain('second remote'); + expect(container.querySelectorAll('.ssv-diff-split')).toHaveLength(1); + }); + + it('forces unified with no toggle on a phone regardless of session split preference', async () => { + Platform.isMobile = true; + Platform.isPhone = true; + const loadDiffContent = vi.fn().mockResolvedValue({ remote: 'remote text', local: 'local text' }); + const { view } = buildView( + [{ id: toChangeId('c-1'), path: 'a.md', kind: 'local-modified' }], + { loadDiffContent }, + ); + view.render(container); + + (container.querySelector('.scv-change-item') as HTMLElement).click(); + await Promise.resolve(); + await Promise.resolve(); + + const diffContainer = container.querySelector('.scv-detail-diff'); + expect(diffContainer?.classList.contains('scv-diff-layout-unified')).toBe(true); + expect(container.querySelector('.scv-diff-layout-toggle')).toBeNull(); + + Platform.isPhone = false; + }); + it('does not render an inline diff pane on desktop -- only notifies onOpenDiff', () => { const { view } = buildView([{ id: toChangeId('c-1'), path: 'a.md', kind: 'local-modified' }]); view.render(container);