Repository navigation
iOS: native soft scroll edge effect under the workspace list chrome - #8575
Merged
Merged
Conversation
…effects The workspace list is a UIViewRepresentable UITableView, so SwiftUI never registers it as the bars' content scroll view and the .soft top edge style never renders: rows hard-clip at the search bar's bottom edge instead of soft-fading under the chrome like the App Store list. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…st chrome Register the workspace UITableView as the content scroll view of its enclosing navigation and tab bar controllers (setContentScrollView top / bottom, iOS 26-gated) from didMoveToWindow, with a layoutSubviews retry until the controller parent chain is assembled. UIKit then renders the table's existing .soft top edge effect under the navigation bar + search drawer and drives the tab bar's bottom edge, matching the App Store's soft blur instead of hard-clipping rows at the search bar boundary. Unregistration on window removal only clears registrations this table still owns, so a replacement table's registration is never clobbered. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an iOS 26 scroll-edge coordinator for ChangesWorkspace list UI integration
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant SwiftUIHost
participant WorkspaceListUITableView
participant WorkspaceListScrollEdgeCoordinator
participant UINavigationController
participant UITabBarController
SwiftUIHost->>WorkspaceListUITableView: Host table view
WorkspaceListUITableView->>WorkspaceListScrollEdgeCoordinator: Register after window/layout attachment
WorkspaceListScrollEdgeCoordinator->>UINavigationController: Register table as bottom content scroll view
WorkspaceListScrollEdgeCoordinator->>UITabBarController: Register table as top content scroll view
WorkspaceListUITableView->>WorkspaceListScrollEdgeCoordinator: Unregister when leaving window
WorkspaceListScrollEdgeCoordinator->>UINavigationController: Clear matching registration
WorkspaceListScrollEdgeCoordinator->>UITabBarController: Clear matching registration
Possibly related PRs
🚥 Pre-merge checks | ✅ 24 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (24 passed)
✨ Finishing Touches🧪 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 |
Contributor
Greptile SummaryThis PR adds native iOS 26 scroll-edge effects to the workspace list. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (9): Last reviewed commit: "Apply review policy: instance helpers an..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Collaborator
Author
|
Verification on isolated sim cmux-sedge-sim (iPhone 17, iOS 26.5): Red (98dd7c1, tests only): hostedTableDrivesNavigationAndTabBarScrollEdgeEffects and departingTableDoesNotClobberReplacementRegistration fail, contentScrollView(for:) returns nil. Note: CI's swift-package-tests job runs macOS Visual before/after with the workspace-list preview fixture (30 seeded rows, scrolled): before, the row under the search bar renders at full opacity and hard-clips at the bar's bottom edge; after, it progressively fades under the toolbar + search chrome, matching the App Store treatment. Also reproduced against a live paired tagged Mac with real workspaces. |
…dge effect renders
SwiftUI fits a UIViewRepresentable inside the safe area, so the table's
frame started below the search drawer and ended above the tab bar
(AX-verified frame {0,176,402,664} on a 402x874 screen): rows hard-clipped
at the table's own bounds and the scroll edge effect had no covered region
to render into. ignoresSafeArea(.container, edges: .vertical) restores the
native underlap; the real UIKit bars still contribute safe area, so
automatic content-inset adjustment keeps rows and indicators clear of the
chrome, and the soft edge effect + bar registration now render the App
Store-style progressive blur at both edges.
Also adds CMUX_UITEST_WORKSPACE_LIST_PREVIEW_TABS=1 to the DEBUG preview
fixture, wrapping the list in a TabView so the floating tab bar's bottom
edge can be dogfooded without Mac pairing. Off by default to keep the App
Store screenshot rig's chrome unchanged.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Collaborator
Author
|
Round 2 (3300379): the registration alone was not enough. The real root cause of the hard edges: SwiftUI fits a UIViewRepresentable inside the safe area, so the table's frame started below the search drawer and ended above the tab bar (AX-verified frame {0,176,402,664} on a 402x874 screen). Content clipped at the table's own bounds, leaving the edge effect no covered region to render into. Fix: Also added |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListLayoutPreviewView.swift`:
- Around line 313-325: Update the Tab labels and notification fixture text in
WorkspaceListLayoutPreviewView to use the repository’s localized API instead of
bare string literals. Reuse the existing mobile.workspaces.title catalog key for
“Workspaces” and add catalog-backed keys/default values for the “Notifications”
label and “Notification feed fixture” text, including matching localization
entries.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5cd0610d-3252-49a8-bbb1-4688928d40a5
📒 Files selected for processing (2)
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListLayoutPreviewView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView.swift
A one-shot registration flag stranded a partially assembled hierarchy: if the navigation controller hosted the table before joining the tab bar controller, the top-only registration cleared the retry flag and the bottom edge never registered (Greptile P1). Re-resolving each layout pass also covers reparenting that never changes the window (compact stack to split sidebar); the coordinator's identity guard makes the repeated call a no-op when nothing changed. Adds a regression test attaching the tab bar controller after the first registration. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListScrollEdgeCoordinator.swift`:
- Line 34: Update the guard in the workspace scroll-edge coordinator to call
unregister() before returning when both navigationContent and tabContent are
nil. Preserve the existing early-return behavior while clearing stale top/bottom
content-scroll-view bindings after reparenting.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 97a83b70-cf4f-4f52-906f-57cd2d93a22f
📒 Files selected for processing (3)
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListScrollEdgeCoordinator.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListUITableView.swiftPackages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/WorkspaceListScrollEdgeEffectTests.swift
The identity guard trusted the coordinator's cache, so when a transient replacement table took the registration over and cleared it on departure, the surviving table saw unchanged targets and never re-registered, leaving both edges dead until the hierarchy changed. The guard now also compares the controllers' effective contentScrollView(for:) against this table, so the next layout pass self-heals the reverse handoff. Regression test covers replacement mount, takeover, departure, and reclaim. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListScrollEdgeCoordinator.swift`:
- Around line 35-50: Update the registration guard in the coordinator’s
layout/update method around topIsCurrent and bottomIsCurrent so a stale
coordinator cannot re-register while either edge is effectively owned by a
different attached scroll view. Treat the effective contentScrollView
registrations as authoritative, defer re-registration while another owner
remains attached, and allow reclaim only after that owner clears its
registration; retain existing coordinator/cache mismatch handling otherwise.
In
`@Packages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/WorkspaceListScrollEdgeEffectTests.swift`:
- Around line 67-76: Extend the replacement handoff test around the existing
top-edge assertions to also verify bottom-edge ownership: assert that
replacement is registered for .bottom before removal, then assert .bottom
returns nil after replacement.removeFromSuperview(). Keep the existing
post-layout assertions confirming fixture.tableView owns both .top and .bottom.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6953be54-0b28-4361-95a3-3d56f4832373
📒 Files selected for processing (2)
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListScrollEdgeCoordinator.swiftPackages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/WorkspaceListScrollEdgeEffectTests.swift
The previous self-heal re-registered whenever the effective registration differed from this table, so two live coexisting tables (SwiftUI transition overlap) stole ownership from each other on every layout pass, making bar ownership layout-order dependent. An edge is now claimed only when its registration is nil or held by a detached scroll view; a different live table keeps ownership, departure clears the edge, and the survivor reclaims on its next layout pass. Tests pin both the no-steal coexistence and the reclaim-after-departure handoffs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
…ercises UIKit's hierarchy-consistency check forbids re-parenting a controller whose view stays in a foreign hierarchy (attempting an addChild-based window-stable variant throws UIViewControllerHierarchyInconsistency), so a same-window assembly scenario is not constructible in a unit test; real container attachment always relocates views. The test now documents that it pins the end state through whichever lifecycle path fires, and points at survivingTableReclaimsRegistrationAfterOwnerDeparts as the window-stable layout-retry coverage. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
ignoresSafeArea applied on every supported release while the scroll edge registration is iOS 26-gated, so iOS 18-25 would scroll full-opacity rows beneath legacy bars with no edge effect. The underlap now lives in an availability-gated ViewModifier: iOS 26 gets the App Store treatment, earlier releases keep the fitted frame they have today. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
An overlapping successor stands down while the owner is live, and UIKit does not guarantee it another layout pass when the owner later departs, so both edge registrations could stay vacant until an unrelated scroll or resize. Clearing an edge now finds the remaining workspace table under the same controller and marks it needing layout; its next pass runs the normal claim arbitration. The reclaim test now only flushes pending window layout instead of dirtying the survivor by hand, so a missing wake fails it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
Converts the coordinator's pure static helpers to instance methods (the type holds state; statics on it read as namespace members) and moves WorkspaceListBarUnderlap into its own file per file-organization policy. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 22, 2026
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.
The workspace list never rendered a scroll edge effect: rows scrolled under the search bar at full opacity and hard-clipped, instead of soft-fading under the chrome like the App Store list.
Two root causes, one per commit pair:
The table is a
UIViewRepresentableUITableView, and SwiftUI only drives bar scroll edge effects for its own scroll views. A newWorkspaceListScrollEdgeCoordinator(same approach as the chat screen'sChatScrollEdgeCoordinator) registers the table as the content scroll view of its enclosing bar controllers viasetContentScrollView(_:for:), resolved by walking from the table to the last view controller before the nearestUINavigationController(top) andUITabBarController(bottom), iOS 26-gated. Registration happens indidMoveToWindowwith alayoutSubviewsretry; unregistration only clears registrations the departing table still owns.SwiftUI fits a representable inside the safe area, so the table's frame started below the search drawer and ended above the tab bar; content clipped at the table's own bounds and the effect had no covered region to render into.
ignoresSafeArea(.container, edges: .vertical)restores the native underlap; the real UIKit bars still contribute safe area, so automatic content-inset adjustment keeps rows and indicators clear of the chrome.Result: App Store-style progressive soft blur under the toolbar + search bar and behind the floating tab bar, verified in dark and light mode on an isolated iPhone 17 sim (iOS 26.5) with the preview fixture and against a live paired tagged Mac. Red/green commit structure: the registration tests land first and fail, the fix follows.
The DEBUG preview fixture gains
CMUX_UITEST_WORKSPACE_LIST_PREVIEW_TABS=1to wrap the list in a TabView so the tab bar's bottom edge is dogfoodable without Mac pairing (off by default; the App Store screenshot rig keeps its bare chrome).🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by CodeRabbit