fix(source-control): dedupe mobile diff render, force unified on phones - #147
Closed
ClaudiaFang wants to merge 5 commits into
Closed
ClaudiaFang wants to merge 5 commits into
ClaudiaFang wants to merge 5 commits into
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.
|
ClaudiaFang
deleted the
claude/fix-mobile-diff-rendering-and-responsive-layout
branch
September 1, 2026 03:51
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.
Summary
renderDiffViewerrendered an empty diff panel synchronously and the async load inSourceControlViewappended a second, real one via a rawrenderDiffPanel(container, ...)call — two Remote/Local panels stacked on screen, the top permanently empty.DiffViewernow owns the diff body's whole lifecycle: content-less options render just the body, and callers fill it in later through a returned handle'ssetContent(), which empties the body first.Platform.isMobilealone isn't enough to decide split vs. unified — tablets reportisMobiletoo, so a saved split preference could still render unreadable ~150px columns on an actual phone.renderDiffViewernow forces unified and hides the layout toggle wheneverPlatform.isPhoneis true, regardless of session preference, across all three surfaces that share it (diff tab, conflict modal, mobile detail).min-width/max-width/minmax(0, 1fr)grid-track CSS so a long unbroken diff line can't push the split panel past its container.Test plan
npx eslint .— 0 errorsnpm run build— passes, includes Obsidian 1.11.0 compat typechecknpx vitest run— 869/869 passing, including new regression coverage:Platform.isPhoneforces unified and hides the toggle regardless of session split preferenceDiffViewer'ssetContent()replaces content rather than appending a second copy🤖 Generated with Claude Code