fix(ui): drop setCssProps last-row override, use CSS :last-child instead - #155
Merged
Conversation
renderDiffViewer rendered an empty diff panel synchronously, then SourceControlView's async load appended a second, real one alongside it via a raw renderDiffPanel(container, ...) call -- two Remote/Local panels stacked on screen, the top one permanently empty. DiffViewer now owns the diff body's whole lifecycle: content-less options render just the body, and callers fill it in later through a returned handle's setContent(), which empties the body before rendering so there's ever only one panel. Also stops relying on Platform.isMobile alone for split/unified: tablets report isMobile too, so the previous "mobile defaults to unified" policy didn't stop a user's saved split preference from producing unreadable ~150px columns on an actual phone. renderDiffViewer now forces unified and hides the layout toggle whenever Platform.isPhone is true, regardless of session preference, across all three diff surfaces (diff tab, conflict modal, mobile detail) that share it. Adds defensive min-width/max-width and minmax(0, 1fr) grid tracks so a long unbroken diff line can't push the split panel past its container.
CLAUDE.md still described src/ui/SyncStatusView.ts as the plugin's main UI and never mentioned the Source Control surface that replaced it, so an agent reading it cold would look for a file that no longer exists and miss the real call chain (SourceControlItemView -> SourceControlView -> SourceControlActionService -> SyncWorkspace -> SyncManager/executors). Also documents the two compatibility identifiers (SOURCE_CONTROL_VIEW_TYPE = 'sync-status-view', the open-sync-status command id) as intentional, not leftover legacy code to clean up.
The remote-delete E2E called service.deleteFile() directly, with a comment saying it reproduced src/ui/SyncStatusView.ts's real call path -- but that view was removed. The production path is now SourceControlActionService.deleteRemote() -> SyncWorkspace.deleteRemote() -> RemoteDeleteExecutor -> gitService.deleteFile(), which also clears tracked metadata and the live status row as part of the same call, not as a separate manual step the way this test's old manager.clearMetadata() call implied. Rebuilds the test on a real SyncManagerWorkspace + SourceControlActionService, verified against a live Gitea sandbox (npm run test:e2e -- --provider gitea: 36 passed, 18 skipped).
docs/source-control-refactor/{roadmap,phase-1..4}.md describe an
in-progress migration (roadmap.md dated 2026-08-22, still narrating
uncommitted WIP) that has since landed on main in full -- nothing in that
directory reflects the current implementation, but nothing marked it as
historical either. Adds a banner to each pointing at the new
docs/source-control.md, which describes only the current architecture and
call chain without duplicating the old roadmap's narrative.
Two regression guards so a future refactor can't silently undo this cleanup: eslint.config.mts's no-restricted-imports rule blocking ui/sync-status and SyncStatusView imports is now asserted directly (it existed before this PR but had no test locking it in), and the remote delete E2E is now locked to keep going through SourceControlActionService/SyncWorkspace rather than quietly reverting to a direct service.deleteFile() provider bypass.
…ection Adds resolveSyncAction/availableSyncActions to ChangeActionPolicy and per-change action override tracking to SyncSelectionStore, so an explicit user choice (e.g. pull instead of the default push) can survive selection state without the store having to know change-kind legality itself. Not yet wired into the UI or ActionService.
SourceControlItem now carries a resolved syncAction (override if still legal for the current kind, else the kind default) and hasActionOverride, projected in SourceControlViewModel with stale-override cleanup baked in. Sync Queue grouping (Upload/Download/Delete) now reads item.syncAction instead of recomputing defaultSyncAction(item.kind), so a queue with overridden items groups by what Sync will actually do. Action execution (ActionService, SyncPlan) still ignores the override — that's the next commit.
SourceControlActionService.sync() now takes SyncIntentRequest[] (changeId + optional action) instead of bare ChangeId[], resolving each via ChangeActionPolicy.resolveSyncAction against the change's current kind before bucketing into push/pull/delete-remote — so a stale intent degrades to the kind's default rather than forcing an illegal action. SourceControlView.runSync threads an item's override through only when hasActionOverride is set; plain queue items still sync via the default. Conflict resolution, plan merging, and the single-commit contract are unchanged.
…ueue Sync Queue rows now render a resolved-action control (icon+label on desktop, icon-only on phone) instead of the plain Download button; clicking it opens a Menu scoped to ChangeActionPolicy.availableSyncActions for the row's kind, plus View diff and Remove from Sync Queue. Choosing the kind's own default clears any stored override instead of recording a redundant one. Repository Changes rows are unchanged — the control is Sync Queue-only, per row, not added to every tree/list row. Adds a DOM-backed Menu/MenuItem mock to tests/setup.ts (Obsidian's real Menu drives a native/DOM popover Node can't render) so queue-row menu interactions can be tested the same way as any other rendered control.
…n phone The Repository Changes row's Download button still rendered its full text label on phone, crowding the filename at narrow widths — the mobile-diff branch fixed diff rendering but never touched this control. Presentation-only: canDownload()/onDownload() are unchanged, the label stays in the DOM (screen readers/tooltip still get it), only its visual display and the button's padding change under body.is-mobile.
…Changes rows Adds a "⋯" row menu (rowMenuActions in ChangeItem.ts) offering the immediate actions relevant to a change's kind — e.g. local-modified gets Push local / Use remote… / View diff / Add to Sync Queue / Delete local…, remote-only gets Download / Delete remote… / Add to Sync Queue / Open remote. conflict/synced get no menu (conflict resolution keeps its own dedicated UI; synced is never actionable). Wired through new onPush/ onDeleteRemote/onDeleteLocal callbacks on SourceControlViewCallbacks, executed immediately via SourceControlActionService — distinct from queuing, which stays override-based via the existing action control. Delete remote goes through the existing ConfirmModal first (no equivalent safety net to Obsidian's own trash, which deleteLocal already uses); delete local does not re-confirm since trashFile already is one. The Modal mock in tests/setup.ts now appends modalEl to document.body on open() (matching real Obsidian), needed to test the confirm flow the same way other rendered controls are tested.
The merged Sync review can list additions/modifications/moves, downloads, and deletions in one long scroll; a row far from its section heading previously carried no cue of its own direction. Each file row now repeats the section's existing icon (already shown once in the heading) inline before the path — presentation only, no new grouping, no selector: classification, conflict resolution, and bucket planning are unchanged.
Separate queued sync intent execution from immediate Source Control actions, reconcile stale action overrides on repository snapshot changes, and keep ViewModel reads observational.
… in header The header showed both "Last sync" and "Last checked" times, which read as duplicated; keep only "Last checked" and drop the now-unused Conflict filter chip (conflicts remain reachable via the default Needs Sync / All views). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…r/reconciler Give file discovery, status resolution, and rename reconciliation each a single owning class (SyncFileDiscovery, SyncStatusResolver, RenameReconciler) instead of one 661-line service implementing all three algorithms. SyncStatusRefreshService now only orchestrates the three plus the incremental create/modify/delete/rename handlers, per the module boundaries in docs/architecture.md. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ol composition root Move the sync-domain and Source Control application constructor graph (SyncManager, SyncStatusRefreshService, SyncDiffService, SyncWorkspace, ChangeRepository, SyncSelectionStore, OperationState, RefreshState, SourceControlViewModel, SourceControlActionService, and the ChangeRepository<->SyncStatusService wiring) out of main.ts and into src/runtime/createSyncRuntime.ts. main.ts now only knows Obsidian lifecycle: settings load/save, view/command/ribbon registration, vault event registration -- it no longer needs to know the sync/Source Control constructor graph to add a plugin lifecycle hook. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…rom SourceControlView Pull the "Sync Queue" and "Repository Changes" regions out of the 730-line SourceControlView into standalone render functions, matching the existing FilterMenu/SourceControlHeader/ChangeTree pattern: pure functions taking state + callbacks, never SyncWorkspace/SourceControlActionService/ SourceControlViewModel directly. SourceControlView keeps ownership of view state (collapsed sections, view mode, folder collapse) and now only orchestrates rendering, the diff pane, and scroll-state management. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…sync->UI dependency Add no-restricted-imports rules per docs/architecture.md: src/ui/** and src/logic/source-control/** may not import a concrete Git provider or a push/pull coordinator/executor directly (must go through SyncWorkspace), and src/logic/sync/** may not import src/ui/source-control/** (dependency direction runs UI -> domain, never back). The last rule caught a real pre-existing violation: SyncDiffService and SyncInteractionPort (sync domain) imported computeDiffStat and DiffStatLoadResult from ui/source-control. Move the pure diff-stat computation (computeDiffStat, cheapLocalStat, addedContentStat, deletedContentStat) and the DiffStatLoadResult contract into a new src/logic/sync/DiffStat.ts; ChangePresentation.ts and DiffStatProvider.ts now depend on the domain for these instead of the other way around. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ndaries CLAUDE.md's "Code Architecture" section duplicated (and had drifted from) docs/architecture.md: it said GitLab/GitHub only (no Gitea, though GiteaService already existed) and described sync-manager.ts as a single file. Replace it with a short contract pointing at docs/architecture.md (and docs/bug-fix-guidelines.md for bug fixes) plus the two compatibility gotchas that aren't covered there. docs/architecture.md's module table and "Current hotspots" section now describe the merged code: createSyncRuntime as the composition root, SyncFileDiscovery/SyncStatusResolver/RenameReconciler as SyncStatusRefreshService's three collaborators, DiffStat.ts, and SourceControlView's reduced scope after the SyncQueueSection/ RepositoryChangesSection extraction. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* refactor(source-control): make view model projection-only
SourceControlViewModel is now a pure read-only projection: it no longer
exposes a `selection` getter or owns any subscription wiring. Selection
mutation (select/deselect, batch select/deselect, action override
set/clear) moves to SourceControlActionService, and the
ChangeRepository -> SyncSelectionStore.reconcile() wiring moves to
createSyncRuntime, which also drops the now-redundant explicit
syncSelectionStore.refresh() call superseded by that reconciliation.
SourceControlView calls injected callbacks instead of reaching into
SyncSelectionStore directly.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* docs: record PR2 item 1 (source control state boundary) session progress
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* refactor(settings): separate settings model from UI
Splits the former src/settings-implementation.ts into src/settings/model.ts
(SyncMetadata, GitServiceType, SymlinkHandling, GitLabFilesPushSettings,
DEFAULT_SETTINGS), src/settings/helpers.ts (isSyncMetadataAtPath,
getServiceName, getEffectiveSymlinkHandling), and
src/ui/settings/GitLabSyncSettingTab.ts (all Obsidian Setting rendering).
src/settings.ts becomes a thin public-compatibility re-export so every
existing `from './settings'` import keeps working unchanged.
The settings UI no longer imports the concrete GitLabFilesPush class for
its own behavior: GitLabSyncSettingTab now depends on a narrow
SettingsHost interface (settings, saveSettings, initializeGitService,
testConnection, activateSourceControlView, onConnectionStatusChange).
Its constructor takes `plugin: Plugin` and `host: SettingsHost` as
separate parameters rather than an intersection type, since intersecting
with Plugin re-introduces Plugin's own version-gated `settings` field
and trips this repo's obsidianmd/no-unsupported-api lint rule. One
remaining wart (RemoteFolderSuggest.attach still requires the concrete
plugin class) is called out with a type-only cast and a comment rather
than widened into SettingsHost or fixed here, since narrowing
RemoteFolderSuggest itself is a separate, unrelated cleanup.
No settings UX change; no lint config change.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* docs: record PR2 item 2 (settings boundary cleanup) session progress
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* refactor(source-control): centralize item projection
Adds SourceControlViewModel.getItem(id), the single SourceControlItem
projection path for callers that need one row's current state by id.
SourceControlItemView.refreshOpenDiffTab() no longer hand-rolls a
SourceControlItem with hardcoded isSelectedForSync: false / operationStatus:
'idle' / a fresh resolveSyncAction() call -- it now reads the ViewModel's
real projection, so a queued/running row's open diff tab refresh reflects
its actual state instead of silently reporting defaults. SourceControlView's
mobile detail loadAndRenderDiff() similarly drops its two-getState() lookup
(needed to also catch 'synced' rows) in favor of the unfiltered getItem().
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* docs: record PR2 item 3 (centralize item projection) session progress
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* refactor(sync): reuse pull orchestration path
SyncManager.pullFile() no longer duplicates PullCoordinator's own
no-prefetched-tree classification (exists/local-content/local-sha/baseline
computation + SyncPlanner.planFor('pull', ...)) -- including the legacy
GitLab revision-keyed baseline correction, which existed identically in
both places. Both now go through PullCoordinator.planSingleFile(), a thin
wrapper over the existing private planFromRemote(), so single-file and
batch pull can no longer silently drift apart on planning semantics.
Deliberately left separate (see planSingleFile's doc comment): interactive
conflict resolution (single-file pull opens a modal inline; batch pull
skips/aggregates conflicts), per-call confirmation (single confirms every
call; batch confirms once for the whole plan), and notification (single
always reports "up to date"; batch only summarizes). Those are deliberate
UX differences, not accidental duplication, so they stay owned by each
caller rather than being forced into one shape.
Also removes SyncManager's now-dead SyncPlanner instance and its
contentsEqual/isBinaryPath/gitBlobSha imports, all of which existed only
to duplicate planFromRemote's own logic.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* docs: record PR2 item 4 (reuse pull orchestration) session progress
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* refactor(services): move ConnectionTestResult to the interface module
ConnectionTestResult is part of GitServiceInterface's contract
(testConnection's return type), not an implementation detail of
BaseGitService, so it now lives in git-service-interface.ts alongside
the interface that references it. git-service-base.ts imports it back
for its own abstract testConnection signature.
Left updateConfig(...args: unknown[]) as-is: every call site invokes
it on the concrete service class, never through GitServiceInterface,
so the loose signature isn't causing an actual type-safety gap.
Converting it to a typed discriminated union would require reshaping
the interface, all three services' updateConfig bodies, and all three
main.ts call sites for no functional benefit -- deferred rather than
folded into this cleanup.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* docs: record PR2 item 5 (provider contract cleanup) session progress
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Lint flagged the JS setCssProps() call for inline style mutation, and it also crashed in the jsdom test environment (no setCssProps polyfill), failing both the Lint and Unit Test CI jobs. The border-bottom removal on the last row is pure presentation, so express it with a :last-child selector instead of touching the DOM in code. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
# Conflicts: # tests/ui/source-control/SourceControlView.test.ts
|
ClaudiaFang
pushed a commit
that referenced
this pull request
Sep 1, 2026
## [1.6.1](1.6.0...1.6.1) (2026-09-01) ### Bug Fixes * **docs:** align agent guidance with source control architecture ([cf2239d](cf2239d)) * **docs:** mark legacy source control migration docs historical ([0f11e17](0f11e17)) * **e2e:** exercise current remote delete application path ([8fcdfea](8fcdfea)) * **mobile:** remove duplicate safe-area padding from sync bar ([3a053a9](3a053a9)) * **source-control:** add compact per-row action control to the Sync Queue ([8d5d427](8d5d427)) * **source-control:** add per-kind advanced action menu to Repository Changes rows ([1a54acb](1a54acb)) * **source-control:** dedupe mobile diff render, force unified on phones ([fd6d7ec](fd6d7ec)) * **source-control:** honor explicit actions in sync queue execution ([735bdc0](735bdc0)) * **source-control:** preserve per-change action overrides in sync selection ([6baa0da](6baa0da)) * **source-control:** remove Conflict filter chip and dedupe sync time in header ([1c69aed](1c69aed)) * **source-control:** remove Conflict filter chip and dedupe sync time in header ([e2015cc](e2015cc)) * **source-control:** resolve queue grouping from actual sync action ([601013f](601013f)) * **source-control:** shrink the inline Download button to icon-only on phone ([b44389d](b44389d)) * **sync-plan:** mark each plan row with its section's direction icon ([c4a198c](c4a198c)) * **test:** guard removed sync status presentation imports ([beba48d](beba48d)) * **ui:** drop setCssProps last-row override, use CSS :last-child instead ([15e1a6b](15e1a6b)) * **ui:** drop setCssProps last-row override, use CSS :last-child instead ([#155](#155)) ([f626fb5](f626fb5)), closes [#154](#154) * **ui:** remove duplicate batch conflict divider ([6aab586](6aab586)) * **ui:** use Obsidian CSS helper for conflict divider ([adee61d](adee61d)) ### Documentation * **architecture:** define module boundaries and bug-fix rules ([025a8c7](025a8c7)) * record explicit sync intent session in progress.md ([d91fa6e](d91fa6e)) * sync CLAUDE.md and architecture.md with the enforced module boundaries ([69e5540](69e5540)) ### Code Refactoring * **architecture:** add ESLint boundary guards and fix a reverse sync->UI dependency ([9f7449c](9f7449c)) * **runtime:** extract createSyncRuntime as the sync/Source Control composition root ([f21f169](f21f169)) * **source-control:** isolate sync intent orchestration ([f554031](f554031)) * **source-control:** responsibility cleanup (PR2) ([#154](#154)) ([499ac9e](499ac9e)) * **sync:** split SyncStatusRefreshService into discovery/resolver/reconciler ([e94a6f7](e94a6f7)) * **ui:** extract SyncQueueSection and RepositoryChangesSection from SourceControlView ([68c3683](68c3683))
Member
Author
|
🎉 This PR is included in version 1.6.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Type of Change
Engineering Standards
Related Issues