Repository navigation
iOS: reserve title space for the badged back button - #10790
azooz2003-bit wants to merge 1 commit into
Conversation
Opening a workspace while another workspace held unread folded the terminal picker into the More menu from the first layout pass (iOS 27 simulator dogfood; sticky until remount because a birth collapse never produces the attach-then-detach signature the recovery ratchet watches). The width model reserved a flat 44pt for the back button, but with unread elsewhere WorkspaceBackButton renders chevron + spacing + a mono badge circle, ~24pt wider (~32pt at "99+"), and that over-claim was exactly the fold margin. Reopening cleared the unread, removed the badge, and hid the bug, which made it look intermittent. The reserve now scales with the badge deterministically from the same unreadCount the button renders. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughThe iOS workspace title width calculation now reserves space for back-button unread badges. Counts above 99 use a wider reserve. The unread count flows from workspace detail data through ChangesUnread Badge Width Handling
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The change reserves additional title space when the back button shows an unread badge, preventing toolbar controls from being folded into More. No actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 24 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (24 passed)
Full details: Description checkExplanation The description clearly explains the problem, cause, fix, and verification. It includes detailed simulator testing and regression-test coverage. The template's Demo Video, Review Trigger, and Checklist sections are not included, but the core description is complete. Full details: Cmux Swift Actor IsolationExplanation PASS: The production diff adds only Full details: Cmux Swift Blocking RuntimeExplanation PASS: The production diff only adds unread-count data flow and deterministic width arithmetic in Full details: Cmux Browser Automation Off-MainExplanation PASS: The pull request changes only iOS toolbar title-width and related tests. The exact diff contains no browser socket commands, WebKit waits, main Full details: Cmux Expensive Synchronous LoadExplanation PASS. The production diff only adds an Full details: Cmux Cache Substitution CorrectnessExplanation PASS — The diff adds unread-count data only to transient SwiftUI toolbar layout. Full details: Cmux No Hacky SleepsExplanation PASS. The pull request changes six Swift files only, including Swift production code and Swift tests. The rule applies to TypeScript, JavaScript, shell, and non-Swift build/runtime scripts; Swift timing and blocking primitives are explicitly covered by a separate check. The diff introduces no covered non-Swift delay or timer changes. Full details: Cmux Algorithmic ComplexityExplanation PASS: The production changes add only constant-time badge-width arithmetic and an integer field propagation. Full details: Cmux Swift ConcurrencyExplanation PASS: The commit adds only synchronous width calculations, stored values, and tests. The complete Swift diff adds no DispatchQueue, Combine, completion-handler, or fire-and-forget Task patterns. Existing async code in WorkspaceDetailView is unchanged; the production view change only passes Full details: Cmux Swift `@Concurrent`Explanation PASS: The diff adds only synchronous width calculations, stored Full details: Cmux Swift Package BoundariesExplanation PASS: The production diff stays inside the existing Full details: Cmux Swiftpm LockfilesExplanation PASS. The PR diff changes only six Swift source and test files under Full details: Cmux Swift LoggingExplanation PASS. The pull request adds no Full details: Cmux User-Facing Error PrivacyExplanation PASS: The PR changes title-width layout math and unread-count state only. The production diff adds no user-facing error, alert, command output, API error body, or recovery copy. Added comments and regression tests are explicitly allowed by the rule. The new values are used only to calculate Full details: Cmux Full InternationalizationExplanation PASS: The production diff changes width calculation and unread-count data flow only. It adds no user-facing Swift text, localization key, string catalog entry, web message, metadata, or rendered copy. The only added "99+" occurrences are developer comments. Tests and fixture updates are explicitly allowed by the rule. Full details: Cmux Swiftui State LayoutExplanation PASS — The diff adds only immutable Full details: Cmux Architecture RethinkExplanation PASS: This is a small local correctness fix. The new immutable Full details: Cmux Swift Auxiliary Window Close ShortcutsExplanation PASS: The PR changes title-width calculation, workspace menu value plumbing, and tests. The diff adds no NSWindow, NSPanel, NSWindowController, Window, WindowGroup, window identifier, Cmd+W, or close-routing code. WorkspaceTitleMenu is an allowed menu/view case. The repository lint also passes and reports 35 registered cmux window identifiers; no auxiliary-window close-shortcut violation is introduced. Full details: Cmux Source ArtifactsExplanation All six changed paths are intentional Swift source or test files under the existing iOS source and test directories. The diff adds badge-aware width logic and regression tests only. No logs, screenshots, recordings, temporary or cache directories, build output, dependency checkout, package-manager download, or broad artifact directory appears in the changed paths. Full details: Cmux No Test Or Debug Seam In Production SourceExplanation PASS. The production diff adds no Full details: Cmux No Ambient Global StateExplanation PASS. The production diff adds no ambient global state.
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
|
Verification extended per dogfood request: the badged mount now proven on BOTH runtimes on isolated simulators. iOS 27.0 (iPhone 17): badged terminal mount keeps back+badge, title, Changes, picker in the bar. iOS 26.5 (iPhone 17): badged mount into a browser-restoring workspace (the hardest combination: badge + restore + two trailing items) keeps the full bar with the workspace-name/tab-subtitle pill. No More menu in either case. |
|
Heads-up: #10898 cherry-picked this commit (c9f3005) and then retiered the constants for the new capsule badge shape it introduces (the capsule is wider than the old clipped circle at two-digit and 99+ counts, so 24/32 under-reserves there). If 10898 merges first, this PR is superseded and can close; if this merges first, 10898 will carry the retier as a small conflict resolution. |
Dogfood on an iOS 27.0 simulator (build containing #10646, verified by ancestry of its CMUX_GIT_SHA): opening a workspace occasionally showed the terminal picker inside the More "…" from the first frame, and reopening the same workspace cleared it. The screenshots carried the tell: every broken shot had an unread badge on the back button, and the healthy after-reopen shot did not (reopening cleared the unread).
Mechanism:
MobileLeadingToolbarTitleWidthreserves a flatbackButtonReserve = 44for the back control, butWorkspaceBackButtonfolds the other-workspace unread count into the button (chevron + 5pt spacing + an 18pt-minimum mono badge circle, wider for "99+"), rendering ~24-32pt wider than the bare chevron. With a badge present the title over-claimed by exactly that amount, the bar over-committed on the first pass, and the picker folded into More. A collapse born on the first pass never emits the attach-then-detach signature that the #10620 recovery ratchet watches, so it stuck until remount, which is why it read as rare and self-healing.Fix: the leading reserve now scales with the same
unreadCountthe button renders: +24pt for a badge, +32pt for the "99+" cap. Deterministic, no measurement or timing involved. Regression tests pin both deltas and the no-back-button case.Verified on an isolated iOS 27.0 simulator with a staged unread badge (second workspace receiving output): badged mount keeps back, title, Changes, and picker all in the bar.
No user-facing strings changed (localization audit: none needed). HIG reference: Navigation bars (https://developer.apple.com/design/human-interface-guidelines/navigation-bars), controls remain visible rather than overflowing.
🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by CodeRabbit
Bug Fixes
Tests