Repository navigation
iOS: tighter toolbar title reserve, pinned nav bar on scrollable surfaces, workspace-titled browser pill #10620
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
fcee00c
iOS: tighten the toolbar title's trailing reserve
azooz2003-bit 9ed6520
Revert "iOS: tighten the toolbar title's trailing reserve"
azooz2003-bit 35b45a9
iOS: size the toolbar title from the measured bar gap
azooz2003-bit 9a3d8c1
iOS: validate the measured bar gap by plausibility, not width stamps
azooz2003-bit 9a8a9ab
iOS: measure toolbar geometry in window space, not per-item islands
azooz2003-bit 131e8b9
iOS: keep onGeometryChange widths; window probe only supplies edges
azooz2003-bit 4efb299
iOS: trim toolbar title reserves 12pt with a collapse-recovery ratchet
azooz2003-bit 8b5b5a4
iOS: pin the workspace-detail navigation bar on scrollable surfaces
azooz2003-bit 4696dfb
iOS: browser-style surfaces title as workspace name, page as subtitle
azooz2003-bit 4ad4f2d
Merge remote-tracking branch 'origin/feat-ios-toolbar-ellipsis' into …
azooz2003-bit cb4e707
Merge remote-tracking branch 'origin/main' into feat-ios-toolbar-comb…
azooz2003-bit 5a6e8e1
iOS: gate the collapse ratchet on content presence; harden the bar pin
azooz2003-bit File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
119 changes: 119 additions & 0 deletions
119
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobilePinnedNavigationBar.swift
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,119 @@ | ||
| import SwiftUI | ||
| #if canImport(UIKit) | ||
| import UIKit | ||
| #endif | ||
|
|
||
| extension View { | ||
| /// Keeps the navigation bar fully expanded over scrollable content. | ||
| /// | ||
| /// iOS 26 minimizes the navigation bar into a floating overflow pill when | ||
| /// the content under it scrolls (Safari-style), which hides the back | ||
| /// button, the workspace title, and the trailing controls behind a "…" | ||
| /// menu on the browser and chat surfaces. The SwiftUI opt-out | ||
| /// (`toolbarMinimizeBehavior(.never)`) does not exist in the iOS 26 SDK, | ||
| /// and UIKit's 26 SDK only exposes a minimize behavior for the tab bar. | ||
| /// The bar's scroll linkage does have a public control: the view | ||
| /// controller's top-edge content scroll view (iOS 15+), which UIKit | ||
| /// otherwise discovers automatically (the browser pane's web view). | ||
| /// Re-pointing that association at a static, never-scrolling stand-in | ||
| /// decouples the bar from the content, so it stays expanded exactly like | ||
| /// the terminal surface, where no system scroll view exists to discover. | ||
| /// | ||
| /// On iOS 27 the native opt-out is applied as well once an Xcode 27 | ||
| /// toolchain builds this target. | ||
| @ViewBuilder | ||
| func mobilePinnedNavigationBar() -> some View { | ||
| #if canImport(UIKit) | ||
| #if compiler(>=6.4) | ||
| if #available(iOS 27.0, *) { | ||
| background(PinnedNavigationBarApplier()) | ||
| .toolbarMinimizeBehavior(.never, for: .navigationBar) | ||
| } else { | ||
| background(PinnedNavigationBarApplier()) | ||
| } | ||
| #else | ||
| background(PinnedNavigationBarApplier()) | ||
| #endif | ||
| #else | ||
| self | ||
| #endif | ||
| } | ||
| } | ||
|
|
||
| #if canImport(UIKit) | ||
| private struct PinnedNavigationBarApplier: UIViewRepresentable { | ||
| func makeUIView(context: Context) -> PinnedNavigationBarProbeView { | ||
| let view = PinnedNavigationBarProbeView() | ||
| view.isUserInteractionEnabled = false | ||
| view.backgroundColor = .clear | ||
| return view | ||
| } | ||
|
|
||
| func updateUIView(_ view: PinnedNavigationBarProbeView, context: Context) { | ||
| view.applyIfNeeded() | ||
| } | ||
|
|
||
| static func dismantleUIView(_ view: PinnedNavigationBarProbeView, coordinator: ()) { | ||
| view.clearAssociation() | ||
| } | ||
| } | ||
|
|
||
| final class PinnedNavigationBarProbeView: UIView { | ||
| /// The stand-in the bar tracks instead of the real content: zero content, | ||
| /// scrolling disabled, so the bar's scroll edge never moves. | ||
| private let anchorScrollView: UIScrollView = { | ||
| let scrollView = UIScrollView() | ||
| scrollView.isScrollEnabled = false | ||
| return scrollView | ||
| }() | ||
| /// The controller currently pointed at the stand-in, so teardown can | ||
| /// release the association instead of leaving the bar tracking a | ||
| /// deallocated probe's scroll view. | ||
| private weak var appliedTarget: UIViewController? | ||
|
|
||
| override func didMoveToWindow() { | ||
| super.didMoveToWindow() | ||
| applyIfNeeded() | ||
| } | ||
|
|
||
| override func layoutSubviews() { | ||
| super.layoutSubviews() | ||
| // Re-assert after layout passes: SwiftUI can re-derive the tracked | ||
| // scroll view for its own containers, and the association resets when | ||
| // the hosting controller re-parents during navigation transitions. | ||
| applyIfNeeded() | ||
| } | ||
|
|
||
| func applyIfNeeded() { | ||
| guard window != nil, let target = barOwningViewController() else { return } | ||
| if target.contentScrollView(for: .top) !== anchorScrollView { | ||
| target.setContentScrollView(anchorScrollView, for: .top) | ||
| appliedTarget = target | ||
| } | ||
| } | ||
|
|
||
| func clearAssociation() { | ||
| guard let target = appliedTarget, | ||
| target.contentScrollView(for: .top) === anchorScrollView | ||
| else { return } | ||
| target.setContentScrollView(nil, for: .top) | ||
| appliedTarget = nil | ||
| } | ||
|
|
||
| /// The pushed screen's view controller: the ancestor whose parent is the | ||
| /// navigation controller (its navigation item drives the bar). Fails | ||
| /// closed when no such ancestor exists (previews, sheets), rather than | ||
| /// re-pointing an unrelated controller's scroll edge. | ||
| private func barOwningViewController() -> UIViewController? { | ||
| var responder: UIResponder? = next | ||
| while let current = responder { | ||
| if let viewController = current as? UIViewController, | ||
| viewController.parent is UINavigationController { | ||
| return viewController | ||
| } | ||
| responder = current.next | ||
| } | ||
| return nil | ||
| } | ||
| } | ||
| #endif | ||
136 changes: 128 additions & 8 deletions
136
...ages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TrailingToolbarItemMeasurement.swift
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,23 +1,143 @@ | ||
| import SwiftUI | ||
| #if canImport(UIKit) | ||
| import UIKit | ||
| #endif | ||
|
|
||
| /// Live, reference-typed presence flags shared between probe islands. UIKit | ||
| /// callbacks read and write these directly at event time; SwiftUI closure | ||
| /// captures of value state would go stale between renders and misreport | ||
| /// during navigation transitions. | ||
| @MainActor | ||
| final class WorkspaceBarPresence { | ||
| /// Whether the detail screen's content view is attached to a window. | ||
| /// False during whole-screen detachments (a deeper push, a pop, scene | ||
| /// teardown), when toolbar probes also detach for lifecycle reasons that | ||
| /// have nothing to do with the overflow More menu. | ||
| var detailContentAttached = false | ||
| } | ||
|
|
||
| extension View { | ||
| /// Tracks whether this view's content is attached to a UIKit window, | ||
| /// writing into the shared presence object in real time. | ||
| @ViewBuilder | ||
| func trackBarPresence(_ presence: WorkspaceBarPresence) -> some View { | ||
| #if canImport(UIKit) | ||
| background(ContentPresenceReader(presence: presence)) | ||
| #else | ||
| self | ||
| #endif | ||
| } | ||
|
|
||
| /// Reports this trailing toolbar item's rendered content width into the | ||
| /// shared measurement dictionary. | ||
| /// shared measurement dictionary, and optionally reports when the item's | ||
| /// content leaves the window while still structurally present. | ||
| /// | ||
| /// The leading title menu caps its width to the bar's remaining space so | ||
| /// the trailing items are never squeezed into the overflow More menu. | ||
| /// Deliberately no `onDisappear` cleanup: overflowing into More also | ||
| /// removes the bar content, so clearing on disappear would release the | ||
| /// reservation and make the collapse sticky. Callers that structurally | ||
| /// remove an item clear its key from the condition that removed it. | ||
| /// the trailing items are never squeezed into the More menu. Width | ||
| /// entries are deliberately never removed on disappear: overflowing into | ||
| /// More also removes the bar content, so clearing the width there would | ||
| /// release the reservation and make the collapse sticky. Callers that | ||
| /// structurally remove an item clear its key from the condition that | ||
| /// removed it. | ||
| /// | ||
| /// `onLeaveBar` is only wired for items that are always structurally | ||
| /// present, and the callback must gate on the screen's content still | ||
| /// being attached (`WorkspaceBarPresence`): a whole-screen detachment | ||
| /// (deeper push, pop) also detaches this probe and is indistinguishable | ||
| /// from a More-menu collapse at this level. | ||
| @ViewBuilder | ||
| func measureTrailingToolbarItem( | ||
| _ key: String, | ||
| into widths: Binding<[String: CGFloat]> | ||
| into widths: Binding<[String: CGFloat]>, | ||
| onLeaveBar: (@MainActor () -> Void)? = nil | ||
| ) -> some View { | ||
| onGeometryChange(for: CGFloat.self) { proxy in | ||
| let measured = onGeometryChange(for: CGFloat.self) { proxy in | ||
| proxy.size.width | ||
| } action: { width in | ||
| widths.wrappedValue[key] = width | ||
| } | ||
| #if canImport(UIKit) | ||
| measured.background(BarPresenceReader(probeKey: key, onLeaveBar: onLeaveBar)) | ||
| #else | ||
| measured | ||
| #endif | ||
| } | ||
| } | ||
|
|
||
| #if canImport(UIKit) | ||
| private struct ContentPresenceReader: UIViewRepresentable { | ||
| let presence: WorkspaceBarPresence | ||
|
|
||
| func makeUIView(context: Context) -> ContentPresenceProbeView { | ||
| let view = ContentPresenceProbeView() | ||
| view.isUserInteractionEnabled = false | ||
| view.backgroundColor = .clear | ||
| view.presence = presence | ||
| return view | ||
| } | ||
|
|
||
| func updateUIView(_ view: ContentPresenceProbeView, context: Context) { | ||
| view.presence = presence | ||
| } | ||
| } | ||
|
|
||
| final class ContentPresenceProbeView: UIView { | ||
| var presence: WorkspaceBarPresence? | ||
|
|
||
| override func didMoveToWindow() { | ||
| super.didMoveToWindow() | ||
| let attached = window != nil | ||
| MainActor.assumeIsolated { | ||
| presence?.detailContentAttached = attached | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// Bridges out of the SwiftUI hosting island to observe whether the item's | ||
| /// content is attached to a UIKit window. Detaching after having been | ||
| /// attached, while the screen's own content stays on a window, is the | ||
| /// observable signature of the system moving the item into the overflow More | ||
| /// menu; the caller supplies that content-presence gate. | ||
| private struct BarPresenceReader: UIViewRepresentable { | ||
| let probeKey: String | ||
| let onLeaveBar: (@MainActor () -> Void)? | ||
|
|
||
| func makeUIView(context: Context) -> BarPresenceProbeView { | ||
| let view = BarPresenceProbeView() | ||
| view.isUserInteractionEnabled = false | ||
| view.backgroundColor = .clear | ||
| view.probeKey = probeKey | ||
| view.onLeaveBar = onLeaveBar | ||
| return view | ||
| } | ||
|
|
||
| func updateUIView(_ view: BarPresenceProbeView, context: Context) { | ||
| view.onLeaveBar = onLeaveBar | ||
| } | ||
| } | ||
|
|
||
| final class BarPresenceProbeView: UIView { | ||
| var probeKey = "" | ||
| var onLeaveBar: (@MainActor () -> Void)? | ||
| private var wasAttached = false | ||
|
|
||
| override func didMoveToWindow() { | ||
| super.didMoveToWindow() | ||
| #if DEBUG | ||
| NSLog( | ||
| "cmux.toolbar.collapse probe key=%@ attached=%d", | ||
| probeKey, window == nil ? 0 : 1 | ||
| ) | ||
| #endif | ||
| if window != nil { | ||
| wasAttached = true | ||
| return | ||
| } | ||
| guard wasAttached else { return } | ||
| wasAttached = false | ||
| MainActor.assumeIsolated { | ||
| onLeaveBar?() | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| } | ||
| } | ||
| #endif | ||
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
Repository: manaflow-ai/cmux
Length of output: 26566
🏁 Script executed:
Repository: manaflow-ai/cmux
Length of output: 36829
🏁 Script executed:
Repository: manaflow-ai/cmux
Length of output: 50373
🌐 Web query:
Apple UIKit UIViewController setContentScrollView(_:for:) contentScrollView(for:) documentation behavior navigation bar scroll edge effect💡 Result:
In UIKit, navigation bars, toolbars, and tab bars automatically adjust their appearance based on the scroll position of a content scroll view [1][2]. Specifically, when the edge of a scroll view's content aligns with the edge of a bar, the bar switches to its scrollEdgeAppearance [3][4]. UIKit attempts to identify the appropriate scroll view to observe by analyzing the view hierarchy [1]. If the hierarchy is complex or UIKit fails to automatically select the correct scroll view, you can manually specify it or override the behavior using the following UIViewController methods: setContentScrollView(_:for:): This method explicitly sets the UIScrollView that bars should observe for a specific NSDirectionalRectEdge (top, bottom, or all) [1][5]. If you pass nil, the view controller will revert to its default behavior of automatically determining the scroll view [1]. contentScrollView(for:): This method returns the scroll view currently being observed for the specified edge [2][6]. You can override this method to return nil if you wish to disable the scroll edge appearance behavior for a specific edge [2]. These APIs were introduced to provide developers with control over the "scroll edge effect" in scenarios where automatic detection is insufficient [7][4]. For example, if you want to prevent a navigation bar from transitioning to its scrollEdgeAppearance, you can override contentScrollView(for:) to return nil [2]. Alternatively, ensuring that both standardAppearance and scrollEdgeAppearance are configured to the same UINavigationBarAppearance object will cause the bar to maintain a consistent look regardless of the scroll state [4][8][9].
Citations:
🏁 Script executed:
Repository: manaflow-ai/cmux
Length of output: 14023
🏁 Script executed:
Repository: manaflow-ai/cmux
Length of output: 50372
🏁 Script executed:
Repository: manaflow-ai/cmux
Length of output: 28939
🏁 Script executed:
Repository: manaflow-ai/cmux
Length of output: 977
🌐 Web query:
site:developer.apple.com/documentation/uikit/uiviewcontroller/setcontentscrollview "_contentScrollView" retain lifecycle nil💡 Result:
Apple’s documentation does not mention the private ivar
_contentScrollView, its retain behavior, lifecycle, or whether it is cleared tonil.What the public API documents:
setContentScrollView(_:for:)stores the scroll view UIKit observes; passingnilcauses automatic selection. (developer.apple.com)setContentScrollView(_:)is equivalent to setting it for.all. (developer.apple.com)contentScrollView(for:)returnsnilby default if no scroll view was explicitly set. (developer.apple.com)Therefore,
_contentScrollViewis an implementation detail. Don’t rely on its ownership or lifecycle; use the documented setter/getter APIs.Citations:
🏁 Script executed:
Repository: manaflow-ai/cmux
Length of output: 696
Make the navigation controller the sole owner of this association.
applyIfNeeded()runs fromupdateUIView,didMoveToWindow, andlayoutSubviews, so layout becomes a repair path for controller state.barOwningViewController()also targets the nearest responder when no navigation ancestor exists, which can assign the edge to the wrong controller. Move installation to an explicit navigation/controller owner, fail closed when it is unavailable, and clear the association withsetContentScrollView(nil, for: .top)during teardown.🤖 Prompt for AI Agents
Sources: Coding guidelines, Path instructions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in 5a6e8e1e73: the ratchet now latches only while the screen's content view is still window-attached (event-time read through a shared reference object), so navigation transitions cannot false-positive; the pin applier fails closed without a navigation-controller ancestor and clears its content-scroll-view association on dismantle.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Length of output: 1763
🏁 Script executed:
Length of output: 10518
@azooz2003-bit, the fail-closed owner selection and teardown cleanup are present.However, the lifecycle ownership issue remains.
updateUIView,didMoveToWindow, andlayoutSubviewsstill callapplyIfNeeded(). In particular,layoutSubviewsstill reassigns the navigation controller’s scroll-view association as a repair path.Please remove the update- and layout-driven reassociation. Install the association through an explicit navigation/controller owner. Keep the existing fail-closed selection and
dismantleUIViewcleanup.You are interacting with an AI system.