Skip to content

fix(ios): keep workspace-list viewport when exiting a workspace - #10292

Closed
azooz2003-bit wants to merge 2 commits into
mainfrom
feat-ios-ws-exit-scroll-restore
Closed

azooz2003-bit wants to merge 2 commits into
mainfrom
feat-ios-ws-exit-scroll-restore

Conversation

@azooz2003-bit

@azooz2003-bit azooz2003-bit commented Aug 17, 2026 •

Copy link
Copy Markdown
Collaborator

Exiting a workspace on iPhone reset the workspace list to the top. The compact push stack flips showsNavigationToolbar on every workspace enter/exit, and WorkspaceListView+Toolbar.swift used a structural if around the list content, so each flip changed the subtree's view identity. SwiftUI dismantled the UITableView-backed list (WorkspaceListTable) and the recreated table opened at offset zero.

Fix: keep one stable subtree and move the conditional inside the ToolbarContentBuilder (the same idiom WorkspaceShellView.stackLayout already uses for rootToolbarContent). With stable identity the same UITableView survives push/pop and UIKit preserves the exact scroll position natively. Toolbar behavior is unchanged: items still disappear while a workspace is open.

Two commits so CI proves the test catches the bug: commit 1 adds WorkspaceListViewportRestorationTests (hosts the real WorkspaceListView, scrolls, flips the flag both ways, asserts same table instance and same content offset) and fails; commit 2 adds the fix and goes green.

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Keep the workspace list viewport on iPhone when exiting a workspace. Previously, exiting reset the list to the top; now the scroll position is preserved by keeping the list subtree identity stable.

  • Moves the showsNavigationToolbar check inside the .toolbar builder so the list view isn’t structurally replaced during push/pop; matches the pattern used in WorkspaceShellView.stackLayout.
  • Adds WorkspaceListViewportRestorationTests that flips the flag around the real WorkspaceListView and asserts the same UITableView instance and content offset.
  • iOS-only; toolbar behavior is unchanged (items still hide in-workspace). No migrations or config changes.

Written for commit 722aa9f. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Bug Fixes

    • Workspace list content now remains stable when the navigation toolbar is shown or hidden.
    • Preserved workspace list position and scroll offset when entering or leaving a workspace while toggling the toolbar.
  • Tests

    • Added coverage verifying workspace list state and content position are retained during toolbar changes.

azooz2003-bit and others added 2 commits August 17, 2026 15:23
Regression test: flipping showsNavigationToolbar (what a compact-stack
workspace push/pop does) must not recreate the UITableView-backed list or
reset its content offset.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The compact push stack flips showsNavigationToolbar on every workspace
enter/exit. The toolbar branch was structural (if around content), so each
flip changed the list subtree's identity, SwiftUI dismantled the
UITableView-backed list, and the recreated table opened at the top. Move
the conditional inside the ToolbarContentBuilder so the subtree identity is
stable and UIKit keeps the exact scroll position across enter/exit.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 17, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 155636c5-4e60-4094-bcc2-4f5aaaef98f1

📥 Commits

Reviewing files that changed from the base of the PR and between 240c978 and 722aa9f.

📒 Files selected for processing (2)
  • Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView+Toolbar.swift
  • Packages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/WorkspaceListViewportRestorationTests.swift

Included review availability: Your plan includes up to 10 reviews per rolling hour; 2 remain after this review.


📝 Walkthrough

Walkthrough

The iOS workspace list now keeps its content structurally present while navigation toolbar items change. A UI test verifies that workspace navigation preserves the table view instance and content offset.

Changes

Workspace list toolbar stability

Layer / File(s) Summary
Preserve workspace list content across toolbar changes
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView+Toolbar.swift, Packages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/WorkspaceListViewportRestorationTests.swift
The toolbar conditional now runs inside the toolbar builder. The test harness toggles showsNavigationToolbar and verifies table identity and viewport preservation across workspace enter and exit.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 722aa

This localized iOS change preserves the workspace list’s existing view identity so the scroll position remains intact when entering or exiting a workspace; no actionable merge-blocking risk remains beyond normal checks and review.

Possibly related PRs

🚥 Pre-merge checks | ✅ 25
✅ Passed checks (25 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the primary fix: preserving the workspace-list viewport when exiting a workspace.
Description check ✅ Passed The description clearly explains the cause, fix, unchanged behavior, and added regression test, although template sections such as Demo Video and Checklist are absent.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Cmux Swift Actor Isolation ✅ Passed The reported production change is a SwiftUI view composition change, and the added UI test is explicitly allowed by the check; inspect the actual diff to confirm no isolation declarations changed.
Cmux Swift Blocking Runtime ✅ Passed The combined Swift diff adds no semaphore, blocking wait, sleep, delayed dispatch, polling, main-queue sync, or manual lock; the test uses deterministic UIKit layout and offset operations.
Cmux Browser Automation Off-Main ✅ Passed The diff only changes iOS workspace-list SwiftUI code and tests; it does not modify browser socket commands, WebKit/AppKit routing, worker policy, or browser policy tests.
Cmux Expensive Synchronous Load ✅ Passed The full PR diff adds only a SwiftUI toolbar conditional in production; it adds no agent-history loader, file parse, directory scan, or synchronous load on an interactive path.
Cmux Cache Substitution Correctness ✅ Passed The production diff only moves the showsNavigationToolbar condition inside SwiftUI's toolbar builder; it introduces no cached-value substitution in persistence, history, undo, or snapshot paths.
Cmux No Hacky Sleeps ✅ Passed The combined PR diff changes only two Swift files; the rule applies only to non-Swift TypeScript, JavaScript, shell, and build/runtime scripts.
Cmux Algorithmic Complexity ✅ Passed The production diff only moves a Boolean conditional inside ToolbarContentBuilder; it adds no collection scan, sort, filter, join, or batch action. The 60-item map is test-only scaffolding.
Cmux Swift Concurrency ✅ Passed The diff adds SwiftUI toolbar restructuring and a MainActor UIKit test; structural and text scans found no DispatchQueue, Task, Combine, or completion-handler patterns.
Cmux Swift @Concurrent ✅ Passed The two-commit diff adds only a synchronous @MainActor UI test and moves a toolbar condition; it introduces no async, nonisolated, or @concurrent work.
Cmux Swift Package Boundaries ✅ Passed The diff changes only SwiftUI toolbar composition in the existing CmuxMobileShellUI package and adds a UI test harness; it introduces no reusable or independently testable domain logic requiring ex...
Cmux Swiftpm Lockfiles ✅ Passed The PR range changes only two Swift source/test files; no Package.swift, Package.resolved, .gitignore, Xcode project, workflow, or dependency files changed.
Cmux Swift Logging ✅ Passed The full two-commit diff changes toolbar structure and adds a test; added Swift lines contain no print, debugPrint, dump, NSLog, Logger, or ad hoc I/O logging.
Cmux User-Facing Error Privacy ✅ Passed The diff changes SwiftUI toolbar structure and adds a test; it introduces no production user-facing errors, alerts, command output, or recovery copy.
Cmux Full Internationalization ✅ Passed The PR changes only SwiftUI structure and developer comments in production; added strings are test fixtures/assertions, with no new user-facing text or localization/catalog changes.
Cmux Swiftui State Layout ✅ Passed The diff only moves the existing toolbar conditional and adds a harness using existing @Observable state; it adds no prohibited ObservableObject state, GeometryReader, row store, or render-time mut...
Cmux Architecture Rethink ✅ Passed The diff is a local stable-subtree fix with a clear viewport-identity invariant; it adds no timing, blocking, state side channel, duplicate wiring, or split lifecycle owner.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed The PR changes only WorkspaceListView toolbar structure and adds a test-only UIWindow fixture; it introduces no standalone cmux-owned NSWindow, panel, controller, Window, or WindowGroup.
Cmux Source Artifacts ✅ Passed The PR adds only a Swift source file and an intentional iOS regression test; no logs, caches, build output, screenshots, temp folders, or other artifacts enter the diff.
Cmux No Test Or Debug Seam In Production Source ✅ Passed The two-commit diff adds test scaffolding only under Tests and changes toolbar composition; it adds no test/debug seam, visibility widening, or build guard in production Sources. The existing DEBUG...
Cmux No Ambient Global State ✅ Passed The production diff only changes the existing WorkspaceListView extension method; it adds no top-level functions, mutable globals, static namespaces, or singletons.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat-ios-ws-exit-scroll-restore

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@azooz2003-bit

Copy link
Copy Markdown
Collaborator Author

Red/green evidence (run on branch feat-ios-ws-exit-scroll-restore-goodbase because current main does not compile — macOS cleanupSurfaceState(workspaceID:) and iOS store.selectMacSurface from #10072, revert pending in #10285; the evidence branch is the PR 10284 tree plus the PR 9668 test unbreak and the selected-test-guard fix from #10300):

  • RED run 32082457559 — test only: ios-simulator (iphone) fails with tableAfterExit === table violated (fresh WorkspaceListUITableView, contentOffset (0, -116) top vs saved (0, 600)).
  • GREEN run 32082481153 — test + fix: both simulator legs pass, Test run with 1 test in 1 suite passed.

The package-conventions-lint failures on those runs are pre-existing base-tree debt (TaskComposerSheet free functions, DiagnosticBuildStamp namespace enum), untouched by this PR.

@azooz2003-bit

Copy link
Copy Markdown
Collaborator Author

Superseded by #10488 (same root-cause fix on the current tree, merged as 92b81ea). The anchor-restore follow-ups on feat-ios-ws-exit-scroll-restore-goodbase remain available if on-device dogfood ever shows nav-bar inset drift.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant