From 8c69cc8da702df74b6635c2991c999aaa990a5f7 Mon Sep 17 00:00:00 2001 From: Tianyao Date: Sat, 22 Aug 2026 23:44:39 +0800 Subject: [PATCH 01/17] refactor(sync-status): integrate source control view model MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add selectedItems and refreshStatus projections to SourceControlViewState so later UI commits can render a 'SELECTED FOR SYNC (N)' section and refresh button states. selectedItems reuses buildSummary.readyToPush (selected + non-synced) so the section and Sync button count never drift. Add RefreshState holder (idle/loading/failed) mirroring OperationState's API shape but for a single view-wide refresh rather than per-change operations. Add ViewModel.refresh(), which delegates to an injected refresh callback wired to SyncWorkspace.refresh() in main.ts and drives the RefreshState lifecycle. Refresh republishes sync.status, so the existing subscription repopulates ChangeRepository — refresh never becomes a second population path, and no ChangeRepository.reload() is added. Wire RefreshState + the refresh delegate into the ViewModel in main.ts. Update the ViewModel docstring to acknowledge the refresh-delegation responsibility (it delegates; no provider/refresh logic lives in it). Domain untouched: only SourceControlViewModel.ts edited and RefreshState.ts new under src/logic/source-control/ (filter/summary/types/repository/store/ adapter unchanged). --- src/logic/source-control/RefreshState.ts | 37 ++++++++++ .../source-control/SourceControlViewModel.ts | 41 ++++++++++- src/main.ts | 5 ++ .../logic/source-control/RefreshState.test.ts | 44 ++++++++++++ .../SourceControlViewModel.test.ts | 69 ++++++++++++++++++- .../SourceControlItemView.test.ts | 4 +- .../source-control/SourceControlView.test.ts | 7 +- 7 files changed, 200 insertions(+), 7 deletions(-) create mode 100644 src/logic/source-control/RefreshState.ts create mode 100644 tests/logic/source-control/RefreshState.test.ts diff --git a/src/logic/source-control/RefreshState.ts b/src/logic/source-control/RefreshState.ts new file mode 100644 index 0000000..7247ebc --- /dev/null +++ b/src/logic/source-control/RefreshState.ts @@ -0,0 +1,37 @@ +/** + * Single-value refresh status for the whole Source Control view, mirroring + * {@link OperationState}'s API shape (start/fail/succeed/get/clear) but for + * one global refresh rather than per-{@link ChangeId} operations. + * + * Holds no refresh logic of its own: {@link SourceControlViewModel.refresh} + * delegates to the injected `syncWorkspace.refresh()` and only drives this + * holder so the UI can show "Refreshing…" / a failed state. Keeping it a + * separate holder (rather than reusing `OperationState`) avoids conflating a + * view-wide background refresh with per-change push/pull operations. + */ +export type RefreshStatus = 'idle' | 'loading' | 'failed'; + +export class RefreshState { + private status: RefreshStatus = 'idle'; + + start(): void { + this.status = 'loading'; + } + + fail(): void { + this.status = 'failed'; + } + + succeed(): void { + this.status = 'idle'; + } + + /** Resets back to idle, clearing a prior failure so the button no longer shows the error state. */ + clear(): void { + this.status = 'idle'; + } + + get(): RefreshStatus { + return this.status; + } +} \ No newline at end of file diff --git a/src/logic/source-control/SourceControlViewModel.ts b/src/logic/source-control/SourceControlViewModel.ts index c880a70..8fe7141 100644 --- a/src/logic/source-control/SourceControlViewModel.ts +++ b/src/logic/source-control/SourceControlViewModel.ts @@ -1,6 +1,7 @@ import type { ChangeRepository } from './ChangeRepository'; import { buildSummary, type SourceControlCounts } from './SourceControlSummary'; import type { OperationState, OperationStatus } from './OperationState'; +import type { RefreshState, RefreshStatus } from './RefreshState'; import type { PushSelectionStore } from './PushSelectionStore'; import { matchesFilter, type SourceControlFilter } from './SourceControlFilter'; import type { ChangeId, SyncChange, SyncChangeKind } from './types'; @@ -19,6 +20,15 @@ export interface SourceControlItem { export interface SourceControlViewState { filter: SourceControlFilter; items: SourceControlItem[]; + /** + * The actionable changes the user has currently selected for push, as + * full row items. Empty when nothing is selected. Reuses the same + * `selected + non-synced` definition as `buildSummary.readyToPush` so the + * "SELECTED FOR SYNC (N)" section and the Sync button count can't drift. + */ + selectedItems: SourceControlItem[]; + /** Current view-wide refresh status, surfaced so the header can render its states. */ + refreshStatus: RefreshStatus; /** Single-source counts from {@link buildSummary} — the view never recomputes these. */ counts: SourceControlCounts; } @@ -37,12 +47,21 @@ export interface SourceControlViewState { * `showSynced` governs whether the synced bucket is surfaced: when false the * synced count is reported as `0` and the `synced` filter yields no items, * matching the "Show synced" toggle (default off). + * + * The one non-projection responsibility is {@link refresh}: it delegates to an + * injected refresh callback (wired to `SyncWorkspace.refresh()` in `main.ts`) + * and drives the injected {@link RefreshState} holder so the UI can surface + * loading/failed states. It holds no provider or refresh logic of its own, + * keeping the event-driven pipeline (`sync.status` → `ChangeRepository` → + * ViewModel → UI) intact — refresh never becomes a second population path. */ export class SourceControlViewModel { constructor( private readonly changes: ChangeRepository, private readonly selection: PushSelectionStore, private readonly operations: OperationState, + private readonly refreshSource: () => Promise, + private readonly refreshState: RefreshState, ) {} getState(filter: SourceControlFilter = 'all', showSynced = false): SourceControlViewState { @@ -52,7 +71,27 @@ export class SourceControlViewModel { .filter(change => matchesFilter(change, filter, this.selection)) .filter(() => this.isRenderable(filter, showSynced)) .map(change => this.toItem(change)); - return { filter, items, counts: summary.counts }; + const selectedItems = summary.readyToPush.map(change => this.toItem(change)); + return { filter, items, selectedItems, refreshStatus: this.refreshState.get(), counts: summary.counts }; + } + + /** + * Triggers a view-wide refresh by delegating to the injected refresh + * source (the Sync Status service boundary) and tracking its lifecycle on + * the {@link RefreshState} holder so the header can render "Refreshing…" + * / a failed state. Refresh republishes `sync.status`, so the existing + * subscription repopulates `ChangeRepository` — this never becomes a + * second population path. + */ + async refresh(): Promise { + this.refreshState.start(); + try { + await this.refreshSource(); + this.refreshState.succeed(); + } catch (error) { + this.refreshState.fail(); + throw error; + } } private isRenderable(filter: SourceControlFilter, showSynced: boolean): boolean { diff --git a/src/main.ts b/src/main.ts index 31d2ef4..e9afa97 100644 --- a/src/main.ts +++ b/src/main.ts @@ -21,6 +21,7 @@ import { SyncDiffService } from './logic/sync/SyncDiffService'; import { SyncManagerWorkspace, type SyncWorkspace } from './logic/sync/SyncWorkspace'; import { ChangeRepository } from './logic/source-control/ChangeRepository'; import { OperationState } from './logic/source-control/OperationState'; +import { RefreshState } from './logic/source-control/RefreshState'; import { PushSelectionStore } from './logic/source-control/PushSelectionStore'; import { SourceControlViewModel } from './logic/source-control/SourceControlViewModel'; import { SourceControlActionService } from './logic/source-control/SourceControlActionService'; @@ -43,6 +44,7 @@ export default class GitLabFilesPush extends Plugin { changeRepository: ChangeRepository; pushSelectionStore: PushSelectionStore; operationState: OperationState; + refreshState: RefreshState; sourceControlViewModel: SourceControlViewModel; sourceControlActions: SourceControlActionService; private unsubscribeChangeRepository?: () => void; @@ -114,10 +116,13 @@ export default class GitLabFilesPush extends Plugin { this.changeRepository = new ChangeRepository(); this.pushSelectionStore = new PushSelectionStore(); this.operationState = new OperationState(); + this.refreshState = new RefreshState(); this.sourceControlViewModel = new SourceControlViewModel( this.changeRepository, this.pushSelectionStore, this.operationState, + () => this.syncWorkspace.refresh(), + this.refreshState, ); this.sourceControlActions = new SourceControlActionService( this.changeRepository, diff --git a/tests/logic/source-control/RefreshState.test.ts b/tests/logic/source-control/RefreshState.test.ts new file mode 100644 index 0000000..45ecf03 --- /dev/null +++ b/tests/logic/source-control/RefreshState.test.ts @@ -0,0 +1,44 @@ +import { describe, expect, it } from 'vitest'; +import { RefreshState } from '../../../src/logic/source-control/RefreshState'; + +describe('RefreshState', () => { + it('starts idle', () => { + expect(new RefreshState().get()).toBe('idle'); + }); + + it('transitions to loading on start', () => { + const state = new RefreshState(); + state.start(); + expect(state.get()).toBe('loading'); + }); + + it('transitions back to idle on succeed', () => { + const state = new RefreshState(); + state.start(); + state.succeed(); + expect(state.get()).toBe('idle'); + }); + + it('transitions to failed on fail', () => { + const state = new RefreshState(); + state.start(); + state.fail(); + expect(state.get()).toBe('failed'); + }); + + it('clears a failure back to idle', () => { + const state = new RefreshState(); + state.start(); + state.fail(); + state.clear(); + expect(state.get()).toBe('idle'); + }); + + it('succeed clears a failed state back to idle', () => { + const state = new RefreshState(); + state.start(); + state.fail(); + state.succeed(); + expect(state.get()).toBe('idle'); + }); +}); \ No newline at end of file diff --git a/tests/logic/source-control/SourceControlViewModel.test.ts b/tests/logic/source-control/SourceControlViewModel.test.ts index cd9b1d1..01c7771 100644 --- a/tests/logic/source-control/SourceControlViewModel.test.ts +++ b/tests/logic/source-control/SourceControlViewModel.test.ts @@ -1,6 +1,7 @@ -import { describe, expect, it } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; import { ChangeRepository } from '../../../src/logic/source-control/ChangeRepository'; import { OperationState } from '../../../src/logic/source-control/OperationState'; +import { RefreshState } from '../../../src/logic/source-control/RefreshState'; import { PushSelectionStore } from '../../../src/logic/source-control/PushSelectionStore'; import { SourceControlViewModel } from '../../../src/logic/source-control/SourceControlViewModel'; import { toChangeId, type SyncChange } from '../../../src/logic/source-control/types'; @@ -10,8 +11,10 @@ function buildViewModel(changes: SyncChange[]) { repository.replace(changes); const selection = new PushSelectionStore(); const operations = new OperationState(); - const viewModel = new SourceControlViewModel(repository, selection, operations); - return { viewModel, selection, operations }; + const refreshState = new RefreshState(); + const refreshSource = vi.fn().mockResolvedValue(undefined); + const viewModel = new SourceControlViewModel(repository, selection, operations, refreshSource, refreshState); + return { viewModel, selection, operations, refreshState, refreshSource }; } describe('SourceControlViewModel', () => { @@ -112,4 +115,64 @@ describe('SourceControlViewModel', () => { expect(item?.path).toBe('new.md'); expect(item?.previousPath).toBe('old.md'); }); + + it('projects selectedItems as the actionable changes currently in PushSelectionStore', () => { + const changes: SyncChange[] = [ + { id: toChangeId('c-1'), path: 'a.md', kind: 'local-only' }, + { id: toChangeId('c-2'), path: 'b.md', kind: 'remote-only' }, + { id: toChangeId('c-3'), path: 'c.md', kind: 'synced' }, + ]; + const { viewModel, selection } = buildViewModel(changes); + selection.includeForPush(toChangeId('c-1')); + selection.includeForPush(toChangeId('c-2')); + selection.includeForPush(toChangeId('c-3')); + + const state = viewModel.getState('all'); + expect(state.selectedItems.map(i => i.id)).toEqual([toChangeId('c-1'), toChangeId('c-2')]); + // Synced is never actionable, so it's excluded even when selected. + expect(state.selectedItems.every(i => i.kind !== 'synced')).toBe(true); + expect(state.selectedItems[0]?.isReadyToPush).toBe(true); + }); + + it('reports an empty selectedItems projection when nothing is selected', () => { + const { viewModel } = buildViewModel([{ id: toChangeId('c-1'), path: 'a.md', kind: 'local-only' }]); + expect(viewModel.getState('all').selectedItems).toEqual([]); + }); + + it('surfaces the current refresh status on every view state', () => { + const { viewModel, refreshState } = buildViewModel([{ id: toChangeId('c-1'), path: 'a.md', kind: 'local-only' }]); + expect(viewModel.getState('all').refreshStatus).toBe('idle'); + refreshState.start(); + expect(viewModel.getState('all').refreshStatus).toBe('loading'); + refreshState.fail(); + expect(viewModel.getState('all').refreshStatus).toBe('failed'); + }); + + it('refresh() delegates to the refresh source and drives the RefreshState lifecycle', async () => { + const { viewModel, refreshState, refreshSource } = buildViewModel([ + { id: toChangeId('c-1'), path: 'a.md', kind: 'local-only' }, + ]); + + await viewModel.refresh(); + + expect(refreshSource).toHaveBeenCalledTimes(1); + expect(refreshState.get()).toBe('idle'); + }); + + it('refresh() marks the RefreshState failed and rethrows when the refresh source rejects', async () => { + const refreshSource = vi.fn().mockRejectedValue(new Error('boom')); + const repository = new ChangeRepository(); + repository.replace([{ id: toChangeId('c-1'), path: 'a.md', kind: 'local-only' }]); + const refreshState = new RefreshState(); + const viewModel = new SourceControlViewModel( + repository, + new PushSelectionStore(), + new OperationState(), + refreshSource, + refreshState, + ); + + await expect(viewModel.refresh()).rejects.toThrow('boom'); + expect(refreshState.get()).toBe('failed'); + }); }); diff --git a/tests/ui/source-control/SourceControlItemView.test.ts b/tests/ui/source-control/SourceControlItemView.test.ts index 32843a8..572fc7c 100644 --- a/tests/ui/source-control/SourceControlItemView.test.ts +++ b/tests/ui/source-control/SourceControlItemView.test.ts @@ -3,6 +3,7 @@ import { TFile, WorkspaceLeaf } from 'obsidian'; import { SourceControlItemView, SOURCE_CONTROL_VIEW_TYPE } from '../../../src/ui/source-control/SourceControlItemView'; import { ChangeRepository } from '../../../src/logic/source-control/ChangeRepository'; import { OperationState } from '../../../src/logic/source-control/OperationState'; +import { RefreshState } from '../../../src/logic/source-control/RefreshState'; import { PushSelectionStore } from '../../../src/logic/source-control/PushSelectionStore'; import { SourceControlViewModel } from '../../../src/logic/source-control/SourceControlViewModel'; import { toChangeId, type SyncChangeKind } from '../../../src/logic/source-control/types'; @@ -17,7 +18,8 @@ function buildPlugin(kind: SyncChangeKind = 'local-only') { repository.replace([{ id: toChangeId('a.md'), path: 'a.md', kind }]); const selection = new PushSelectionStore(); const operations = new OperationState(); - const viewModel = new SourceControlViewModel(repository, selection, operations); + const refreshState = new RefreshState(); + const viewModel = new SourceControlViewModel(repository, selection, operations, vi.fn().mockResolvedValue(undefined), refreshState); const push = vi.fn().mockResolvedValue(undefined); const loadDiffContent = vi.fn().mockResolvedValue({ remote: 'remote text', local: 'local text' }); const openDiffTab = vi.fn().mockResolvedValue(undefined); diff --git a/tests/ui/source-control/SourceControlView.test.ts b/tests/ui/source-control/SourceControlView.test.ts index da25583..17e02d7 100644 --- a/tests/ui/source-control/SourceControlView.test.ts +++ b/tests/ui/source-control/SourceControlView.test.ts @@ -3,6 +3,7 @@ import { Platform } from 'obsidian'; import { SourceControlView, type SourceControlViewCallbacks } from '../../../src/ui/source-control/SourceControlView'; import { ChangeRepository } from '../../../src/logic/source-control/ChangeRepository'; import { OperationState } from '../../../src/logic/source-control/OperationState'; +import { RefreshState } from '../../../src/logic/source-control/RefreshState'; import { PushSelectionStore } from '../../../src/logic/source-control/PushSelectionStore'; import { SourceControlViewModel } from '../../../src/logic/source-control/SourceControlViewModel'; import { toChangeId, type SyncChange } from '../../../src/logic/source-control/types'; @@ -15,7 +16,9 @@ function buildView(changes: SyncChange[], callbacks: Partial ({ serviceName: 'GitHub', @@ -23,7 +26,7 @@ function buildView(changes: SyncChange[], callbacks: Partial { From 625fad25c59911b07949db4f3af15aee3a9f3a5f Mon Sep 17 00:00:00 2001 From: Tianyao Date: Sat, 22 Aug 2026 23:47:00 +0800 Subject: [PATCH 02/17] feat(sync-status): add selection workflow and sync action UI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add the 'SELECTED FOR SYNC (N)' summary section above the change tree, rendered only when the user has at least one actionable change selected for push. Its count comes straight from the ViewModel's single-source selectedItems projection (the same selected + non-synced definition as the Sync button count), so the section and the Sync button can never drift. Synced changes are excluded even when selected. Drop the 'Ready to Push' chip from the filter row. The visible row is now four chips — All / Local / Remote / Conflict — backed by the unchanged domain filters (all / changes / remote-changes / conflicts). data-filter attributes keep the domain values; only the displayed labels change via new i18n keys (sourceControl.filter.local / .remote / .conflict). The ready-to-push and synced domain filters remain in the type (the former is just no longer exposed as a chip; the latter still surfaces via the 'Show synced' toggle). Add sourceControl.section.selectedForSync i18n key (en/zh-cn/zh-tw) and CSS for .scv-selected-section. FilterMenu/SourceControlView tests updated. --- src/i18n/locales/en.ts | 4 +++ src/i18n/locales/zh-cn.ts | 4 +++ src/i18n/locales/zh-tw.ts | 4 +++ src/ui/source-control/FilterMenu.ts | 33 ++++++++++++------- src/ui/source-control/SourceControlView.ts | 17 ++++++++++ styles.css | 26 +++++++++++++++ tests/ui/source-control/FilterMenu.test.ts | 14 ++++++-- .../source-control/SourceControlView.test.ts | 29 ++++++++++++++++ 8 files changed, 117 insertions(+), 14 deletions(-) diff --git a/src/i18n/locales/en.ts b/src/i18n/locales/en.ts index 796e03f..1285651 100644 --- a/src/i18n/locales/en.ts +++ b/src/i18n/locales/en.ts @@ -250,6 +250,9 @@ const en = { 'sourceControl.filter.all': 'All', 'sourceControl.filter.changes': 'Changes', + 'sourceControl.filter.local': 'Local', + 'sourceControl.filter.remote': 'Remote', + 'sourceControl.filter.conflict': 'Conflict', 'sourceControl.filter.readyToPush': 'Ready to Push', 'sourceControl.filter.remoteChanges': 'Remote Changes', 'sourceControl.filter.conflicts': 'Conflicts', @@ -257,6 +260,7 @@ const en = { 'sourceControl.filter.showSynced': 'Show synced', 'sourceControl.section.all': 'ALL', 'sourceControl.section.readyToPush': 'READY TO PUSH', + 'sourceControl.section.selectedForSync': 'SELECTED FOR SYNC', 'sourceControl.section.changes': 'CHANGES', 'sourceControl.section.remoteChanges': 'REMOTE CHANGES', 'sourceControl.section.conflicts': 'CONFLICTS', diff --git a/src/i18n/locales/zh-cn.ts b/src/i18n/locales/zh-cn.ts index 13c06aa..9315e90 100644 --- a/src/i18n/locales/zh-cn.ts +++ b/src/i18n/locales/zh-cn.ts @@ -252,6 +252,9 @@ const zhCn: Partial> = { 'sourceControl.filter.all': '全部', 'sourceControl.filter.changes': '更改', + 'sourceControl.filter.local': '本地', + 'sourceControl.filter.remote': '远程', + 'sourceControl.filter.conflict': '冲突', 'sourceControl.filter.readyToPush': '待推送', 'sourceControl.filter.remoteChanges': '远程更改', 'sourceControl.filter.conflicts': '冲突', @@ -259,6 +262,7 @@ const zhCn: Partial> = { 'sourceControl.filter.showSynced': '显示已同步', 'sourceControl.section.all': '全部', 'sourceControl.section.readyToPush': '待推送', + 'sourceControl.section.selectedForSync': '已选同步', 'sourceControl.section.changes': '更改', 'sourceControl.section.remoteChanges': '远程更改', 'sourceControl.section.conflicts': '冲突', diff --git a/src/i18n/locales/zh-tw.ts b/src/i18n/locales/zh-tw.ts index 3f421d7..4f3d9aa 100644 --- a/src/i18n/locales/zh-tw.ts +++ b/src/i18n/locales/zh-tw.ts @@ -252,6 +252,9 @@ const zhTw: Partial> = { 'sourceControl.filter.all': '全部', 'sourceControl.filter.changes': '變更', + 'sourceControl.filter.local': '本地', + 'sourceControl.filter.remote': '遠端', + 'sourceControl.filter.conflict': '衝突', 'sourceControl.filter.readyToPush': '待推送', 'sourceControl.filter.remoteChanges': '遠端變更', 'sourceControl.filter.conflicts': '衝突', @@ -259,6 +262,7 @@ const zhTw: Partial> = { 'sourceControl.filter.showSynced': '顯示已同步', 'sourceControl.section.all': '全部', 'sourceControl.section.readyToPush': '待推送', + 'sourceControl.section.selectedForSync': '已選同步', 'sourceControl.section.changes': '變更', 'sourceControl.section.remoteChanges': '遠端變更', 'sourceControl.section.conflicts': '衝突', diff --git a/src/ui/source-control/FilterMenu.ts b/src/ui/source-control/FilterMenu.ts index f7bd306..292ea50 100644 --- a/src/ui/source-control/FilterMenu.ts +++ b/src/ui/source-control/FilterMenu.ts @@ -2,18 +2,29 @@ import { t, type TranslationKey } from '../../i18n'; import type { SourceControlFilter } from '../../logic/source-control/SourceControlFilter'; /** - * Action filter chips, in spec order. `synced` is deliberately NOT a permanent - * chip — it surfaces only when the user opts in via the "Show synced" toggle, - * so a quiet workspace isn't dominated by a large synced count. + * Action filter chips, in spec order. The visible row is four chips — + * All / Local / Remote / Conflict — backed by the unchanged domain filters + * (`all` / `changes` / `remote-changes` / `conflicts`). "Ready to Push" is no + * longer a chip: it's surfaced as the inline "SELECTED FOR SYNC (N)" section + * instead. `synced` is deliberately NOT a permanent chip — it surfaces only + * when the user opts in via the "Show synced" toggle, so a quiet workspace + * isn't dominated by a large synced count. */ -const ACTION_FILTERS: SourceControlFilter[] = ['all', 'changes', 'ready-to-push', 'remote-changes', 'conflicts']; +const ACTION_FILTERS: SourceControlFilter[] = ['all', 'changes', 'remote-changes', 'conflicts']; +/** + * Displayed chip labels. Domain values stay as `data-filter` attributes; only + * the visible label changes (e.g. the `changes` domain filter reads "Local" + * because it surfaces local-side changes). `ready-to-push` and `synced` keep + * their existing keys even though `ready-to-push` is no longer a chip, so the + * record stays total over {@link SourceControlFilter}. + */ const FILTER_LABEL_KEYS: Record = { all: 'sourceControl.filter.all', - changes: 'sourceControl.filter.changes', + changes: 'sourceControl.filter.local', 'ready-to-push': 'sourceControl.filter.readyToPush', - 'remote-changes': 'sourceControl.filter.remoteChanges', - conflicts: 'sourceControl.filter.conflicts', + 'remote-changes': 'sourceControl.filter.remote', + conflicts: 'sourceControl.filter.conflict', synced: 'sourceControl.filter.synced', }; @@ -25,10 +36,10 @@ export interface FilterMenuCallbacks { } /** - * Renders the Source Control filter row: the five action chips (All, Changes, - * Ready to Push, Remote Changes, Conflicts) followed by a "Show synced" - * toggle. The `synced` chip is appended only when `showSynced` is on, so a - * hidden synced bucket contributes no chip and no count to the row. + * Renders the Source Control filter row: the four action chips (All, Local, + * Remote, Conflict) followed by a "Show synced" toggle. The `synced` chip is + * appended only when `showSynced` is on, so a hidden synced bucket contributes + * no chip and no count to the row. * * Per-filter counts come straight from the ViewModel's single-source counts; * the menu never recomputes one. diff --git a/src/ui/source-control/SourceControlView.ts b/src/ui/source-control/SourceControlView.ts index f54ee12..41c7fdc 100644 --- a/src/ui/source-control/SourceControlView.ts +++ b/src/ui/source-control/SourceControlView.ts @@ -145,6 +145,8 @@ export class SourceControlView { const query = this.searchQuery.trim().toLowerCase(); const items = query ? state.items.filter(item => item.path.toLowerCase().includes(query)) : state.items; + this.renderSelectedSection(container, state.selectedItems); + const body = container.createDiv({ cls: 'scv-body' }); this.renderActiveFilterHeader(body, state.filter, items.length); if (items.length === 0) { @@ -220,6 +222,21 @@ export class SourceControlView { header.createSpan({ cls: 'scv-active-filter-count', text: String(count) }); } + /** + * Renders the "SELECTED FOR SYNC (N)" summary, only when the user has at + * least one actionable change selected for push. Sits above the tree so the + * current push batch is always visible regardless of the active filter. + * The count comes straight from the ViewModel's single-source + * `selectedItems` projection (same definition as the Sync button count), + * so the two can never drift. + */ + private renderSelectedSection(container: HTMLElement, selectedItems: readonly SourceControlItem[]): void { + if (selectedItems.length === 0) return; + const section = container.createDiv({ cls: 'scv-selected-section' }); + section.createSpan({ cls: 'scv-selected-section-title', text: t('sourceControl.section.selectedForSync') }); + section.createSpan({ cls: 'scv-selected-section-count', text: String(selectedItems.length) }); + } + private renderDetail(root: HTMLElement): void { const detail = root.createDiv({ cls: 'scv-detail' }); const bar = detail.createDiv({ cls: 'scv-detail-bar' }); diff --git a/styles.css b/styles.css index 0e235b5..bb614b6 100644 --- a/styles.css +++ b/styles.css @@ -216,6 +216,32 @@ text-align: center; } +/* ── Selected-for-sync summary ──────────────────────────────── */ +.scv-selected-section { + display: flex; + align-items: center; + gap: 6px; + padding: 6px 12px; + margin: 4px 12px 0 12px; + border-radius: 6px; + background: var(--background-modifier-hover); + color: var(--text-muted); + font-size: 0.78em; + font-weight: 600; + text-transform: uppercase; + letter-spacing: 0.02em; +} + +.scv-selected-section-count { + background: var(--interactive-accent); + color: var(--text-on-accent); + border-radius: 10px; + padding: 1px 6px; + font-size: 0.9em; + min-width: 18px; + text-align: center; +} + /* ── Push button ───────────────────────────────────────────────── */ .scv-push-btn { display: flex; diff --git a/tests/ui/source-control/FilterMenu.test.ts b/tests/ui/source-control/FilterMenu.test.ts index dc6858f..5b90a91 100644 --- a/tests/ui/source-control/FilterMenu.test.ts +++ b/tests/ui/source-control/FilterMenu.test.ts @@ -18,18 +18,26 @@ describe('renderFilterMenu', () => { callbacks = { onFilterChange: vi.fn(), onToggleShowSynced: vi.fn() }; }); - it('renders the five action chips (no synced chip) when showSynced is false', () => { + it('renders the four action chips (All/Local/Remote/Conflict, no synced chip) when showSynced is false', () => { renderFilterMenu(container, 'all', zeroCounts, false, callbacks); const filters = Array.from(container.querySelectorAll('.scv-filter-option')).map(el => el.getAttribute('data-filter')); - expect(filters).toEqual(['all', 'changes', 'ready-to-push', 'remote-changes', 'conflicts']); + expect(filters).toEqual(['all', 'changes', 'remote-changes', 'conflicts']); + }); + + it('labels the chips with the domain-relabeled display names (Local/Remote/Conflict)', () => { + renderFilterMenu(container, 'all', zeroCounts, false, callbacks); + + const labels = Array.from(container.querySelectorAll('.scv-filter-option .scv-filter-label')).map(el => el.textContent); + // Domain values stay as data-filter; only the visible labels change. + expect(labels).toEqual(['All', 'Local', 'Remote', 'Conflict']); }); it('appends the synced chip when showSynced is true', () => { renderFilterMenu(container, 'all', { ...zeroCounts, synced: 7 }, true, callbacks); const filters = Array.from(container.querySelectorAll('.scv-filter-option')).map(el => el.getAttribute('data-filter')); - expect(filters).toEqual(['all', 'changes', 'ready-to-push', 'remote-changes', 'conflicts', 'synced']); + expect(filters).toEqual(['all', 'changes', 'remote-changes', 'conflicts', 'synced']); const syncedOption = container.querySelector('.scv-filter-option[data-filter="synced"]'); expect(syncedOption?.querySelector('.scv-filter-count')?.textContent).toBe('7'); }); diff --git a/tests/ui/source-control/SourceControlView.test.ts b/tests/ui/source-control/SourceControlView.test.ts index 17e02d7..67a00f8 100644 --- a/tests/ui/source-control/SourceControlView.test.ts +++ b/tests/ui/source-control/SourceControlView.test.ts @@ -168,6 +168,35 @@ describe('SourceControlView', () => { expect(selection.isIncluded(toChangeId('c-1'))).toBe(false); }); + + it('renders the "SELECTED FOR SYNC" section only when at least one actionable change is selected', () => { + const { view, selection } = buildView([ + { id: toChangeId('c-1'), path: 'a.md', kind: 'local-only' }, + { id: toChangeId('c-2'), path: 'b.md', kind: 'synced' }, + ]); + + view.render(container); + expect(container.querySelector('.scv-selected-section')).toBeNull(); + + selection.includeForPush(toChangeId('c-1')); + view.render(container); + const section = container.querySelector('.scv-selected-section'); + expect(section).not.toBeNull(); + expect(section?.querySelector('.scv-selected-section-title')?.textContent).toBe('SELECTED FOR SYNC'); + expect(section?.querySelector('.scv-selected-section-count')?.textContent).toBe('1'); + }); + + it('excludes synced changes from the SELECTED FOR SYNC count even when selected', () => { + const { view, selection } = buildView([ + { id: toChangeId('c-1'), path: 'a.md', kind: 'local-only' }, + { id: toChangeId('c-2'), path: 'b.md', kind: 'synced' }, + ]); + selection.includeForPush(toChangeId('c-1')); + selection.includeForPush(toChangeId('c-2')); + view.render(container); + + expect(container.querySelector('.scv-selected-section-count')?.textContent).toBe('1'); + }); }); describe('push action', () => { From 759b717da7e6d49dbc6f99e2dc18b5ab71480d59 Mon Sep 17 00:00:00 2001 From: Tianyao Date: Sat, 22 Aug 2026 23:51:33 +0800 Subject: [PATCH 03/17] feat(sync-status): add refresh and operation feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a refresh button to the Source Control header with three states driven by the RefreshState holder: idle (icon-only), loading ('Refreshing...' with a spinning icon, disabled), and failed ('Refresh failed'). The button reads refreshStatus off the ViewModel state and calls a new onRefresh callback. Wire onRefresh in SourceControlViewCallbacks and SourceControlItemView to the ViewModel's refresh() delegate via a runRefresh helper that renders immediately (so the loading state shows once refresh() sets RefreshState to 'loading' synchronously) then re-renders on settle (idle on success, failed on rejection — the rejection is swallowed since the state was already recorded on the holder). Add text labels alongside the per-change OperationIndicator icon ('Syncing' / 'Synced' / 'Failed') via new sourceControl.op.* i18n keys, so an in-flight operation is readable rather than icon-only. Add sourceControl.refresh.* i18n keys (en/zh-cn/zh-tw) and CSS for .scv-refresh-btn states and .scv-op-label. The push button's full-width layout becomes flex:1 so the refresh button sits beside it. Refresh and operation tests added. --- src/i18n/locales/en.ts | 6 ++ src/i18n/locales/zh-cn.ts | 6 ++ src/i18n/locales/zh-tw.ts | 6 ++ src/ui/source-control/OperationIndicator.ts | 15 +++- src/ui/source-control/SourceControlHeader.ts | 41 ++++++++-- .../source-control/SourceControlItemView.ts | 16 ++++ src/ui/source-control/SourceControlView.ts | 13 +++- styles.css | 49 +++++++++++- .../SourceControlItemView.test.ts | 13 ++++ .../source-control/SourceControlView.test.ts | 75 ++++++++++++++++++- 10 files changed, 225 insertions(+), 15 deletions(-) diff --git a/src/i18n/locales/en.ts b/src/i18n/locales/en.ts index 1285651..7b8f1ad 100644 --- a/src/i18n/locales/en.ts +++ b/src/i18n/locales/en.ts @@ -267,6 +267,12 @@ const en = { 'sourceControl.section.synced': 'SYNCED', 'sourceControl.push': ' Sync ({count})', 'sourceControl.push.tooltip': 'Push {count} ready file(s)', + 'sourceControl.refresh.tooltip': 'Refresh', + 'sourceControl.refresh.refreshing': 'Refreshing…', + 'sourceControl.refresh.failed': 'Refresh failed', + 'sourceControl.op.syncing': 'Syncing', + 'sourceControl.op.synced': 'Synced', + 'sourceControl.op.failed': 'Failed', 'sourceControl.empty': 'No changes', 'sourceControl.detail.back': 'Back', 'sourceControl.info.lastSync': 'Last sync: {time}', diff --git a/src/i18n/locales/zh-cn.ts b/src/i18n/locales/zh-cn.ts index 9315e90..52a1089 100644 --- a/src/i18n/locales/zh-cn.ts +++ b/src/i18n/locales/zh-cn.ts @@ -269,6 +269,12 @@ const zhCn: Partial> = { 'sourceControl.section.synced': '已同步', 'sourceControl.push': ' 同步 ({count})', 'sourceControl.push.tooltip': '推送 {count} 个已就绪的文件', + 'sourceControl.refresh.tooltip': '刷新', + 'sourceControl.refresh.refreshing': '刷新中…', + 'sourceControl.refresh.failed': '刷新失败', + 'sourceControl.op.syncing': '同步中', + 'sourceControl.op.synced': '已同步', + 'sourceControl.op.failed': '失败', 'sourceControl.empty': '没有更改', 'sourceControl.detail.back': '返回', 'sourceControl.info.lastSync': '上次同步：{time}', diff --git a/src/i18n/locales/zh-tw.ts b/src/i18n/locales/zh-tw.ts index 4f3d9aa..7c8950c 100644 --- a/src/i18n/locales/zh-tw.ts +++ b/src/i18n/locales/zh-tw.ts @@ -269,6 +269,12 @@ const zhTw: Partial> = { 'sourceControl.section.synced': '已同步', 'sourceControl.push': ' 同步 ({count})', 'sourceControl.push.tooltip': '推送 {count} 個已就緒的檔案', + 'sourceControl.refresh.tooltip': '重新整理', + 'sourceControl.refresh.refreshing': '重新整理中…', + 'sourceControl.refresh.failed': '重新整理失敗', + 'sourceControl.op.syncing': '同步中', + 'sourceControl.op.synced': '已同步', + 'sourceControl.op.failed': '失敗', 'sourceControl.empty': '沒有變更', 'sourceControl.detail.back': '返回', 'sourceControl.info.lastSync': '上次同步：{time}', diff --git a/src/ui/source-control/OperationIndicator.ts b/src/ui/source-control/OperationIndicator.ts index 8785db8..ebd40e0 100644 --- a/src/ui/source-control/OperationIndicator.ts +++ b/src/ui/source-control/OperationIndicator.ts @@ -1,17 +1,26 @@ import { setIcon } from 'obsidian'; +import { t, type TranslationKey } from '../../i18n'; import { ICONS } from '../components/icons'; import type { OperationStatus } from '../../logic/source-control/OperationState'; +const OP_LABEL_KEYS: Record, TranslationKey> = { + running: 'sourceControl.op.syncing', + success: 'sourceControl.op.synced', + failed: 'sourceControl.op.failed', +}; + /** - * Renders a small per-change status indicator for an in-flight operation. - * Renders nothing for 'idle' — the common case — so rows stay quiet until an + * Renders a small per-change status indicator for an in-flight operation: + * icon plus a short text label ("Syncing" / "Synced" / "Failed"). Renders + * nothing for 'idle' — the common case — so rows stay quiet until an * operation is actually running/finished. */ export function renderOperationIndicator(container: HTMLElement, status: OperationStatus): HTMLElement | undefined { if (status === 'idle') return undefined; const el = container.createSpan({ cls: `scv-op-indicator scv-op-${status}` }); - setIcon(el, operationIcon(status)); + setIcon(el.createSpan({ cls: 'scv-op-icon' }), operationIcon(status)); + el.createSpan({ cls: 'scv-op-label', text: t(OP_LABEL_KEYS[status]) }); return el; } diff --git a/src/ui/source-control/SourceControlHeader.ts b/src/ui/source-control/SourceControlHeader.ts index 87f6e7e..12cac4d 100644 --- a/src/ui/source-control/SourceControlHeader.ts +++ b/src/ui/source-control/SourceControlHeader.ts @@ -1,5 +1,6 @@ -import { Platform, setIcon } from 'obsidian'; +import { Platform, setIcon, setTooltip } from 'obsidian'; import { t } from '../../i18n'; +import type { RefreshStatus } from '../../logic/source-control/RefreshState'; import { ICONS } from '../components/icons'; import { renderPushButton } from './PushButton'; @@ -14,17 +15,20 @@ export interface SourceControlWorkspaceInfo { export interface SourceControlHeaderProps { readyToPushCount: number; workspaceInfo: SourceControlWorkspaceInfo; + refreshStatus: RefreshStatus; } export interface SourceControlHeaderCallbacks { onPush: () => void; + onRefresh: () => void; } /** - * Renders the Sync status view's connection/branch/last-sync info and Push - * button. No title here -- Obsidian's own tab header already shows "Sync - * status" (SourceControlItemView.getDisplayText), so repeating it in-panel - * duplicated the label, most visibly on mobile's stacked tab layout. + * Renders the Sync status view's connection/branch/last-sync info, Sync + * button, and Refresh button. No title here -- Obsidian's own tab header + * already shows "Sync status" (SourceControlItemView.getDisplayText), so + * repeating it in-panel duplicated the label, most visibly on mobile's + * stacked tab layout. */ export function renderSourceControlHeader( container: HTMLElement, @@ -34,10 +38,37 @@ export function renderSourceControlHeader( const header = container.createDiv({ cls: 'scv-header' }); const titleRow = header.createDiv({ cls: 'scv-header-title-row' }); renderPushButton(titleRow, props.readyToPushCount, callbacks.onPush); + renderRefreshButton(titleRow, props.refreshStatus, callbacks.onRefresh); renderInfoStrip(header, props.workspaceInfo); } +function renderRefreshButton(container: HTMLElement, status: RefreshStatus, onRefresh: () => void): void { + const btn = container.createEl('button', { cls: `scv-refresh-btn is-${status}` }); + btn.setAttr('aria-label', t('sourceControl.refresh.tooltip')); + setIcon(btn.createSpan({ cls: 'scv-refresh-btn-icon' }), ICONS.refresh); + + const label = btn.createSpan({ cls: 'scv-refresh-btn-label' }); + if (status === 'loading') { + label.textContent = t('sourceControl.refresh.refreshing'); + btn.disabled = true; + } else if (status === 'failed') { + label.textContent = t('sourceControl.refresh.failed'); + setTooltip(btn, t('sourceControl.refresh.failed')); + } else { + label.textContent = ''; + setTooltip(btn, t('sourceControl.refresh.tooltip')); + } + + // Show the label span only when there's text (loading/failed); idle stays icon-only. + if (status === 'idle') label.addClass('is-hidden'); + + btn.addEventListener('click', () => { + if (status === 'loading') return; + onRefresh(); + }); +} + function renderInfoStrip(container: HTMLElement, info: SourceControlWorkspaceInfo): void { const strip = container.createDiv({ cls: 'scv-info' }); diff --git a/src/ui/source-control/SourceControlItemView.ts b/src/ui/source-control/SourceControlItemView.ts index 602187d..1cc4508 100644 --- a/src/ui/source-control/SourceControlItemView.ts +++ b/src/ui/source-control/SourceControlItemView.ts @@ -33,6 +33,7 @@ export class SourceControlItemView extends ItemView { super(leaf); const callbacks: SourceControlViewCallbacks = { onPush: (changeIds) => this.runAction(this.plugin.sourceControlActions.push(changeIds)), + onRefresh: () => this.runRefresh(), loadDiffContent: (item: SourceControlItem) => this.plugin.sourceControlActions.loadDiffContent(item), // Desktop: the panel is a narrow sidebar, so the diff opens in a // full-width main-area tab instead of splitting that sidebar. @@ -109,4 +110,19 @@ export class SourceControlItemView extends ItemView { this.renderView(); void action.finally(() => this.renderView()); } + + /** + * Refresh reuses the same render-then-settle pattern as {@link runAction}, + * but the ViewModel's refresh() sets its `RefreshState` to 'loading' + * synchronously (before the first `await`), so the immediate render shows + * "Refreshing…". The settle render projects 'idle' on success or + * 'failed' on rejection. The rejection is swallowed here so a failed + * refresh surfaces as the button's failed state rather than an unhandled + * rejection — the state was already recorded on the `RefreshState` holder. + */ + private runRefresh(): void { + const refresh = this.plugin.sourceControlViewModel.refresh(); + this.renderView(); + void refresh.then(() => this.renderView(), () => this.renderView()); + } } diff --git a/src/ui/source-control/SourceControlView.ts b/src/ui/source-control/SourceControlView.ts index 41c7fdc..1545684 100644 --- a/src/ui/source-control/SourceControlView.ts +++ b/src/ui/source-control/SourceControlView.ts @@ -19,6 +19,8 @@ export interface SourceControlDiffContent { export interface SourceControlViewCallbacks { /** Hands push intent off to whatever wires this view to the sync pipeline; never called by the UI directly against a Git provider. */ onPush: (changeIds: ChangeId[]) => void | Promise; + /** Triggers a view-wide refresh; the host wires this to the ViewModel's refresh delegate. */ + onRefresh: () => void; /** Notified when a change is selected for diff viewing, in addition to this view's own diff pane rendering. */ onOpenDiff?: (item: SourceControlItem) => void | Promise; /** Supplies diff content for the selected change; omit to leave the diff pane empty. */ @@ -120,8 +122,15 @@ export class SourceControlView { renderSourceControlHeader( container, - { readyToPushCount: state.counts['ready-to-push'], workspaceInfo: this.getWorkspaceInfo() }, - { onPush: () => { void this.callbacks.onPush(this.selection.getSelectedChangeIds()); } }, + { + readyToPushCount: state.counts['ready-to-push'], + workspaceInfo: this.getWorkspaceInfo(), + refreshStatus: state.refreshStatus, + }, + { + onPush: () => { void this.callbacks.onPush(this.selection.getSelectedChangeIds()); }, + onRefresh: () => this.callbacks.onRefresh(), + }, ); this.renderSearchBox(container); diff --git a/styles.css b/styles.css index bb614b6..797b641 100644 --- a/styles.css +++ b/styles.css @@ -18,8 +18,8 @@ .scv-header-title-row { display: flex; align-items: center; - justify-content: flex-end; gap: 8px; + padding: 0 10px; } /* ── Workspace info strip (provider · branch · vault folder · last sync) ── */ @@ -248,8 +248,8 @@ align-items: center; justify-content: center; gap: 6px; - width: calc(100% - 20px); - margin: 8px 10px; + flex: 1 1 auto; + margin: 8px 0; padding: 7px 11px; border-radius: 5px; font-size: 0.85em; @@ -270,6 +270,43 @@ .scv-push-btn:not(:disabled):hover { opacity: 0.85; } +/* ── Refresh button ───────────────────────────────────────────── */ +.scv-refresh-btn { + display: flex; + align-items: center; + gap: 5px; + flex: 0 0 auto; + margin: 8px 0; + padding: 6px 9px; + border-radius: 5px; + font-size: 0.8em; + cursor: pointer; + border: 1px solid var(--background-modifier-border); + background: var(--background-modifier-form-field); + color: var(--text-muted); + min-height: 32px; +} + +.scv-refresh-btn:hover { color: var(--text-normal); } + +.scv-refresh-btn.is-loading { cursor: progress; } +.scv-refresh-btn.is-loading .scv-refresh-btn-icon { animation: scv-refresh-spin 1s linear infinite; } + +.scv-refresh-btn.is-failed { + color: var(--text-error); + border-color: var(--background-modifier-error-border, var(--background-modifier-border)); +} + +.scv-refresh-btn:disabled { opacity: 0.7; cursor: progress; } + +.scv-refresh-btn-label { white-space: nowrap; } +.scv-refresh-btn.is-idle .scv-refresh-btn-label { display: none; } + +@keyframes scv-refresh-spin { + from { transform: rotate(0deg); } + to { transform: rotate(360deg); } +} + /* ── Change sections & tree ───────────────────────────────────────── */ .scv-section-header { display: flex; @@ -446,11 +483,17 @@ .scv-op-indicator { display: inline-flex; align-items: center; + gap: 3px; flex-shrink: 0; } .scv-op-indicator .svg-icon { width: 14px; height: 14px; } +.scv-op-label { + font-size: 0.72em; + white-space: nowrap; +} + .scv-op-running { color: var(--text-muted); } .scv-op-success { color: var(--color-green); } .scv-op-failed { color: var(--color-red); } diff --git a/tests/ui/source-control/SourceControlItemView.test.ts b/tests/ui/source-control/SourceControlItemView.test.ts index 572fc7c..f48f4fa 100644 --- a/tests/ui/source-control/SourceControlItemView.test.ts +++ b/tests/ui/source-control/SourceControlItemView.test.ts @@ -81,6 +81,19 @@ describe('SourceControlItemView', () => { expect(push).toHaveBeenCalledWith([toChangeId('a.md')]); }); + it('forwards refresh clicks to the ViewModel refresh delegate', async () => { + const { plugin } = buildPlugin(); + const refreshSpy = vi.spyOn(plugin.sourceControlViewModel, 'refresh').mockResolvedValue(undefined); + const view = new SourceControlItemView({} as WorkspaceLeaf, plugin); + await view.onOpen(); + + const container = view.containerEl.children[1] as HTMLElement; + (container.querySelector('.scv-refresh-btn') as HTMLButtonElement).click(); + await Promise.resolve(); + + expect(refreshSpy).toHaveBeenCalledTimes(1); + }); + it('re-renders when the shared SyncStatusService publishes a change', async () => { const { plugin, repository, status } = buildPlugin(); const view = new SourceControlItemView({} as WorkspaceLeaf, plugin); diff --git a/tests/ui/source-control/SourceControlView.test.ts b/tests/ui/source-control/SourceControlView.test.ts index 67a00f8..dc5beee 100644 --- a/tests/ui/source-control/SourceControlView.test.ts +++ b/tests/ui/source-control/SourceControlView.test.ts @@ -20,13 +20,14 @@ function buildView(changes: SyncChange[], callbacks: Partial ({ + const onRefresh = callbacks.onRefresh ?? vi.fn(); + const view = new SourceControlView(viewModel, selection, { onPush, onRefresh, ...callbacks }, () => ({ serviceName: 'GitHub', branch: 'main', vaultFolder: '', lastSyncTime: 0, })); - return { view, selection, operations, refreshState, refreshSource, onPush }; + return { view, selection, operations, refreshState, refreshSource, onPush, onRefresh }; } describe('SourceControlView', () => { @@ -236,6 +237,14 @@ describe('SourceControlView', () => { expect(indicator?.classList.contains('scv-op-running')).toBe(true); }); + it('renders a text label alongside the operation indicator', () => { + const { view, operations } = buildView([{ id: toChangeId('c-1'), path: 'a.md', kind: 'local-only' }]); + operations.start(toChangeId('c-1')); + view.render(container); + + expect(container.querySelector('.scv-op-indicator .scv-op-label')?.textContent).toBe('Syncing'); + }); + it('shows no indicator once the operation is idle again', () => { const { view, operations } = buildView([{ id: toChangeId('c-1'), path: 'a.md', kind: 'local-only' }]); operations.start(toChangeId('c-1')); @@ -246,6 +255,68 @@ describe('SourceControlView', () => { }); }); + describe('refresh', () => { + it('renders the refresh button in the idle state by default', () => { + const { view } = buildView([{ id: toChangeId('c-1'), path: 'a.md', kind: 'local-only' }]); + view.render(container); + + const btn = container.querySelector('.scv-refresh-btn'); + expect(btn).not.toBeNull(); + expect(btn?.classList.contains('is-idle')).toBe(true); + }); + + it('renders the "Refreshing…" label while loading', () => { + const { view, refreshState } = buildView([{ id: toChangeId('c-1'), path: 'a.md', kind: 'local-only' }]); + view.render(container); + refreshState.start(); + view.render(container); + + const btn = container.querySelector('.scv-refresh-btn'); + expect(btn?.classList.contains('is-loading')).toBe(true); + expect(btn?.querySelector('.scv-refresh-btn-label')?.textContent).toBe('Refreshing…'); + expect((btn as HTMLButtonElement).disabled).toBe(true); + }); + + it('renders the "Refresh failed" label in the failed state', () => { + const { view, refreshState } = buildView([{ id: toChangeId('c-1'), path: 'a.md', kind: 'local-only' }]); + view.render(container); + refreshState.fail(); + view.render(container); + + const btn = container.querySelector('.scv-refresh-btn'); + expect(btn?.classList.contains('is-failed')).toBe(true); + expect(btn?.querySelector('.scv-refresh-btn-label')?.textContent).toBe('Refresh failed'); + }); + + it('calls onRefresh when the refresh button is clicked', () => { + const onRefresh = vi.fn(); + const { view } = buildView( + [{ id: toChangeId('c-1'), path: 'a.md', kind: 'local-only' }], + { onRefresh }, + ); + view.render(container); + + (container.querySelector('.scv-refresh-btn') as HTMLButtonElement).click(); + + expect(onRefresh).toHaveBeenCalledTimes(1); + }); + + it('does not call onRefresh while a refresh is already loading', () => { + const onRefresh = vi.fn(); + const { view, refreshState } = buildView( + [{ id: toChangeId('c-1'), path: 'a.md', kind: 'local-only' }], + { onRefresh }, + ); + view.render(container); + refreshState.start(); + view.render(container); + + (container.querySelector('.scv-refresh-btn') as HTMLButtonElement).click(); + + expect(onRefresh).not.toHaveBeenCalled(); + }); + }); + describe('diff selection', () => { // Desktop has no inline diff pane -- clicking a change only notifies // onOpenDiff, and the host (SourceControlItemView) opens a main-area From dd8ddd5761d5e146c7ae0a146bff60d882575f9f Mon Sep 17 00:00:00 2001 From: Tianyao Date: Sun, 23 Aug 2026 00:06:16 +0800 Subject: [PATCH 04/17] feat(source-control): presentation adapter, diff stat, responsive mobile Add ChangePresentation UI adapter so all kind-specific presentation (badge letter, subtitle, rename display, deleted-locally tooltip) lives in the UI layer, keeping the domain filters/summary semantics-only. remote-only is badged 'D' (deleted locally) rather than 'A'; rename and subtitle move out of ChangeItem into the adapter. Thread an optional diff-stat through each row: local-only stats are eagerly resolved from the in-memory sync.status (no provider call) on render and cached; two-sided stats lazy-load on open and reuse the diff content the pane already fetches. The cache clears on refresh. Null results (binary/missing content) are cached too so they aren't retried every rerender. Responsive mobile layout: chips collapse to a single filter dropdown, the header push button is hidden, and a sticky bottom sync bar appears when a push selection exists. The mobile tree uses a flatter shape (collapseSingleChild + maxDepth). No src/logic/source-control/ files touched beyond ViewModel + RefreshState. - New: src/ui/source-control/ChangePresentation.ts - New i18n: sourceControl.status.{added,modified,renamed,deletedLocally, modifiedRemotely,conflict,synced} + deletedLocally.tooltip (en/zh-cn/zh-tw) - Tests: ChangePresentation (badge/subtitle/rename/stat), ChangeTree (remote-only D, subtitle, diff-stat span), SourceControlView (stat caching/clear-on-refresh/lazy-load, mobile dropdown + bottom sync bar) - 629 tests pass, eslint clean, build + Obsidian 1.11 compat pass --- src/i18n/locales/en.ts | 8 ++ src/i18n/locales/zh-cn.ts | 8 ++ src/i18n/locales/zh-tw.ts | 8 ++ src/ui/source-control/ChangeItem.ts | 51 +++---- src/ui/source-control/ChangePresentation.ts | 101 ++++++++++++++ src/ui/source-control/FilterMenu.ts | 57 ++++++-- src/ui/source-control/SourceControlHeader.ts | 10 +- .../source-control/SourceControlItemView.ts | 24 ++++ src/ui/source-control/SourceControlView.ts | 74 +++++++++- styles.css | 75 ++++++++++- .../source-control/ChangePresentation.test.ts | 114 ++++++++++++++++ tests/ui/source-control/ChangeTree.test.ts | 29 ++++ .../source-control/SourceControlView.test.ts | 127 ++++++++++++++++++ 13 files changed, 645 insertions(+), 41 deletions(-) create mode 100644 src/ui/source-control/ChangePresentation.ts create mode 100644 tests/ui/source-control/ChangePresentation.test.ts diff --git a/src/i18n/locales/en.ts b/src/i18n/locales/en.ts index 7b8f1ad..7038db6 100644 --- a/src/i18n/locales/en.ts +++ b/src/i18n/locales/en.ts @@ -273,6 +273,14 @@ const en = { 'sourceControl.op.syncing': 'Syncing', 'sourceControl.op.synced': 'Synced', 'sourceControl.op.failed': 'Failed', + 'sourceControl.status.added': 'Added', + 'sourceControl.status.modified': 'Modified', + 'sourceControl.status.renamed': 'Renamed', + 'sourceControl.status.deletedLocally': 'Deleted locally', + 'sourceControl.status.deletedLocally.tooltip': 'Remote file will be removed during sync', + 'sourceControl.status.modifiedRemotely': 'Modified remotely', + 'sourceControl.status.conflict': 'Conflict', + 'sourceControl.status.synced': 'Synced', 'sourceControl.empty': 'No changes', 'sourceControl.detail.back': 'Back', 'sourceControl.info.lastSync': 'Last sync: {time}', diff --git a/src/i18n/locales/zh-cn.ts b/src/i18n/locales/zh-cn.ts index 52a1089..f97c41a 100644 --- a/src/i18n/locales/zh-cn.ts +++ b/src/i18n/locales/zh-cn.ts @@ -275,6 +275,14 @@ const zhCn: Partial> = { 'sourceControl.op.syncing': '同步中', 'sourceControl.op.synced': '已同步', 'sourceControl.op.failed': '失败', + 'sourceControl.status.added': '已添加', + 'sourceControl.status.modified': '已修改', + 'sourceControl.status.renamed': '已重命名', + 'sourceControl.status.deletedLocally': '本地已删除', + 'sourceControl.status.deletedLocally.tooltip': '远程文件将在同步时被移除', + 'sourceControl.status.modifiedRemotely': '远程已修改', + 'sourceControl.status.conflict': '冲突', + 'sourceControl.status.synced': '已同步', 'sourceControl.empty': '没有更改', 'sourceControl.detail.back': '返回', 'sourceControl.info.lastSync': '上次同步：{time}', diff --git a/src/i18n/locales/zh-tw.ts b/src/i18n/locales/zh-tw.ts index 7c8950c..d3dea73 100644 --- a/src/i18n/locales/zh-tw.ts +++ b/src/i18n/locales/zh-tw.ts @@ -275,6 +275,14 @@ const zhTw: Partial> = { 'sourceControl.op.syncing': '同步中', 'sourceControl.op.synced': '已同步', 'sourceControl.op.failed': '失敗', + 'sourceControl.status.added': '已新增', + 'sourceControl.status.modified': '已修改', + 'sourceControl.status.renamed': '已重新命名', + 'sourceControl.status.deletedLocally': '本地已刪除', + 'sourceControl.status.deletedLocally.tooltip': '遠端檔案將於同步時被移除', + 'sourceControl.status.modifiedRemotely': '遠端已修改', + 'sourceControl.status.conflict': '衝突', + 'sourceControl.status.synced': '已同步', 'sourceControl.empty': '沒有變更', 'sourceControl.detail.back': '返回', 'sourceControl.info.lastSync': '上次同步：{time}', diff --git a/src/ui/source-control/ChangeItem.ts b/src/ui/source-control/ChangeItem.ts index bbb312c..027b945 100644 --- a/src/ui/source-control/ChangeItem.ts +++ b/src/ui/source-control/ChangeItem.ts @@ -1,57 +1,53 @@ import { setIcon } from 'obsidian'; import { ICONS } from '../components/icons'; import { renderOperationIndicator } from './OperationIndicator'; +import { presentChange, type ChangeStat } from './ChangePresentation'; import type { SourceControlItem } from '../../logic/source-control/SourceControlViewModel'; -import type { ChangeId, SyncChangeKind } from '../../logic/source-control/types'; +import type { ChangeId } from '../../logic/source-control/types'; export interface ChangeItemCallbacks { onToggleSelect: (id: ChangeId, selected: boolean) => void; onOpenDiff: (item: SourceControlItem) => void; -} - -interface KindBadge { - letter: string; - cls: string; + /** Looks up a cached diff stat for a row, if one has been computed. */ + getDiffStat?: (id: ChangeId) => ChangeStat | undefined; } /** - * Single-letter status badge per change kind, matching the VS Code style - * tree example in the Phase 3 spec (`M daily.md`, `A idea.md`, `! settings.md`). + * Renders a single change row: selection checkbox, status badge, name (with + * rename arrow for moves), optional diff-stat span, and operation + * indicator. All kind-specific presentation (badge letter, subtitle, + * rename display) comes from {@link presentChange} so this component stays a + * pure renderer. */ -const KIND_BADGE: Record = { - 'local-only': { letter: 'A', cls: 'local-only' }, - 'local-modified': { letter: 'M', cls: 'local-modified' }, - 'remote-only': { letter: 'A', cls: 'remote-only' }, - 'remote-modified': { letter: 'M', cls: 'remote-modified' }, - moved: { letter: 'R', cls: 'moved' }, - conflict: { letter: '!', cls: 'conflict' }, - synced: { letter: 'S', cls: 'synced' }, -}; - -/** Renders a single change row: selection checkbox, status badge, name, operation indicator. */ export function renderChangeItem( container: HTMLElement, item: SourceControlItem, displayName: string, callbacks: ChangeItemCallbacks, ): HTMLElement { + const view = presentChange(item, displayName); + const row = container.createDiv({ cls: `scv-change-item scv-kind-${item.kind}` }); row.setAttr('data-change-id', item.id); + if (view.tooltip) row.setAttr('title', view.tooltip); const checkbox = row.createEl('input', { type: 'checkbox', cls: 'scv-change-select' }); checkbox.checked = item.isReadyToPush; checkbox.addEventListener('change', () => callbacks.onToggleSelect(item.id, checkbox.checked)); - const badge = KIND_BADGE[item.kind]; + const badge = view.badge; row.createSpan({ cls: `scv-badge scv-badge-${badge.cls}`, text: badge.letter }); const label = row.createDiv({ cls: 'scv-change-name' }); - if (item.previousPath) { - const previousName = item.previousPath.split('/').pop() ?? item.previousPath; - label.createSpan({ cls: 'scv-change-rename-from', text: previousName }); + if (view.renameFrom) { + label.createSpan({ cls: 'scv-change-rename-from', text: view.renameFrom }); setIcon(label.createSpan({ cls: 'scv-change-rename-arrow' }), ICONS.moved); } - label.createSpan({ cls: 'scv-change-name-text', text: displayName }); + label.createSpan({ cls: 'scv-change-name-text', text: view.displayName }); + label.createSpan({ cls: 'scv-change-subtitle', text: view.subtitle }); + + const stat = callbacks.getDiffStat?.(item.id); + if (stat) row.createSpan({ cls: 'scv-diff-stat', text: formatStat(stat) }); renderOperationIndicator(row, item.operationStatus); @@ -62,3 +58,10 @@ export function renderChangeItem( return row; } + +function formatStat(stat: ChangeStat): string { + const parts: string[] = []; + if (stat.additions > 0) parts.push(`+${stat.additions}`); + if (stat.deletions > 0) parts.push(`-${stat.deletions}`); + return parts.join(' '); +} \ No newline at end of file diff --git a/src/ui/source-control/ChangePresentation.ts b/src/ui/source-control/ChangePresentation.ts new file mode 100644 index 0000000..cd41171 --- /dev/null +++ b/src/ui/source-control/ChangePresentation.ts @@ -0,0 +1,101 @@ +import { computeSideBySideDiff } from '../../utils/diff'; +import { t, type TranslationKey } from '../../i18n'; +import type { SourceControlItem } from '../../logic/source-control/SourceControlViewModel'; +import type { SyncChangeKind } from '../../logic/source-control/types'; + +/** Additions/deletions for a single change's diff, the +/- stat a row shows. */ +export interface ChangeStat { + additions: number; + deletions: number; +} + +/** + * UI-only presentation of one change: the badge letter + class, a short + * subtitle, the display name (with rename "from" separated out), and an + * optional tooltip. All UI-specific meaning (M/A/D/R icons, "Deleted + * locally" wording, rename arrow) lives here rather than in the domain, so + * `SyncChangeKind` / `SourceControlFilter` / `SourceControlSummary` stay + * semantics-only and untouched. + */ +export interface ChangeRowView { + badge: { letter: string; cls: string }; + subtitle: string; + displayName: string; + /** Present for a tracked rename: the old name shown before an arrow and `displayName`. */ + renameFrom?: string; + tooltip?: string; +} + +const SUBTITLE_KEYS: Record = { + 'local-only': 'sourceControl.status.added', + 'local-modified': 'sourceControl.status.modified', + 'remote-only': 'sourceControl.status.deletedLocally', + 'remote-modified': 'sourceControl.status.modifiedRemotely', + moved: 'sourceControl.status.renamed', + conflict: 'sourceControl.status.conflict', + synced: 'sourceControl.status.synced', +}; + +/** + * Single-letter status badge per change kind. Note `remote-only` (locally + * deleted) is badged `D`, not `A` — a local deletion is what the user sees, + * per the resolved decision to keep the domain filter semantics (remote-only + * stays in the Remote bucket) while the UI row reads "Deleted locally". + */ +const BADGE: Record = { + 'local-only': { letter: 'A', cls: 'local-only' }, + 'local-modified': { letter: 'M', cls: 'local-modified' }, + 'remote-only': { letter: 'D', cls: 'remote-only' }, + 'remote-modified': { letter: 'M', cls: 'remote-modified' }, + moved: { letter: 'R', cls: 'moved' }, + conflict: { letter: '!', cls: 'conflict' }, + synced: { letter: 'S', cls: 'synced' }, +}; + +/** + * Projects a {@link SourceControlItem} into a UI row view. `displayName` is + * the tree node's file name (passed in from `ChangeTree`); the rename "from" + * name is derived here from `item.previousPath` so the rename-arrow rendering + * moves out of `ChangeItem`. + */ +export function presentChange(item: SourceControlItem, displayName: string): ChangeRowView { + const view: ChangeRowView = { + badge: BADGE[item.kind], + subtitle: t(SUBTITLE_KEYS[item.kind]), + displayName, + }; + if (item.previousPath) view.renameFrom = item.previousPath.split('/').pop() ?? item.previousPath; + if (item.kind === 'remote-only') view.tooltip = t('sourceControl.status.deletedLocally.tooltip'); + return view; +} + +/** + * +/- stat for a two-sided diff (local-modified / remote-only / + * remote-modified / moved / conflict), reusing the existing LCS op logic in + * `utils/diff.ts`. Additions = added ops, deletions = removed ops. + */ +export function computeDiffStat(remote: string, local: string): ChangeStat { + const rows = computeSideBySideDiff(remote, local); + let additions = 0; + let deletions = 0; + for (const row of rows) { + if (row.right.type === 'added') additions++; + if (row.left.type === 'removed') deletions++; + } + return { additions, deletions }; +} + +/** + * Cheap stat for a `local-only` change: additions only (the local line + * count), no deletions and no remote/provider call. A trailing newline + * doesn't add a phantom line. + */ +export function cheapLocalStat(local: string): ChangeStat { + return { additions: countLines(local), deletions: 0 }; +} + +function countLines(s: string): number { + if (s === '') return 0; + const lines = s.replace(/\r\n/g, '\n').replace(/\r/g, '\n').split('\n'); + return lines[lines.length - 1] === '' ? lines.length - 1 : lines.length; +} \ No newline at end of file diff --git a/src/ui/source-control/FilterMenu.ts b/src/ui/source-control/FilterMenu.ts index 292ea50..806a642 100644 --- a/src/ui/source-control/FilterMenu.ts +++ b/src/ui/source-control/FilterMenu.ts @@ -35,12 +35,21 @@ export interface FilterMenuCallbacks { onToggleShowSynced: (show: boolean) => void; } +export interface FilterMenuOptions { + /** When true a compact `` dropdown replaces the chips (same domain + * values, same counts inline as "Label (N)"), with the "Show synced" toggle + * kept below it. + * * Per-filter counts come straight from the ViewModel's single-source counts; * the menu never recomputes one. */ @@ -50,25 +59,51 @@ export function renderFilterMenu( counts: Record, showSynced: boolean, callbacks: FilterMenuCallbacks, + options: FilterMenuOptions = {}, ): void { const menu = container.createDiv({ cls: 'scv-filter-menu' }); - const renderChip = (value: SourceControlFilter): void => { - const isActive = value === current; - const btn = menu.createEl('button', { cls: `scv-filter-option${isActive ? ' is-active' : ''}` }); - btn.setAttr('data-filter', value); - btn.setAttr('aria-pressed', String(isActive)); - btn.createSpan({ cls: 'scv-filter-label', text: t(FILTER_LABEL_KEYS[value]) }); - btn.createSpan({ cls: 'scv-filter-count', text: String(counts[value] ?? 0) }); - btn.addEventListener('click', () => callbacks.onFilterChange(value)); - }; + if (options.isMobile) { + renderFilterDropdown(menu, current, counts, callbacks); + } else { + const renderChip = (value: SourceControlFilter): void => { + const isActive = value === current; + const btn = menu.createEl('button', { cls: `scv-filter-option${isActive ? ' is-active' : ''}` }); + btn.setAttr('data-filter', value); + btn.setAttr('aria-pressed', String(isActive)); + btn.createSpan({ cls: 'scv-filter-label', text: t(FILTER_LABEL_KEYS[value]) }); + btn.createSpan({ cls: 'scv-filter-count', text: String(counts[value] ?? 0) }); + btn.addEventListener('click', () => callbacks.onFilterChange(value)); + }; - for (const value of ACTION_FILTERS) renderChip(value); - if (showSynced) renderChip('synced'); + for (const value of ACTION_FILTERS) renderChip(value); + if (showSynced) renderChip('synced'); + } const toggle = menu.createEl('label', { cls: 'scv-filter-show-synced' }); const checkbox = toggle.createEl('input', { type: 'checkbox', cls: 'scv-filter-show-synced-checkbox' }); checkbox.checked = showSynced; checkbox.addEventListener('change', () => callbacks.onToggleShowSynced(checkbox.checked)); toggle.createSpan({ cls: 'scv-filter-show-synced-label', text: t('sourceControl.filter.showSynced') }); +} + +/** + * Mobile filter dropdown: one ``, header push button + hidden, sticky bottom sync bar, flatter tree (`maxDepth: 2`). + +Domain-untouched invariant verified: +`git diff claude/source-control-foundation -- src/logic/source-control/` +shows ONLY `RefreshState.ts` (new) + `SourceControlViewModel.ts` (edited). ## Verification Evidence ```text npx eslint . -> PASS, 0 errors -npm run build -> PASS, incl. Obsidian 1.11 compatibility -npx vitest run -> PASS, 56 files / 613 tests -npm run test:e2e -- --provider gitea -> PASS, 2 files / 14 tests; container removed -actionlint v1.7.12 .github/workflows/ci.yml -> PASS, 0 errors -git diff --check -> PASS -real CI run 32338116598 -> PASS after failed-only rerun of a disabled Gitea leg assigned to an offline runner -GitHub/GitLab sandbox branch query -> PASS, no e2e/pr/127 or source-branch refs remain +npm run build -> PASS, incl. Obsidian 1.11.0 compat typecheck + esbuild +npx vitest run -> PASS, 61 files / 629 tests +git diff claude/source-control-foundation -- src/logic/source-control/ -> only RefreshState.ts + SourceControlViewModel.ts ``` -The AGENTS-required Haiku verifier was unavailable in this environment, so verification ran -locally in this session. +Pre-commit husky hook (`npm run lint && npm run build`) ran green on every +commit. ## Exact Next Step -Complete the remaining Obsidian desktop/mobile move smoke tests. Verify moving and editing a -tracked file appears under Moves and applies as one remote move, while an occupied remote -destination remains a skipped conflict. +The plan's four commits are all landed and locally green. Remaining before +declaring the feature fully done per AGENTS.md Definition of Done: +- Manual Obsidian verification (desktop + mobile) of the runtime UI surface: + refresh button states, "SELECTED FOR SYNC" section, per-row subtitles/badges + (esp. `remote-only` → `D` "Deleted locally"), diff-stat `+N -M` spans, and + the mobile filter dropdown + bottom sync bar. +- If opening a PR is desired, push `feat/sync-status-workflow-ui` and open a PR + against the base branch (`claude/source-control-foundation`) with the four + commits; the base branch name should be confirmed with the user first. \ No newline at end of file From 853793c307925ef8aa286c9c12fefe9fbabc6084 Mon Sep 17 00:00:00 2001 From: Tianyao Date: Sun, 23 Aug 2026 00:27:10 +0800 Subject: [PATCH 06/17] feat(source-control): selected-section rows, drop show-synced toggle, colored diff-stat - Selected section now lists actual selected change rows (full renderChangeItem rows with unselecting checkboxes) instead of a bare count; sits in a boxed region between the filter and the tree. - Tree keeps selected rows visible but muted via an is-selected class (italic name, reduced opacity) so context isn't lost. - Remove the 'Show synced' toggle and the synced chip from the UI; the domain synced filter/summary stay computed by the ViewModel but have no entry point. - Drop the inline status subtitle from each row; the kind label now lives on the badge tooltip, removing the M/Modified redundancy. - Split the diff stat into green additions / red deletions spans. - Use design tokens (--radius-s/--radius-m) for chip and section radii. --- src/ui/source-control/ChangeItem.ts | 44 ++++++---- src/ui/source-control/FilterMenu.ts | 57 +++++-------- src/ui/source-control/SourceControlView.ts | 82 +++++++++++-------- styles.css | 61 ++++++++++---- tests/ui/setup-dom.ts | 3 +- tests/ui/source-control/ChangeTree.test.ts | 20 ++++- tests/ui/source-control/FilterMenu.test.ts | 46 ++++------- .../source-control/SourceControlView.test.ts | 60 +++++++------- 8 files changed, 206 insertions(+), 167 deletions(-) diff --git a/src/ui/source-control/ChangeItem.ts b/src/ui/source-control/ChangeItem.ts index 027b945..a8cbafa 100644 --- a/src/ui/source-control/ChangeItem.ts +++ b/src/ui/source-control/ChangeItem.ts @@ -1,4 +1,4 @@ -import { setIcon } from 'obsidian'; +import { setIcon, setTooltip } from 'obsidian'; import { ICONS } from '../components/icons'; import { renderOperationIndicator } from './OperationIndicator'; import { presentChange, type ChangeStat } from './ChangePresentation'; @@ -14,10 +14,15 @@ export interface ChangeItemCallbacks { /** * Renders a single change row: selection checkbox, status badge, name (with - * rename arrow for moves), optional diff-stat span, and operation - * indicator. All kind-specific presentation (badge letter, subtitle, - * rename display) comes from {@link presentChange} so this component stays a - * pure renderer. + * rename arrow for moves), optional diff-stat, and operation indicator. All + * kind-specific presentation (badge letter, kind label, rename display) + * comes from {@link presentChange} so this component stays a pure renderer. + * + * The kind's short label (e.g. "Modified") is shown as the badge tooltip + * rather than an inline subtitle, so the row reads `M name +3 -1` + * without the `M`/`Modified` redundancy. A row selected for push gets an + * `is-selected` class so the tree can keep it visible but visually muted + * while the dedicated Selected section carries the working copy. */ export function renderChangeItem( container: HTMLElement, @@ -27,7 +32,7 @@ export function renderChangeItem( ): HTMLElement { const view = presentChange(item, displayName); - const row = container.createDiv({ cls: `scv-change-item scv-kind-${item.kind}` }); + const row = container.createDiv({ cls: `scv-change-item scv-kind-${item.kind}${item.isReadyToPush ? ' is-selected' : ''}` }); row.setAttr('data-change-id', item.id); if (view.tooltip) row.setAttr('title', view.tooltip); @@ -35,8 +40,8 @@ export function renderChangeItem( checkbox.checked = item.isReadyToPush; checkbox.addEventListener('change', () => callbacks.onToggleSelect(item.id, checkbox.checked)); - const badge = view.badge; - row.createSpan({ cls: `scv-badge scv-badge-${badge.cls}`, text: badge.letter }); + const badgeEl = row.createSpan({ cls: `scv-badge scv-badge-${view.badge.cls}`, text: view.badge.letter }); + setTooltip(badgeEl, view.subtitle); const label = row.createDiv({ cls: 'scv-change-name' }); if (view.renameFrom) { @@ -44,10 +49,8 @@ export function renderChangeItem( setIcon(label.createSpan({ cls: 'scv-change-rename-arrow' }), ICONS.moved); } label.createSpan({ cls: 'scv-change-name-text', text: view.displayName }); - label.createSpan({ cls: 'scv-change-subtitle', text: view.subtitle }); - const stat = callbacks.getDiffStat?.(item.id); - if (stat) row.createSpan({ cls: 'scv-diff-stat', text: formatStat(stat) }); + renderDiffStat(row, callbacks.getDiffStat?.(item.id)); renderOperationIndicator(row, item.operationStatus); @@ -59,9 +62,18 @@ export function renderChangeItem( return row; } -function formatStat(stat: ChangeStat): string { - const parts: string[] = []; - if (stat.additions > 0) parts.push(`+${stat.additions}`); - if (stat.deletions > 0) parts.push(`-${stat.deletions}`); - return parts.join(' '); +/** + * Renders the +/- diff stat as two colored spans (green additions, red + * deletions) so the magnitude and direction read at a glance. Nothing is + * rendered when the stat is unavailable or zero on both sides. + */ +function renderDiffStat(row: HTMLElement, stat: ChangeStat | undefined): void { + if (!stat) return; + const hasAdd = stat.additions > 0; + const hasDel = stat.deletions > 0; + if (!hasAdd && !hasDel) return; + const wrap = row.createSpan({ cls: 'scv-diff-stat' }); + if (hasAdd) wrap.createSpan({ cls: 'scv-diff-stat-add', text: `+${stat.additions}` }); + if (hasAdd && hasDel) wrap.createSpan({ cls: 'scv-diff-stat-sep', text: ' ' }); + if (hasDel) wrap.createSpan({ cls: 'scv-diff-stat-del', text: `-${stat.deletions}` }); } \ No newline at end of file diff --git a/src/ui/source-control/FilterMenu.ts b/src/ui/source-control/FilterMenu.ts index 806a642..c729bf1 100644 --- a/src/ui/source-control/FilterMenu.ts +++ b/src/ui/source-control/FilterMenu.ts @@ -5,10 +5,10 @@ import type { SourceControlFilter } from '../../logic/source-control/SourceContr * Action filter chips, in spec order. The visible row is four chips — * All / Local / Remote / Conflict — backed by the unchanged domain filters * (`all` / `changes` / `remote-changes` / `conflicts`). "Ready to Push" is no - * longer a chip: it's surfaced as the inline "SELECTED FOR SYNC (N)" section - * instead. `synced` is deliberately NOT a permanent chip — it surfaces only - * when the user opts in via the "Show synced" toggle, so a quiet workspace - * isn't dominated by a large synced count. + * longer a chip: it's surfaced as the dedicated "SELECTED FOR SYNC" section + * instead. `synced` is intentionally not surfaced in the UI: a quiet + * workspace stays quiet, and the domain `synced` filter/summary (still + * computed by the ViewModel) simply has no chip to open it. */ const ACTION_FILTERS: SourceControlFilter[] = ['all', 'changes', 'remote-changes', 'conflicts']; @@ -16,8 +16,8 @@ const ACTION_FILTERS: SourceControlFilter[] = ['all', 'changes', 'remote-changes * Displayed chip labels. Domain values stay as `data-filter` attributes; only * the visible label changes (e.g. the `changes` domain filter reads "Local" * because it surfaces local-side changes). `ready-to-push` and `synced` keep - * their existing keys even though `ready-to-push` is no longer a chip, so the - * record stays total over {@link SourceControlFilter}. + * their keys so the record stays total over {@link SourceControlFilter}, + * even though neither is a chip. */ const FILTER_LABEL_KEYS: Record = { all: 'sourceControl.filter.all', @@ -31,8 +31,6 @@ const FILTER_LABEL_KEYS: Record = { export interface FilterMenuCallbacks { /** Switches the active filter chip. */ onFilterChange: (filter: SourceControlFilter) => void; - /** Toggles whether synced changes are surfaced at all (the "Show synced" switch). */ - onToggleShowSynced: (show: boolean) => void; } export interface FilterMenuOptions { @@ -42,13 +40,8 @@ export interface FilterMenuOptions { /** * Renders the Source Control filter row: the four action chips (All, Local, - * Remote, Conflict) followed by a "Show synced" toggle. The `synced` chip is - * appended only when `showSynced` is on, so a hidden synced bucket contributes - * no chip and no count to the row. - * - * On mobile a single `` dropdown replaces the + * chips (same domain values, counts inline as "Label (N)"). * * Per-filter counts come straight from the ViewModel's single-source counts; * the menu never recomputes one. @@ -57,7 +50,6 @@ export function renderFilterMenu( container: HTMLElement, current: SourceControlFilter, counts: Record, - showSynced: boolean, callbacks: FilterMenuCallbacks, options: FilterMenuOptions = {}, ): void { @@ -65,26 +57,20 @@ export function renderFilterMenu( if (options.isMobile) { renderFilterDropdown(menu, current, counts, callbacks); - } else { - const renderChip = (value: SourceControlFilter): void => { - const isActive = value === current; - const btn = menu.createEl('button', { cls: `scv-filter-option${isActive ? ' is-active' : ''}` }); - btn.setAttr('data-filter', value); - btn.setAttr('aria-pressed', String(isActive)); - btn.createSpan({ cls: 'scv-filter-label', text: t(FILTER_LABEL_KEYS[value]) }); - btn.createSpan({ cls: 'scv-filter-count', text: String(counts[value] ?? 0) }); - btn.addEventListener('click', () => callbacks.onFilterChange(value)); - }; - - for (const value of ACTION_FILTERS) renderChip(value); - if (showSynced) renderChip('synced'); + return; } - const toggle = menu.createEl('label', { cls: 'scv-filter-show-synced' }); - const checkbox = toggle.createEl('input', { type: 'checkbox', cls: 'scv-filter-show-synced-checkbox' }); - checkbox.checked = showSynced; - checkbox.addEventListener('change', () => callbacks.onToggleShowSynced(checkbox.checked)); - toggle.createSpan({ cls: 'scv-filter-show-synced-label', text: t('sourceControl.filter.showSynced') }); + const renderChip = (value: SourceControlFilter): void => { + const isActive = value === current; + const btn = menu.createEl('button', { cls: `scv-filter-option${isActive ? ' is-active' : ''}` }); + btn.setAttr('data-filter', value); + btn.setAttr('aria-pressed', String(isActive)); + btn.createSpan({ cls: 'scv-filter-label', text: t(FILTER_LABEL_KEYS[value]) }); + btn.createSpan({ cls: 'scv-filter-count', text: String(counts[value] ?? 0) }); + btn.addEventListener('click', () => callbacks.onFilterChange(value)); + }; + + for (const value of ACTION_FILTERS) renderChip(value); } /** @@ -98,9 +84,8 @@ function renderFilterDropdown( counts: Record, callbacks: FilterMenuCallbacks, ): void { - const values: SourceControlFilter[] = [...ACTION_FILTERS]; const select = menu.createEl('select', { cls: 'scv-filter-dropdown' }); - for (const value of values) { + for (const value of ACTION_FILTERS) { const option = select.createEl('option', { value }); option.textContent = `${t(FILTER_LABEL_KEYS[value])} (${counts[value] ?? 0})`; if (value === current) option.setAttr('selected', 'selected'); diff --git a/src/ui/source-control/SourceControlView.ts b/src/ui/source-control/SourceControlView.ts index 0901b39..1e192ac 100644 --- a/src/ui/source-control/SourceControlView.ts +++ b/src/ui/source-control/SourceControlView.ts @@ -9,6 +9,7 @@ import { ICONS } from '../components/icons'; import { renderDiffLayoutToggle, type DiffLayout } from '../components/DiffLayoutToggle'; import { renderDiffPanel } from '../components/DiffPanel'; import { renderChangeTree, type ChangeTreeCallbacks } from './ChangeTree'; +import { renderChangeItem } from './ChangeItem'; import { renderFilterMenu } from './FilterMenu'; import { renderSourceControlHeader, type SourceControlWorkspaceInfo } from './SourceControlHeader'; @@ -76,12 +77,16 @@ const MOBILE_TREE_OPTIONS = { collapseSingleChild: true, maxDepth: 2 }; * - Every filter — including "All" — renders a single flat tree. "All" no * longer breaks the view into CHANGES / REMOTE CHANGES / SYNCED sections, so * a change never appears twice and SYNCED never leaks into All. - * - Synced is hidden by default (`showSynced = false`): the `synced` chip is - * absent and synced rows render nowhere. The "Show synced" toggle opts in. + * - Synced is not surfaced in the UI: there is no `synced` chip and no + * "Show synced" toggle, so a quiet workspace stays quiet. The domain + * `synced` filter/summary are still computed by the ViewModel but simply + * have no entry point here. + * - Selected changes get a first-class "SELECTED FOR SYNC (N)" region + * (between the filter and the tree) listing the working push batch; the + * same rows remain in the tree, visually muted via `is-selected`. */ export class SourceControlView { private filter: SourceControlFilter = 'all'; - private showSynced = false; private searchQuery = ''; private readonly collapsedFolders = new Set(); private selectedChangeId: ChangeId | null = null; @@ -122,7 +127,6 @@ export class SourceControlView { } getFilter(): SourceControlFilter { return this.filter; } - getShowSynced(): boolean { return this.showSynced; } getSelectedChangeId(): ChangeId | null { return this.selectedChangeId; } private rerender(): void { @@ -130,7 +134,7 @@ export class SourceControlView { } private renderMain(container: HTMLElement): void { - const state = this.viewModel.getState(this.filter, this.showSynced); + const state = this.viewModel.getState(this.filter); const isMobile = Platform.isMobile; @@ -153,27 +157,26 @@ export class SourceControlView { this.renderSearchBox(container); + const treeCallbacks: ChangeTreeCallbacks = { + onToggleFolder: (path) => this.toggleFolder(path), + onToggleSelect: (id, selected) => this.toggleSelect(id, selected), + onToggleFolderSelect: (ids, selected) => this.toggleFolderSelect(ids, selected), + onOpenDiff: (item) => this.openDiff(item), + getDiffStat: (id) => this.diffStatCache.get(id) ?? undefined, + }; + renderFilterMenu( container, this.filter, state.counts, - this.showSynced, - { - onFilterChange: (filter) => { this.filter = filter; this.rerender(); }, - onToggleShowSynced: (show) => { - this.showSynced = show; - // If the user hid synced while viewing it, fall back to All. - if (!show && this.filter === 'synced') this.filter = 'all'; - this.rerender(); - }, - }, + { onFilterChange: (filter) => { this.filter = filter; this.rerender(); } }, { isMobile }, ); const query = this.searchQuery.trim().toLowerCase(); const items = query ? state.items.filter(item => item.path.toLowerCase().includes(query)) : state.items; - this.renderSelectedSection(container, state.selectedItems); + this.renderSelectedSection(container, state.selectedItems, treeCallbacks); const body = container.createDiv({ cls: 'scv-body' }); this.renderActiveFilterHeader(body, state.filter, items.length); @@ -182,14 +185,6 @@ export class SourceControlView { return; } - const treeCallbacks: ChangeTreeCallbacks = { - onToggleFolder: (path) => this.toggleFolder(path), - onToggleSelect: (id, selected) => this.toggleSelect(id, selected), - onToggleFolderSelect: (ids, selected) => this.toggleFolderSelect(ids, selected), - onOpenDiff: (item) => this.openDiff(item), - getDiffStat: (id) => this.diffStatCache.get(id) ?? undefined, - }; - renderChangeTree(body, items, this.collapsedFolders, treeCallbacks, isMobile ? MOBILE_TREE_OPTIONS : TREE_OPTIONS); this.eagerLoadLocalStats(items); @@ -256,18 +251,31 @@ export class SourceControlView { } /** - * Renders the "SELECTED FOR SYNC (N)" summary, only when the user has at - * least one actionable change selected for push. Sits above the tree so the - * current push batch is always visible regardless of the active filter. - * The count comes straight from the ViewModel's single-source + * Renders the "SELECTED FOR SYNC (N)" workspace — a first-class region + * (not just a count) that lists every actionable change the user has + * ticked for push, each as a full row whose checkbox unselects it. Sits + * between the filter and the tree so the working push batch stays + * visible regardless of the active filter, and the same items remain in + * the tree (visually muted via `is-selected`) so context isn't lost. + * The set comes straight from the ViewModel's single-source * `selectedItems` projection (same definition as the Sync button count), - * so the two can never drift. + * so the section and the button can never drift. */ - private renderSelectedSection(container: HTMLElement, selectedItems: readonly SourceControlItem[]): void { + private renderSelectedSection( + container: HTMLElement, + selectedItems: readonly SourceControlItem[], + callbacks: ChangeTreeCallbacks, + ): void { if (selectedItems.length === 0) return; const section = container.createDiv({ cls: 'scv-selected-section' }); - section.createSpan({ cls: 'scv-selected-section-title', text: t('sourceControl.section.selectedForSync') }); - section.createSpan({ cls: 'scv-selected-section-count', text: String(selectedItems.length) }); + const header = section.createDiv({ cls: 'scv-selected-section-header' }); + header.createSpan({ cls: 'scv-selected-section-title', text: t('sourceControl.section.selectedForSync') }); + header.createSpan({ cls: 'scv-selected-section-count', text: String(selectedItems.length) }); + + const list = section.createDiv({ cls: 'scv-selected-section-list' }); + for (const item of selectedItems) { + renderChangeItem(list, item, basename(item.path), callbacks); + } } private renderDetail(root: HTMLElement): void { @@ -293,8 +301,8 @@ export class SourceControlView { private async loadAndRenderDiff(container: HTMLElement, changeId: ChangeId): Promise { if (!this.callbacks.loadDiffContent) return; - const item = this.viewModel.getState('all', this.showSynced).items.find(i => i.id === changeId) - ?? this.viewModel.getState('synced', this.showSynced).items.find(i => i.id === changeId); + const item = this.viewModel.getState('all').items.find(i => i.id === changeId) + ?? this.viewModel.getState('synced', true).items.find(i => i.id === changeId); if (!item) return; const content = await this.callbacks.loadDiffContent(item); @@ -386,4 +394,10 @@ export class SourceControlView { btn.createSpan({ cls: 'scv-mobile-sync-count', text: String(readyCount) }); btn.addEventListener('click', () => { void this.callbacks.onPush(this.selection.getSelectedChangeIds()); }); } +} + +/** Last path segment of a change path, for the Selected section's flat row labels. */ +function basename(path: string): string { + const slash = path.lastIndexOf('/'); + return slash === -1 ? path : path.slice(slash + 1); } \ No newline at end of file diff --git a/styles.css b/styles.css index 1be2af7..b7e539a 100644 --- a/styles.css +++ b/styles.css @@ -135,7 +135,7 @@ align-items: center; gap: 5px; padding: 4px 10px; - border-radius: 20px; + border-radius: var(--radius-s); border: 1px solid transparent; font-size: 0.80em; font-weight: 500; @@ -160,7 +160,7 @@ .scv-filter-count { background: rgba(0, 0, 0, 0.12); - border-radius: 10px; + border-radius: var(--radius-s); padding: 1px 6px; font-size: 0.85em; min-width: 18px; @@ -216,32 +216,54 @@ text-align: center; } -/* ── Selected-for-sync summary ──────────────────────────────── */ +/* ── Selected-for-sync workspace ───────────────────────────── */ .scv-selected-section { + margin: 6px 8px 4px 8px; + padding: 6px 0; + border-radius: var(--radius-m); + background: var(--background-secondary); + border: 1px solid var(--background-modifier-border); + overflow: hidden; +} + +.scv-selected-section-header { display: flex; align-items: center; gap: 6px; - padding: 6px 12px; - margin: 4px 12px 0 12px; - border-radius: 6px; - background: var(--background-modifier-hover); + padding: 2px 10px 6px 10px; color: var(--text-muted); - font-size: 0.78em; + font-size: 0.74em; font-weight: 600; text-transform: uppercase; letter-spacing: 0.02em; } +.scv-selected-section-title { flex-shrink: 0; } + .scv-selected-section-count { background: var(--interactive-accent); color: var(--text-on-accent); - border-radius: 10px; + border-radius: var(--radius-s); padding: 1px 6px; font-size: 0.9em; min-width: 18px; text-align: center; } +.scv-selected-section-list { + display: flex; + flex-direction: column; +} + +.scv-selected-section-list .scv-change-item { + border-left: none; + background: transparent; +} + +.scv-selected-section-list .scv-change-item:hover { + background: var(--background-modifier-hover); +} + /* ── Push button ───────────────────────────────────────────────── */ .scv-push-btn { display: flex; @@ -460,23 +482,28 @@ white-space: nowrap; } -.scv-change-subtitle { - color: var(--text-faint); - font-family: var(--font-interface); - font-size: 0.92em; - font-weight: normal; - white-space: nowrap; - flex-shrink: 0; +.scv-change-item.is-selected { + opacity: 0.55; +} + +.scv-change-item.is-selected .scv-change-name-text { + font-style: italic; } .scv-diff-stat { + display: inline-flex; + align-items: baseline; + gap: 1px; font-family: var(--font-monospace); font-size: 0.72em; - color: var(--text-muted); flex-shrink: 0; white-space: nowrap; } +.scv-diff-stat-add { color: var(--color-green); } +.scv-diff-stat-del { color: var(--color-red); } +.scv-diff-stat-sep { width: 2px; } + .scv-change-rename-from { color: var(--text-faint); text-decoration: line-through; diff --git a/tests/ui/setup-dom.ts b/tests/ui/setup-dom.ts index 514c3b5..7f79309 100644 --- a/tests/ui/setup-dom.ts +++ b/tests/ui/setup-dom.ts @@ -22,13 +22,14 @@ export function setupObsidianDOM(): void { const proto = window.HTMLElement.prototype; if ('createEl' in proto) return; - type DomOpts = { cls?: string; text?: string; type?: string }; + type DomOpts = { cls?: string; text?: string; type?: string; value?: string }; const toOpts = (o?: DomOpts | string): DomOpts => (typeof o === 'string' ? { cls: o } : o ?? {}); function applyOpts(el: Element, o: DomOpts): void { if (o.cls) el.className = o.cls; if (o.text) el.textContent = o.text; if (o.type) (el as HTMLInputElement).type = o.type; + if (o.value !== undefined) (el as HTMLInputElement).value = o.value; } Object.assign(proto, { diff --git a/tests/ui/source-control/ChangeTree.test.ts b/tests/ui/source-control/ChangeTree.test.ts index 4b1ecd5..878d2f1 100644 --- a/tests/ui/source-control/ChangeTree.test.ts +++ b/tests/ui/source-control/ChangeTree.test.ts @@ -56,18 +56,20 @@ describe('renderChangeTree', () => { expect(container.querySelector('.scv-badge')?.textContent).toBe('D'); }); - it('renders a status subtitle next to the change name', () => { + it('does not render an inline status subtitle (the kind label lives on the badge tooltip)', () => { const items = [item({ id: toChangeId('c-1'), path: 'a.md', kind: 'local-modified' })]; renderChangeTree(container, items, new Set(), callbacks); - expect(container.querySelector('.scv-change-subtitle')?.textContent).toBe('Modified'); + expect(container.querySelector('.scv-change-subtitle')).toBeNull(); }); - it('renders the diff stat from the getDiffStat callback when available', () => { + it('renders the diff stat as colored add/del spans from the getDiffStat callback', () => { const items = [item({ id: toChangeId('c-1'), path: 'a.md', kind: 'local-modified' })]; callbacks.getDiffStat = () => ({ additions: 3, deletions: 1 }); renderChangeTree(container, items, new Set(), callbacks); + expect(container.querySelector('.scv-diff-stat-add')?.textContent).toBe('+3'); + expect(container.querySelector('.scv-diff-stat-del')?.textContent).toBe('-1'); expect(container.querySelector('.scv-diff-stat')?.textContent).toBe('+3 -1'); }); @@ -98,6 +100,18 @@ describe('renderChangeTree', () => { expect(checkbox.checked).toBe(true); }); + it('marks a ready-to-push row with the is-selected class', () => { + const items = [ + item({ id: toChangeId('c-1'), path: 'a.md', kind: 'local-only', isReadyToPush: true }), + item({ id: toChangeId('c-2'), path: 'b.md', kind: 'local-only', isReadyToPush: false }), + ]; + renderChangeTree(container, items, new Set(), callbacks); + + const rows = container.querySelectorAll('.scv-change-item'); + expect(rows[0]?.classList.contains('is-selected')).toBe(true); + expect(rows[1]?.classList.contains('is-selected')).toBe(false); + }); + it('calls onToggleSelect with the ChangeId when the checkbox changes', () => { const items = [item({ id: toChangeId('c-1'), path: 'a.md', kind: 'local-only' })]; renderChangeTree(container, items, new Set(), callbacks); diff --git a/tests/ui/source-control/FilterMenu.test.ts b/tests/ui/source-control/FilterMenu.test.ts index 5b90a91..552fdbc 100644 --- a/tests/ui/source-control/FilterMenu.test.ts +++ b/tests/ui/source-control/FilterMenu.test.ts @@ -11,73 +11,63 @@ const zeroCounts: Record = { describe('renderFilterMenu', () => { let container: HTMLElement; - let callbacks: { onFilterChange: (f: SourceControlFilter) => void; onToggleShowSynced: (s: boolean) => void }; + let callbacks: { onFilterChange: (f: SourceControlFilter) => void }; beforeEach(() => { container = createContainer(); - callbacks = { onFilterChange: vi.fn(), onToggleShowSynced: vi.fn() }; + callbacks = { onFilterChange: vi.fn() }; }); - it('renders the four action chips (All/Local/Remote/Conflict, no synced chip) when showSynced is false', () => { - renderFilterMenu(container, 'all', zeroCounts, false, callbacks); + it('renders the four action chips (All/Local/Remote/Conflict) and never a synced chip', () => { + renderFilterMenu(container, 'all', zeroCounts, callbacks); const filters = Array.from(container.querySelectorAll('.scv-filter-option')).map(el => el.getAttribute('data-filter')); expect(filters).toEqual(['all', 'changes', 'remote-changes', 'conflicts']); + expect(container.querySelector('.scv-filter-option[data-filter="synced"]')).toBeNull(); }); it('labels the chips with the domain-relabeled display names (Local/Remote/Conflict)', () => { - renderFilterMenu(container, 'all', zeroCounts, false, callbacks); + renderFilterMenu(container, 'all', zeroCounts, callbacks); const labels = Array.from(container.querySelectorAll('.scv-filter-option .scv-filter-label')).map(el => el.textContent); // Domain values stay as data-filter; only the visible labels change. expect(labels).toEqual(['All', 'Local', 'Remote', 'Conflict']); }); - it('appends the synced chip when showSynced is true', () => { - renderFilterMenu(container, 'all', { ...zeroCounts, synced: 7 }, true, callbacks); + it('does not render a Show synced toggle', () => { + renderFilterMenu(container, 'all', zeroCounts, callbacks); - const filters = Array.from(container.querySelectorAll('.scv-filter-option')).map(el => el.getAttribute('data-filter')); - expect(filters).toEqual(['all', 'changes', 'remote-changes', 'conflicts', 'synced']); - const syncedOption = container.querySelector('.scv-filter-option[data-filter="synced"]'); - expect(syncedOption?.querySelector('.scv-filter-count')?.textContent).toBe('7'); + expect(container.querySelector('.scv-filter-show-synced-checkbox')).toBeNull(); }); it('marks the current filter chip as active', () => { - renderFilterMenu(container, 'conflicts', zeroCounts, false, callbacks); + renderFilterMenu(container, 'conflicts', zeroCounts, callbacks); const active = container.querySelector('.scv-filter-option.is-active'); expect(active?.getAttribute('data-filter')).toBe('conflicts'); }); it('shows the per-filter count from the ViewModel', () => { - renderFilterMenu(container, 'all', { ...zeroCounts, conflicts: 3 }, false, callbacks); + renderFilterMenu(container, 'all', { ...zeroCounts, conflicts: 3 }, callbacks); const conflictsOption = container.querySelector('.scv-filter-option[data-filter="conflicts"]'); expect(conflictsOption?.querySelector('.scv-filter-count')?.textContent).toBe('3'); }); it('calls onFilterChange with the clicked filter value', () => { - renderFilterMenu(container, 'all', zeroCounts, false, callbacks); + renderFilterMenu(container, 'all', zeroCounts, callbacks); (container.querySelector('.scv-filter-option[data-filter="remote-changes"]') as HTMLButtonElement).click(); expect(callbacks.onFilterChange).toHaveBeenCalledWith('remote-changes'); }); - it('renders the Show synced toggle reflecting the showSynced state', () => { - renderFilterMenu(container, 'all', zeroCounts, false, callbacks); - const checkbox = container.querySelector('.scv-filter-show-synced-checkbox') as HTMLInputElement; - expect(checkbox).not.toBeNull(); - expect(checkbox.checked).toBe(false); - }); - - it('calls onToggleShowSynced when the Show synced checkbox changes', () => { - renderFilterMenu(container, 'all', zeroCounts, false, callbacks); - - const checkbox = container.querySelector('.scv-filter-show-synced-checkbox') as HTMLInputElement; - checkbox.checked = true; - checkbox.dispatchEvent(new Event('change')); + it('renders a mobile dropdown (no chips) when isMobile is true', () => { + renderFilterMenu(container, 'all', { ...zeroCounts, conflicts: 3 }, callbacks, { isMobile: true }); - expect(callbacks.onToggleShowSynced).toHaveBeenCalledWith(true); + expect(container.querySelector('.scv-filter-dropdown')).not.toBeNull(); + expect(container.querySelector('.scv-filter-option')).toBeNull(); + const options = Array.from(container.querySelectorAll('.scv-filter-dropdown option')).map(o => (o as HTMLOptionElement).value); + expect(options).toEqual(['all', 'changes', 'remote-changes', 'conflicts']); }); }); \ 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 3d183de..a6cf1f7 100644 --- a/tests/ui/source-control/SourceControlView.test.ts +++ b/tests/ui/source-control/SourceControlView.test.ts @@ -94,8 +94,8 @@ describe('SourceControlView', () => { }); }); - describe('show synced toggle', () => { - it('hides the synced chip and synced rows by default', () => { + describe('synced surfacing removed', () => { + it('never renders a synced chip or a Show synced toggle', () => { const { view } = buildView([ { id: toChangeId('c-1'), path: 'a.md', kind: 'local-only' }, { id: toChangeId('c-2'), path: 'd.md', kind: 'synced' }, @@ -103,44 +103,18 @@ describe('SourceControlView', () => { view.render(container); expect(container.querySelector('.scv-filter-option[data-filter="synced"]')).toBeNull(); - expect(view.getShowSynced()).toBe(false); + expect(container.querySelector('.scv-filter-show-synced-checkbox')).toBeNull(); }); - it('reveals the synced chip and renders synced rows when toggled on', () => { + it('excludes synced rows from every filter view', () => { const { view } = buildView([ { id: toChangeId('c-1'), path: 'a.md', kind: 'local-only' }, { id: toChangeId('c-2'), path: 'd.md', kind: 'synced' }, ]); view.render(container); - const checkbox = container.querySelector('.scv-filter-show-synced-checkbox') as HTMLInputElement; - checkbox.checked = true; - checkbox.dispatchEvent(new Event('change')); - - expect(view.getShowSynced()).toBe(true); - const syncedChip = container.querySelector('.scv-filter-option[data-filter="synced"]'); - expect(syncedChip).not.toBeNull(); - expect(syncedChip?.querySelector('.scv-filter-count')?.textContent).toBe('1'); - }); - - it('falls back to All when synced is hidden while viewing the synced filter', () => { - const { view } = buildView([ - { id: toChangeId('c-1'), path: 'a.md', kind: 'local-only' }, - { id: toChangeId('c-2'), path: 'd.md', kind: 'synced' }, - ]); - view.render(container); - - // Opt in and switch to the synced filter. - const toggle = container.querySelector('.scv-filter-show-synced-checkbox') as HTMLInputElement; - toggle.checked = true; - toggle.dispatchEvent(new Event('change')); - (container.querySelector('.scv-filter-option[data-filter="synced"]') as HTMLButtonElement).click(); - expect(view.getFilter()).toBe('synced'); - - // Opt back out: filter snaps back to All. - toggle.checked = false; - toggle.dispatchEvent(new Event('change')); - expect(view.getFilter()).toBe('all'); + expect(container.querySelectorAll('.scv-change-item')).toHaveLength(1); + expect(container.querySelector('.scv-kind-synced')).toBeNull(); }); }); @@ -187,6 +161,28 @@ describe('SourceControlView', () => { expect(section?.querySelector('.scv-selected-section-count')?.textContent).toBe('1'); }); + it('lists the selected change as a real row inside the Selected section, unselecting on checkbox clear', () => { + const { view, selection } = buildView([ + { id: toChangeId('c-1'), path: 'notes/a.md', kind: 'local-only' }, + ]); + selection.includeForPush(toChangeId('c-1')); + view.render(container); + + const section = container.querySelector('.scv-selected-section') as HTMLElement; + const row = section.querySelector('.scv-change-item') as HTMLElement; + expect(row?.getAttribute('data-change-id')).toBe('c-1'); + expect(row?.querySelector('.scv-change-name-text')?.textContent).toBe('a.md'); + expect(row?.classList.contains('is-selected')).toBe(true); + + const checkbox = row.querySelector('.scv-change-select') as HTMLInputElement; + expect(checkbox.checked).toBe(true); + checkbox.checked = false; + checkbox.dispatchEvent(new Event('change')); + + expect(selection.isIncluded(toChangeId('c-1'))).toBe(false); + expect(container.querySelector('.scv-selected-section')).toBeNull(); + }); + it('excludes synced changes from the SELECTED FOR SYNC count even when selected', () => { const { view, selection } = buildView([ { id: toChangeId('c-1'), path: 'a.md', kind: 'local-only' }, From a9d3e98956054555b419edbc0da1e4898d4ab7a2 Mon Sep 17 00:00:00 2001 From: Tianyao Date: Sun, 23 Aug 2026 00:31:24 +0800 Subject: [PATCH 07/17] fix(source-control): make whole view scroll, add clear-selection, click-to-collapse folders - Move the Selected section into the scroll body so the whole lower region (selected rows + filter header + tree) scrolls as one. Previously the selected section sat outside the scroll container with no height cap, so a tall selection blew out the layout under .scv-root{overflow:hidden} and nothing scrolled. - Add a 'Clear' button to the Selected section header that deselects every selected change in one shot, instead of unchecking rows one by one. - Make the entire folder row toggle collapse (clicking the folder name or chevron expands/collapses); only the select-all checkbox keeps its own action. The chevron stops propagation so it doesn't double-fire. --- src/i18n/locales/en.ts | 2 ++ src/i18n/locales/zh-cn.ts | 2 ++ src/i18n/locales/zh-tw.ts | 2 ++ src/ui/source-control/ChangeTree.ts | 15 +++++++++++-- src/ui/source-control/SourceControlView.ts | 22 +++++++++++++++++-- styles.css | 16 ++++++++++++++ .../source-control/SourceControlView.test.ts | 21 ++++++++++++++++++ 7 files changed, 76 insertions(+), 4 deletions(-) diff --git a/src/i18n/locales/en.ts b/src/i18n/locales/en.ts index 7038db6..dc82447 100644 --- a/src/i18n/locales/en.ts +++ b/src/i18n/locales/en.ts @@ -261,6 +261,8 @@ const en = { 'sourceControl.section.all': 'ALL', 'sourceControl.section.readyToPush': 'READY TO PUSH', 'sourceControl.section.selectedForSync': 'SELECTED FOR SYNC', + 'sourceControl.section.clearSelection': 'Clear', + 'sourceControl.section.clearSelection.tooltip': 'Deselect all changes', 'sourceControl.section.changes': 'CHANGES', 'sourceControl.section.remoteChanges': 'REMOTE CHANGES', 'sourceControl.section.conflicts': 'CONFLICTS', diff --git a/src/i18n/locales/zh-cn.ts b/src/i18n/locales/zh-cn.ts index f97c41a..d2fa864 100644 --- a/src/i18n/locales/zh-cn.ts +++ b/src/i18n/locales/zh-cn.ts @@ -263,6 +263,8 @@ const zhCn: Partial> = { 'sourceControl.section.all': '全部', 'sourceControl.section.readyToPush': '待推送', 'sourceControl.section.selectedForSync': '已选同步', + 'sourceControl.section.clearSelection': '清除', + 'sourceControl.section.clearSelection.tooltip': '取消全部选择', 'sourceControl.section.changes': '更改', 'sourceControl.section.remoteChanges': '远程更改', 'sourceControl.section.conflicts': '冲突', diff --git a/src/i18n/locales/zh-tw.ts b/src/i18n/locales/zh-tw.ts index d3dea73..f62d1eb 100644 --- a/src/i18n/locales/zh-tw.ts +++ b/src/i18n/locales/zh-tw.ts @@ -263,6 +263,8 @@ const zhTw: Partial> = { 'sourceControl.section.all': '全部', 'sourceControl.section.readyToPush': '待推送', 'sourceControl.section.selectedForSync': '已選同步', + 'sourceControl.section.clearSelection': '清除', + 'sourceControl.section.clearSelection.tooltip': '取消全部選取', 'sourceControl.section.changes': '變更', 'sourceControl.section.remoteChanges': '遠端變更', 'sourceControl.section.conflicts': '衝突', diff --git a/src/ui/source-control/ChangeTree.ts b/src/ui/source-control/ChangeTree.ts index 21884f2..1dce928 100644 --- a/src/ui/source-control/ChangeTree.ts +++ b/src/ui/source-control/ChangeTree.ts @@ -66,6 +66,8 @@ function renderFolder( const collapsed = collapsedFolders.has(folder.path); const folderEl = container.createDiv({ cls: 'scv-tree-folder' }); const row = folderEl.createDiv({ cls: 'scv-tree-folder-row' }); + row.setAttr('role', 'button'); + row.setAttr('aria-expanded', String(!collapsed)); const fileIds = collectFileIds(folder); const selectedCount = fileIds.filter(id => byId.get(id)?.isReadyToPush).length; @@ -77,12 +79,21 @@ function renderFolder( checkbox.addEventListener('change', () => callbacks.onToggleFolderSelect(fileIds, checkbox.checked)); const toggle = row.createEl('button', { cls: 'scv-tree-folder-toggle' }); - toggle.setAttr('aria-expanded', String(!collapsed)); + toggle.setAttr('aria-hidden', 'true'); toggle.setText(collapsed ? '▶' : '▼'); - toggle.addEventListener('click', () => callbacks.onToggleFolder(folder.path)); + // The whole row toggles; the chevron is just a visual affordance, so + // stop its click from double-firing the row handler. + toggle.addEventListener('click', (evt) => { evt.stopPropagation(); callbacks.onToggleFolder(folder.path); }); row.createSpan({ cls: 'scv-tree-folder-name', text: folder.name }); + // Clicking anywhere on the row (name, padding) toggles collapse — except + // the select-all checkbox, which keeps its own action. + row.addEventListener('click', (evt) => { + if (evt.target === checkbox) return; + callbacks.onToggleFolder(folder.path); + }); + if (!collapsed) { const childrenEl = folderEl.createDiv({ cls: 'scv-tree-children' }); renderNodes(childrenEl, folder.children, byId, collapsedFolders, callbacks, options); diff --git a/src/ui/source-control/SourceControlView.ts b/src/ui/source-control/SourceControlView.ts index 1e192ac..d69d770 100644 --- a/src/ui/source-control/SourceControlView.ts +++ b/src/ui/source-control/SourceControlView.ts @@ -176,9 +176,13 @@ export class SourceControlView { const query = this.searchQuery.trim().toLowerCase(); const items = query ? state.items.filter(item => item.path.toLowerCase().includes(query)) : state.items; - this.renderSelectedSection(container, state.selectedItems, treeCallbacks); - + // The scroll container: selected section + active-filter header + tree + // all live here so the whole lower region scrolls as one. Pinned + // controls (header, search, filter) stay outside so they don't scroll + // away; a tall Selected section therefore scrolls with the tree + // instead of blowing out the layout under `.scv-root { overflow: hidden }`. const body = container.createDiv({ cls: 'scv-body' }); + this.renderSelectedSection(body, state.selectedItems, treeCallbacks); this.renderActiveFilterHeader(body, state.filter, items.length); if (items.length === 0) { body.createDiv({ cls: 'scv-empty', text: t('sourceControl.empty') }); @@ -272,12 +276,26 @@ export class SourceControlView { header.createSpan({ cls: 'scv-selected-section-title', text: t('sourceControl.section.selectedForSync') }); header.createSpan({ cls: 'scv-selected-section-count', text: String(selectedItems.length) }); + const clearBtn = header.createEl('button', { + cls: 'scv-selected-section-clear', + attr: { type: 'button' }, + }); + clearBtn.createSpan({ cls: 'scv-selected-section-clear-label', text: t('sourceControl.section.clearSelection') }); + setTooltip(clearBtn, t('sourceControl.section.clearSelection.tooltip')); + clearBtn.addEventListener('click', () => this.clearSelection(selectedItems)); + const list = section.createDiv({ cls: 'scv-selected-section-list' }); for (const item of selectedItems) { renderChangeItem(list, item, basename(item.path), callbacks); } } + /** Unselects every change currently in the Selected section in one shot. */ + private clearSelection(items: readonly SourceControlItem[]): void { + for (const item of items) this.selection.excludeFromPush(item.id); + this.rerender(); + } + private renderDetail(root: HTMLElement): void { const detail = root.createDiv({ cls: 'scv-detail' }); const bar = detail.createDiv({ cls: 'scv-detail-bar' }); diff --git a/styles.css b/styles.css index b7e539a..1436a36 100644 --- a/styles.css +++ b/styles.css @@ -240,6 +240,22 @@ .scv-selected-section-title { flex-shrink: 0; } +.scv-selected-section-clear { + margin-left: auto; + border: none; + background: transparent; + color: var(--text-muted); + font-size: 0.9em; + padding: 2px 6px; + border-radius: var(--radius-s); + cursor: pointer; +} + +.scv-selected-section-clear:hover { + color: var(--text-normal); + background: var(--background-modifier-hover); +} + .scv-selected-section-count { background: var(--interactive-accent); color: var(--text-on-accent); diff --git a/tests/ui/source-control/SourceControlView.test.ts b/tests/ui/source-control/SourceControlView.test.ts index a6cf1f7..77fccbb 100644 --- a/tests/ui/source-control/SourceControlView.test.ts +++ b/tests/ui/source-control/SourceControlView.test.ts @@ -183,6 +183,27 @@ describe('SourceControlView', () => { expect(container.querySelector('.scv-selected-section')).toBeNull(); }); + it('clears all selected changes at once via the Clear button in the section header', () => { + const { view, selection } = buildView([ + { id: toChangeId('c-1'), path: 'a.md', kind: 'local-only' }, + { id: toChangeId('c-2'), path: 'b.md', kind: 'local-modified' }, + { id: toChangeId('c-3'), path: 'c.md', kind: 'remote-only' }, + ]); + selection.includeForPush(toChangeId('c-1')); + selection.includeForPush(toChangeId('c-2')); + selection.includeForPush(toChangeId('c-3')); + view.render(container); + + const clearBtn = container.querySelector('.scv-selected-section-clear') as HTMLButtonElement; + expect(clearBtn).not.toBeNull(); + clearBtn.click(); + + expect(selection.isIncluded(toChangeId('c-1'))).toBe(false); + expect(selection.isIncluded(toChangeId('c-2'))).toBe(false); + expect(selection.isIncluded(toChangeId('c-3'))).toBe(false); + expect(container.querySelector('.scv-selected-section')).toBeNull(); + }); + it('excludes synced changes from the SELECTED FOR SYNC count even when selected', () => { const { view, selection } = buildView([ { id: toChangeId('c-1'), path: 'a.md', kind: 'local-only' }, From 7538e0a8e4cf273e2d3480ad8f9280f540038838 Mon Sep 17 00:00:00 2001 From: Tianyao Date: Sun, 23 Aug 2026 00:38:00 +0800 Subject: [PATCH 08/17] refactor(source-control): Selected section becomes a read-only action queue - Selected rows now render as queue items (badge + name + diff-stat, NO checkbox) instead of full tree rows, so the section reads as an action preview of the working push batch rather than a second active copy of the tree. Selection still happens in the tree below via checkboxes. - Eager-load diff stats for every selected change (any kind) so the queue previews +/- next to each row; tree two-sided rows stay lazy on open. - Drop the is-selected muting (opacity/italic) from tree rows: a checked checkbox is the only selection signal in the browser, matching the queue/ browser role split. - Export renderDiffStat and add renderSelectedQueueItem (reuses presentChange + the colored diff-stat spans). --- src/ui/source-control/ChangeItem.ts | 38 ++++++++++++++++- src/ui/source-control/SourceControlView.ts | 41 +++++++++++++----- styles.css | 37 ++++++++++------ .../source-control/SourceControlView.test.ts | 42 +++++++++++++++---- 4 files changed, 126 insertions(+), 32 deletions(-) diff --git a/src/ui/source-control/ChangeItem.ts b/src/ui/source-control/ChangeItem.ts index a8cbafa..3f46675 100644 --- a/src/ui/source-control/ChangeItem.ts +++ b/src/ui/source-control/ChangeItem.ts @@ -67,7 +67,7 @@ export function renderChangeItem( * deletions) so the magnitude and direction read at a glance. Nothing is * rendered when the stat is unavailable or zero on both sides. */ -function renderDiffStat(row: HTMLElement, stat: ChangeStat | undefined): void { +export function renderDiffStat(row: HTMLElement, stat: ChangeStat | undefined): void { if (!stat) return; const hasAdd = stat.additions > 0; const hasDel = stat.deletions > 0; @@ -76,4 +76,40 @@ function renderDiffStat(row: HTMLElement, stat: ChangeStat | undefined): void { if (hasAdd) wrap.createSpan({ cls: 'scv-diff-stat-add', text: `+${stat.additions}` }); if (hasAdd && hasDel) wrap.createSpan({ cls: 'scv-diff-stat-sep', text: ' ' }); if (hasDel) wrap.createSpan({ cls: 'scv-diff-stat-del', text: `-${stat.deletions}` }); +} + +/** + * Renders a compact queue row for the "SELECTED FOR SYNC" section: badge + + * name (with rename arrow for moves) + diff-stat, with NO selection checkbox + * and NO operation indicator. The Selected section is an action preview of + * the working push batch, not a second copy of the tree — selection happens + * in the tree below, so the queue stays read-only (clicking opens the diff). + */ +export function renderSelectedQueueItem( + container: HTMLElement, + item: SourceControlItem, + displayName: string, + callbacks: ChangeItemCallbacks, +): HTMLElement { + const view = presentChange(item, displayName); + + const row = container.createDiv({ cls: `scv-queue-item scv-kind-${item.kind}` }); + row.setAttr('data-change-id', item.id); + if (view.tooltip) row.setAttr('title', view.tooltip); + + const badgeEl = row.createSpan({ cls: `scv-badge scv-badge-${view.badge.cls}`, text: view.badge.letter }); + setTooltip(badgeEl, view.subtitle); + + const label = row.createDiv({ cls: 'scv-queue-name' }); + if (view.renameFrom) { + label.createSpan({ cls: 'scv-change-rename-from', text: view.renameFrom }); + setIcon(label.createSpan({ cls: 'scv-change-rename-arrow' }), ICONS.moved); + } + label.createSpan({ cls: 'scv-queue-name-text', text: view.displayName }); + + renderDiffStat(row, callbacks.getDiffStat?.(item.id)); + + row.addEventListener('click', () => callbacks.onOpenDiff(item)); + + return row; } \ No newline at end of file diff --git a/src/ui/source-control/SourceControlView.ts b/src/ui/source-control/SourceControlView.ts index d69d770..4959960 100644 --- a/src/ui/source-control/SourceControlView.ts +++ b/src/ui/source-control/SourceControlView.ts @@ -9,7 +9,7 @@ import { ICONS } from '../components/icons'; import { renderDiffLayoutToggle, type DiffLayout } from '../components/DiffLayoutToggle'; import { renderDiffPanel } from '../components/DiffPanel'; import { renderChangeTree, type ChangeTreeCallbacks } from './ChangeTree'; -import { renderChangeItem } from './ChangeItem'; +import { renderSelectedQueueItem } from './ChangeItem'; import { renderFilterMenu } from './FilterMenu'; import { renderSourceControlHeader, type SourceControlWorkspaceInfo } from './SourceControlHeader'; @@ -192,6 +192,7 @@ export class SourceControlView { renderChangeTree(body, items, this.collapsedFolders, treeCallbacks, isMobile ? MOBILE_TREE_OPTIONS : TREE_OPTIONS); this.eagerLoadLocalStats(items); + this.eagerLoadSelectedStats(state.selectedItems); if (isMobile) this.renderMobileSyncBar(container, state.counts['ready-to-push']); } @@ -255,15 +256,15 @@ export class SourceControlView { } /** - * Renders the "SELECTED FOR SYNC (N)" workspace — a first-class region - * (not just a count) that lists every actionable change the user has - * ticked for push, each as a full row whose checkbox unselects it. Sits - * between the filter and the tree so the working push batch stays - * visible regardless of the active filter, and the same items remain in - * the tree (visually muted via `is-selected`) so context isn't lost. - * The set comes straight from the ViewModel's single-source - * `selectedItems` projection (same definition as the Sync button count), - * so the section and the button can never drift. + * Renders the "SELECTED FOR SYNC (N)" workspace — a read-only action + * preview of the working push batch. Each queued change is a compact row + * (badge + name + diff-stat, NO checkbox): this is not a second copy of + * the tree but the queue the Sync button will act on. Selection itself + * happens in the tree below, which keeps the same rows visible but + * muted via `is-selected` so context isn't lost. The set comes straight + * from the ViewModel's single-source `selectedItems` projection (same + * definition as the Sync button count), so the section and the button + * can never drift. */ private renderSelectedSection( container: HTMLElement, @@ -286,7 +287,7 @@ export class SourceControlView { const list = section.createDiv({ cls: 'scv-selected-section-list' }); for (const item of selectedItems) { - renderChangeItem(list, item, basename(item.path), callbacks); + renderSelectedQueueItem(list, item, basename(item.path), callbacks); } } @@ -388,6 +389,24 @@ export class SourceControlView { })).then(() => this.rerender()); } + /** + * Eagerly resolves +/- stats for every change in the Selected section so + * the action queue previews `+3 -1` next to each row. Unlike + * {@link eagerLoadLocalStats} this covers all kinds (two-sided changes may + * involve a remote fetch), but the selected set is the user's working + * push batch — small and worth the round-trip. Null results are cached so + * an unavailable stat isn't retried on every rerender. + */ + private eagerLoadSelectedStats(selectedItems: readonly SourceControlItem[]): void { + if (!this.callbacks.loadDiffStat) return; + const pending = selectedItems.filter(item => !this.diffStatCache.has(item.id)); + if (pending.length === 0) return; + void Promise.all(pending.map(async item => { + const stat = await this.callbacks.loadDiffStat!(item); + this.diffStatCache.set(item.id, stat ?? null); + })).then(() => this.rerender()); + } + /** * Lazily resolves the +/- stat for a single two-sided change on open, * caching it so subsequent renders show the stat without a refetch. diff --git a/styles.css b/styles.css index 1436a36..5e37b12 100644 --- a/styles.css +++ b/styles.css @@ -271,15 +271,36 @@ flex-direction: column; } -.scv-selected-section-list .scv-change-item { - border-left: none; - background: transparent; +.scv-queue-item { + display: flex; + align-items: center; + gap: 5px; + padding: 4px 10px; + font-family: var(--font-monospace); + font-size: 0.80em; + color: var(--text-normal); + cursor: pointer; } -.scv-selected-section-list .scv-change-item:hover { +.scv-queue-item:hover { background: var(--background-modifier-hover); } +.scv-queue-name { + flex: 1; + min-width: 0; + display: flex; + align-items: center; + gap: 5px; + overflow: hidden; +} + +.scv-queue-name-text { + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} + /* ── Push button ───────────────────────────────────────────────── */ .scv-push-btn { display: flex; @@ -498,14 +519,6 @@ white-space: nowrap; } -.scv-change-item.is-selected { - opacity: 0.55; -} - -.scv-change-item.is-selected .scv-change-name-text { - font-style: italic; -} - .scv-diff-stat { display: inline-flex; align-items: baseline; diff --git a/tests/ui/source-control/SourceControlView.test.ts b/tests/ui/source-control/SourceControlView.test.ts index 77fccbb..cc8e008 100644 --- a/tests/ui/source-control/SourceControlView.test.ts +++ b/tests/ui/source-control/SourceControlView.test.ts @@ -161,7 +161,7 @@ describe('SourceControlView', () => { expect(section?.querySelector('.scv-selected-section-count')?.textContent).toBe('1'); }); - it('lists the selected change as a real row inside the Selected section, unselecting on checkbox clear', () => { + it('lists the selected change as a read-only queue row (badge + name, no checkbox) inside the Selected section', () => { const { view, selection } = buildView([ { id: toChangeId('c-1'), path: 'notes/a.md', kind: 'local-only' }, ]); @@ -169,15 +169,26 @@ describe('SourceControlView', () => { view.render(container); const section = container.querySelector('.scv-selected-section') as HTMLElement; - const row = section.querySelector('.scv-change-item') as HTMLElement; + const row = section.querySelector('.scv-queue-item') as HTMLElement; expect(row?.getAttribute('data-change-id')).toBe('c-1'); - expect(row?.querySelector('.scv-change-name-text')?.textContent).toBe('a.md'); - expect(row?.classList.contains('is-selected')).toBe(true); + expect(row?.querySelector('.scv-queue-name-text')?.textContent).toBe('a.md'); + expect(row?.querySelector('.scv-badge')?.textContent).toBe('A'); + // The queue is an action preview, not a second copy of the tree: + // no selection checkbox here — selection happens in the tree below. + expect(row?.querySelector('.scv-change-select')).toBeNull(); + }); - const checkbox = row.querySelector('.scv-change-select') as HTMLInputElement; - expect(checkbox.checked).toBe(true); - checkbox.checked = false; - checkbox.dispatchEvent(new Event('change')); + it('unselects via the tree row checkbox, removing the change from the Selected section', () => { + const { view, selection } = buildView([ + { id: toChangeId('c-1'), path: 'notes/a.md', kind: 'local-only' }, + ]); + selection.includeForPush(toChangeId('c-1')); + view.render(container); + + const treeCheckbox = container.querySelector('.scv-body .scv-change-item .scv-change-select') as HTMLInputElement; + expect(treeCheckbox.checked).toBe(true); + treeCheckbox.checked = false; + treeCheckbox.dispatchEvent(new Event('change')); expect(selection.isIncluded(toChangeId('c-1'))).toBe(false); expect(container.querySelector('.scv-selected-section')).toBeNull(); @@ -517,6 +528,21 @@ describe('SourceControlView', () => { expect(loadDiffStat).toHaveBeenCalledTimes(1); expect(container.querySelector('.scv-diff-stat')?.textContent).toBe('+1 -4'); }); + + it('eager-loads stats for selected changes of any kind so the Selected queue previews them', async () => { + const loadDiffStat = vi.fn().mockResolvedValue({ additions: 2, deletions: 1 }); + const { view, selection } = buildView( + [{ id: toChangeId('c-1'), path: 'a.md', kind: 'local-modified' }], + { loadDiffStat }, + ); + selection.includeForPush(toChangeId('c-1')); + view.render(container); + await flush(); + + // A two-sided change in the queue is eager-loaded (unlike tree-only rows). + expect(loadDiffStat).toHaveBeenCalledWith(expect.objectContaining({ kind: 'local-modified' })); + expect(container.querySelector('.scv-selected-section .scv-queue-item .scv-diff-stat')?.textContent).toBe('+2 -1'); + }); }); describe('mobile layout', () => { From 639840a29bae86c930324914537812399be78667 Mon Sep 17 00:00:00 2001 From: Tianyao Date: Sun, 23 Aug 2026 00:52:56 +0800 Subject: [PATCH 09/17] refactor(source-control): converge UI to sync-intent workflow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #135 review flagged that the view mixed VS Code staged / Git status / sync-queue concepts. Converged to a single sync-intent workflow, view-layer only (no domain file touched; the domain-untouched invariant still holds). Filter chips redesigned to All / Needs Sync / Remote / Conflict / Synced via a UI chip model mapping (domain filter, showSynced): - Needs Sync (domain all, showSynced=false) = actionable set, default — keeps a quiet workspace quiet. - All (domain all, showSynced=true) composes actionable + synced by concatenating getState('all',false) + getState('synced',true) in the view; the domain all filter still returns actionable-only, so no domain change was needed. - Synced re-surfaces (domain synced, showSynced=true); Local dropped. SELECTED FOR SYNC renamed to SYNC QUEUE (the queue stays a compact read-only action preview). Badge tooltips read Added locally / Modified locally. Mobile sync bar becomes N files selected + Sync. Verification: eslint 0 errors; build clean (incl. Obsidian 1.11.0 compat); vitest 632/633 (1 unrelated pre-existing ci-workflow.test.ts failure from an uncommitted ci.yml). Domain diff shows only RefreshState.ts + SourceControlViewModel.ts. --- progress.md | 7 +- session-handoff.md | 72 ++++------- src/i18n/locales/en.ts | 9 +- src/i18n/locales/zh-cn.ts | 9 +- src/i18n/locales/zh-tw.ts | 9 +- src/ui/source-control/ChangeItem.ts | 2 +- src/ui/source-control/FilterMenu.ts | 118 ++++++++++-------- src/ui/source-control/SourceControlView.ts | 92 ++++++++------ styles.css | 21 ++-- .../source-control/ChangePresentation.test.ts | 4 +- tests/ui/source-control/FilterMenu.test.ts | 60 +++++---- .../source-control/SourceControlView.test.ts | 64 +++++++--- 12 files changed, 257 insertions(+), 210 deletions(-) diff --git a/progress.md b/progress.md index 8cc915e..917eec5 100644 --- a/progress.md +++ b/progress.md @@ -4,8 +4,8 @@ Completed work is archived in [archive/](./archive/), one file per calendar mont ## Current State -**Last Updated:** 2026-08-22 -**Active Feature:** sync-status-workflow-ui plan (`.kilo/plans/1787412338771-sync-status-workflow-ui.md`) — code complete on `feat/sync-status-workflow-ui` (4 commits, all green); manual Obsidian desktop/mobile UI verification remains. Prior active feature feat-026 / issue #105 (sync architecture refactor on `refactor/sync-domain-pipeline`) still has manual Obsidian move smoke tests outstanding. +**Last Updated:** 2026-08-23 +**Active Feature:** sync-status-workflow-ui plan (`.kilo/plans/1787412338771-sync-status-workflow-ui.md`) on `feat/sync-status-workflow-ui` — core 4 commits + 2 follow-up UX commits + 1 queue-refactor commit + 1 UX-convergence commit (8 total); automated checks green; manual Obsidian desktop/mobile UI verification remains. The convergence pass (renamed `SELECTED FOR SYNC`→`SYNC QUEUE`, 5-chip filter `All/Needs Sync/Remote/Conflict/Synced` with "All" composing actionable+synced view-side, default `Needs Sync`, `Added/Modified locally` badge tooltips, mobile bar `N files selected`+`Sync`) stayed entirely in the view layer — domain-untouched invariant still holds. Prior active feature feat-026 / issue #105 (sync architecture refactor on `refactor/sync-domain-pipeline`) still has manual Obsidian move smoke tests outstanding. **Parallel Work:** PR #87 (4x Dependabot security alerts via npm overrides) and Issue #57 (live-credential smoke test). ## Outstanding Items @@ -14,10 +14,11 @@ Completed work is archived in [archive/](./archive/), one file per calendar mont 1. **feat-025 manual verification** — Tree view code is complete and all automated checks pass; manual Obsidian verification in a real vault remains for user to confirm functionality (tree hierarchy, folder expand/collapse, checkboxes, Show synced toggle). 2. **PR #87** — Dependabot security patches via npm overrides; awaiting review/merge. 3. **Issue #57** — Live-credential smoke test; pre-existing, relevant before pushing major sync work. -4. **sync-status-workflow-ui manual verification** — All 4 commits landed and automated checks pass (eslint/build/vitest green); manual Obsidian desktop + mobile verification of the runtime UI (refresh button states, selected-for-sync section, per-row subtitles/badges incl. `remote-only`→`D`, diff-stat spans, mobile filter dropdown + bottom sync bar) remains before opening a PR. +4. **sync-status-workflow-ui manual verification** — 8 commits landed, automated checks pass (eslint/build/vitest green; 632/633, the 1 failure is the unrelated pre-existing `ci-workflow.test.ts` vs an uncommitted `ci.yml`); manual Obsidian desktop + mobile verification of the runtime UI (refresh button states, `SYNC QUEUE` section, 5-chip filter incl. `Needs Sync` default and `Synced`, badge tooltips `Modified locally`/`Added locally`, diff-stat spans, mobile `N files selected`+`Sync` bar) remains before opening a PR. ## Latest Evidence +- [x] sync-status-workflow-ui UX convergence (2026-08-23), branch `feat/sync-status-workflow-ui`, commit (pending): converged the PR's mixed workflow concepts per review feedback, view-layer only. (1) Renamed `SELECTED FOR SYNC`→`SYNC QUEUE` (i18n en/zh-cn/zh-tw); the queue stays a compact read-only action preview (badge+name+diff-stat, no checkbox). (2) Filter chips redesigned to `All / Needs Sync / Remote / Conflict / Synced` via a UI chip model mapping `(domain filter, showSynced)` — `Needs Sync` (domain `all`, actionable) is the default; `All` composes actionable+synced by concatenating `getState('all',false)` + `getState('synced',true)` in the view (the domain `all` filter still returns actionable-only, so no domain change); `Synced` re-surfaces (domain `synced`, showSynced=true); `Local` dropped. (3) Badge tooltip wording `Added`→`Added locally`, `Modified`→`Modified locally`. (4) Mobile sync bar → `N files selected` + `Sync` button (was a single full-width `SELECTED FOR SYNC (N)` button). Domain-untouched invariant verified: `git diff claude/source-control-foundation -- src/logic/source-control/` shows only `RefreshState.ts` (new) + `SourceControlViewModel.ts` (edited). Verification: `npx eslint .` — 0 errors; `npm run build` — clean incl. Obsidian 1.11.0 compat; `npx vitest run` — 632/633 (1 unrelated pre-existing failure in `ci-workflow.test.ts` from an uncommitted `ci.yml`, not touched by this work). - [x] sync-status-workflow-ui plan (2026-08-22), branch `feat/sync-status-workflow-ui`, 4 commits (`8c69cc8` → `dd8ddd5`) ahead of `claude/source-control-foundation` @ `f449125`: (1) ViewModel `selectedItems`/`refreshStatus` projections + `refresh()` delegate backed by new `RefreshState` (idle/loading/failed); (2) filter chips drop `ready-to-push` (4 chips: All/Local/Remote/Conflict) + "SELECTED FOR SYNC (N)" section; (3) refresh button (idle/loading/failed) + `OperationIndicator` text labels + `runRefresh` render-on-start-and-settle; (4) new `ChangePresentation` UI adapter (`remote-only` badged `D`, subtitles, rename display), eager local-only diff-stat from in-memory `sync.status` + lazy two-sided stat on open + clear-on-refresh cache (null results cached to stop an eager-retry rerender loop), responsive mobile (filter dropdown, hidden header push button, sticky bottom sync bar, flatter tree). Domain-untouched invariant verified: `git diff claude/source-control-foundation -- src/logic/source-control/` shows only `RefreshState.ts` (new) + `SourceControlViewModel.ts` (edited). Verification: `npx eslint .` — 0 errors; `npm run build` — clean incl. Obsidian 1.11.0 compat; `npx vitest run` — 61 files / 629 tests; husky pre-commit hook green on every commit. Manual Obsidian desktop/mobile UI verification remains. - [x] Issue #105 post-push CI hardening (2026-08-20), commit `948df28`: diagnosed run 32336155736 as two exhausted transient-provider attempts rather than a planner regression (GitHub 503/socket close; GitLab deadline exceeded). Increased provider E2E attempts from 2 to 3. A duplicate matrix cancelled by the shared push/PR concurrency group now produces a neutral aggregate gate with `run-ci=false`, so it neither creates a misleading `E2E gate` failure nor starts duplicate downstream CI; real failures still block. SyncManager E2E push preconditions now include `success`, `failed`, and provider `errors` in assertion diagnostics instead of surfacing only a secondary count mismatch. Added workflow contract and diagnostic unit tests and updated the E2E documentation. Verification: `actionlint v1.7.12 .github/workflows/ci.yml` — 0 errors; `npx eslint .` — 0 errors; `npm run build` — clean including Obsidian 1.11 compatibility; `npx vitest run` — 56 files / 613 tests; `npm run test:e2e -- --provider gitea` — 2 files / 14 tests and container cleanup; `git diff --check` — clean. Real CI run 32338116598 passed GitHub/GitLab production E2E, independent verification, cleanup, aggregate gate, Node 22/24 tests, lint, package, and build/release. The initial disabled-Gitea job landed on offline runner `heavenweb-runner-8`; failed-only rerun completed its skip in 11s and the full run concluded success. Provider API checks found no remaining `e2e/pr/127/**` or branch-source E2E refs. AGENTS-required Haiku was unavailable, so verification ran locally and through real CI. diff --git a/session-handoff.md b/session-handoff.md index 468ea0c..3f74bec 100644 --- a/session-handoff.md +++ b/session-handoff.md @@ -1,62 +1,42 @@ # Session Handoff -**Date:** 2026-08-22 -**Branch:** `feat/sync-status-workflow-ui` (4 commits ahead of `claude/source-control-foundation` @ `f449125`) -**Active Feature:** sync-status-workflow-ui plan (`.kilo/plans/1787412338771-sync-status-workflow-ui.md`) — COMPLETE - -## Completed This Session - -Implemented the full four-commit "Sync Status Workflow UI" feature. All four -commits land on `feat/sync-status-workflow-ui`, each passing the husky -pre-commit hook (`npm run lint && npm run build`): - -1. `8c69cc8` — `SourceControlViewModel` gains `selectedItems` + - `refreshStatus` projections and a `refresh()` delegate backed by a new - `RefreshState` holder (idle/loading/failed, mirrors `OperationState`). - `main.ts` wires `() => syncWorkspace.refresh()` as the delegate. 5-arg - ViewModel constructor; 3 test helpers updated. -2. `625fad2` — Filter chips drop `ready-to-push` (now 4: All/Local/Remote/ - Conflict via new `sourceControl.filter.local/remote/conflict` i18n; domain - `data-filter` values unchanged). New `renderSelectedSection()` shows - "SELECTED FOR SYNC (N)" above the tree. -3. `759b717` — Refresh button (idle icon-only / loading "Refreshing…" - spinning+disabled / failed "Refresh failed") in the header; `onRefresh` - added to `SourceControlViewCallbacks`; `OperationIndicator` now renders - icon + text label; `SourceControlItemView.runRefresh()` renders on start - and settle, swallows rejection. -4. `dd8ddd5` — New `ChangePresentation` UI adapter (badge letter/subtitle/ - rename/tooltip per kind; `remote-only` badged `D` not `A`). Diff-stat - threaded through rows: local-only stats eager-loaded from in-memory - `sync.status` (no provider call) + cached; two-sided stats lazy-load on - open; cache clears on refresh (null results cached too, to avoid an - eager-retry rerender loop that initially OOM'd the test worker). - Responsive mobile: chips → single filter `` dropdown replaces the - * chips (same domain values, counts inline as "Label (N)"). + * Renders the Source Control filter row: five chips — All / Needs Sync / + * Remote / Conflict / Synced. On mobile a single `` over the same domain filter values, - * options labeled "Label (N)". Keeps the chip row's counts and domain values - * but collapses four chips into a single control. + * Mobile filter dropdown: one `