Repository navigation
Fix iOS chat top scroll edge blend - #6910
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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:
📝 WalkthroughWalkthroughThe chat screen now uses a dedicated scroll-edge coordinator, updated transcript viewport inset handling, and iOS-versioned top-edge layout helpers. UI tests and transcript metrics were extended to capture the new top-edge behavior. ChangesiOS chat scroll-edge coordination
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 24 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (24 passed)
✨ Finishing Touches📝 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 |
Contributor
Greptile SummaryImplements iOS 26 native top scroll-edge blending for the chat transcript by extending the transcript and composer under the navigation bar and device bottom chrome, then centralizing all
Confidence Score: 5/5The change is safe to merge — all scroll-edge, inset, and underlap paths are guarded by iOS 26 availability checks, pre-iOS 26 behavior is unchanged, and the two previously identified coordinator bugs are correctly resolved in this revision. The automaticBottomAdjustment subtraction correctly avoids double-counting the safe-area contribution on iOS 26. The wasPinnedToTop guard symmetrically mirrors the wasAtBottom restoration path. nearestNavigationContentViewController returns nil when no nav controller exists in the parent chain, and firstNavigationController no longer traverses presentedViewController — both previously flagged issues are fixed. The trackedTranscriptTables short-circuit prevents O(n) traversal into hosted table cells on every layout/keyboard update. No files require special attention. Important Files Changed
Reviews (11): Last reviewed commit: "Fix iOS chat policy findings" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
This comment has been minimized.
This comment has been minimized.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Screen/ChatKeyboardTrackingViewController.swift`:
- Around line 381-382: The geometry update path is rescanning transcript
subtrees because trackedTranscriptTables(in:) keeps descending even after it
finds a ChatTranscriptUITableView. Update trackedTranscriptTables(in:) in
ChatKeyboardTrackingViewController to short-circuit once the table is found, or
cache the table reference so the keyboard/layout update path in the caller does
not repeatedly walk visible cells and hosted row views.
In
`@Packages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Transcript/ChatTranscriptUITableView.swift`:
- Around line 117-123: When updating ChatTranscriptUITableView’s viewport after
a composer or bottom-inset change, the current conditional order lets the
bottom-restoration path run even when the transcript was pinned to the top,
which moves contentOffset.y away from the top inset. In the same viewport update
logic, handle wasPinnedToTop before the snapshot.wasAtBottom || bottomChanged
branch, and force the pinned-top offset using setClampedContentOffsetY with
-adjustedContentInset.top. Keep restoreKeyboardViewport(snapshot) only for the
non-pinned cases so the top position is preserved.
🪄 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: a7f58844-2c5f-4cb1-8ea2-cded2ce36367
📒 Files selected for processing (6)
Packages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Screen/ChatKeyboardTrackingViewController.swiftPackages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Screen/ChatScrollEdgeCoordinator.swiftPackages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Transcript/ChatTranscriptTableView.swiftPackages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Transcript/ChatTranscriptUITableView.swiftPackages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Transcript/Rows/ChatDateHeaderView.swiftios/cmuxUITests/cmuxUITests.swift
💤 Files with no reviewable changes (1)
- Packages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Screen/ChatScrollEdgeCoordinator.swift
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/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Screen/ChatScreen.swift`:
- Line 117: The top underlap is applied too broadly in ChatScreen, which causes
the `.overlay(alignment: .top)` error banner to inherit the underlapped region.
Update the `ChatKeyboardTrackingContainer` / `.chatTopBarUnderlapContainer()`
usage so only the transcript-hosting subtree ignores the top safe area, or
explicitly restore the top safe-area offset for the overlay path, keeping the
error toast below the navigation bar.
🪄 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: 12419394-179a-4be0-9e32-ab6eed8224bf
📒 Files selected for processing (6)
Packages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Screen/ChatKeyboardTrackedRoot.swiftPackages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Screen/ChatKeyboardTrackingContainer.swiftPackages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Screen/ChatKeyboardTrackingViewController.swiftPackages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Screen/ChatScreen.swiftPackages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Screen/ChatScrollEdgeCoordinator.swiftios/cmuxUITests/cmuxUITests.swift
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Packages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Transcript/ChatTranscriptTableView.swift (1)
290-307: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winDon’t expand the UI-test scroll seam in production source.
Adding the
"top"mode makesCMUX_UITEST_CHAT_INITIAL_SCROLLa broader DEBUG/test-only behavior inside a productionSources/Swift file. Drive the new evidence test to top via XCUI scrolling/metrics instead of adding another app-code test mode. As per path instructions,**/Sources/**/*.swift: “do not add test-only or debug-only seams in production Swift source files.” As per coding guidelines, production Swift changes must not add test/debug seams in production source.🤖 Prompt for 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. In `@Packages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Transcript/ChatTranscriptTableView.swift` around lines 290 - 307, The initial-scroll handling in ChatTranscriptTableView should not gain a new test-only “top” path inside production Sources code. Remove the added CMUX_UITEST_CHAT_INITIAL_SCROLL “top” branch from the debug initial scroll logic in ChatTranscriptTableView and keep the app code limited to existing behavior, then update the UI test to scroll the transcript to the top using XCUI interactions or metrics instead of relying on a new app-side seam.Sources: Coding guidelines, Path instructions
🤖 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.
Outside diff comments:
In
`@Packages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Transcript/ChatTranscriptTableView.swift`:
- Around line 290-307: The initial-scroll handling in ChatTranscriptTableView
should not gain a new test-only “top” path inside production Sources code.
Remove the added CMUX_UITEST_CHAT_INITIAL_SCROLL “top” branch from the debug
initial scroll logic in ChatTranscriptTableView and keep the app code limited to
existing behavior, then update the UI test to scroll the transcript to the top
using XCUI interactions or metrics instead of relying on a new app-side seam.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: dcf7c513-0cd9-476f-b106-03496c04f716
📒 Files selected for processing (2)
Packages/iOS/CmuxAgentChatUI/Sources/CmuxAgentChatUI/Transcript/ChatTranscriptTableView.swiftios/cmuxUITests/cmuxUITests.swift
Greptile P1s (ChatScrollEdgeCoordinator): - nearestNavigationContentViewController now returns nil when no UINavigationController exists in the parent chain, so the caller falls back to `owner` instead of installing top scroll-edge state on an unrelated root controller. - firstNavigationController no longer descends into presentedViewController, so the chat's top scroll-edge interaction can't attach to a navigation bar inside an unrelated modal layered above the chat. CodeRabbit: - trackedTranscriptTables short-circuits at the transcript table instead of re-walking its cells/hosted rows on every geometry update. - applyTranscriptViewportInsets handles wasPinnedToTop first, the symmetric counterpart to wasAtBottom, so a composer/bottom-inset change can't drift the first row back under the toolbar. - The chat error toast is now a ZStack sibling that respects the top safe area instead of an overlay on the underlapped layout, so on iOS 26 it renders below the navigation bar rather than under it. - Removed the new test-only CMUX_UITEST_CHAT_INITIAL_SCROLL "top" seam from production source (restores the pre-existing "middle"-only behavior) per no-test-debug-seam-in-production-source; the evidence UI test now drives the transcript to the top with XCUI scrolling instead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
Addressed CodeRabbit's outside-diff note on |
azooz2003-bit
force-pushed
the
feat-ios-top-scroll-edge
branch
from
June 27, 2026 03:29
e6235f9 to
ba2251e
Compare
azooz2003-bit
force-pushed
the
feat-ios-top-scroll-edge
branch
from
June 27, 2026 03:37
ba2251e to
133ce84
Compare
This was referenced Jun 27, 2026
This branch was successfully deployed
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
2 participants
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
Testing
Notes
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Fixes iOS 26 chat scroll-edge blending at the top and bottom by underlapping the navigation bar and device bottom and coordinating native soft-edge effects. Keeps the “Today” header visible, preserves pin at top/bottom, keeps the error toast below the bar, ensures the scroll-to-bottom button clears the floating composer, and avoids double-counting the bottom safe area on iOS 26; older iOS keep legacy top padding.
ChatScrollEdgeCoordinatorto apply soft edges, register the transcript as the top content scroll view with the nearest navigation owner (fallback to the owner), attach a bottom edge interaction to the composer, avoid unrelated roots/modals, ensure a single owner, and reset on detach..automaticinset adjustment on iOS 26; add.mobileChatTopScrollEdgeLayout(legacyTopPadding:)for pre‑iOS 26 hosts; move the error toast to a ZStack sibling so it renders below the bar.ChatTranscriptUITableView.applyTranscriptViewportInsets(topChromeInset:adjustedBottomInset:composerOverlayBottomInset:)to target the final adjusted bottom inset without double-counting UIKit’s safe-area addition, preserve pin-at-top when the bottom inset changes (and pin-at-bottom), clamp offsets, and exposeadjustedTopInset,adjustedBottomInset,visibleTopY, andtopChromeOverlayInset; renameisKeyboardViewportExternallyDriventoisViewportInsetsExternallyDriven.ChatTranscriptOverlayGeometryso the scroll-to-bottom button pads above the floating composer; add accessibility IDs toChatScrollToBottomButtonandChatDateHeader; short-circuit transcript table lookup to avoid walking hosted rows.Written for commit 6c989bc. Summary will update on new commits.
Summary by CodeRabbit
mobileChatTopScrollEdgeLayouthelper to preserve legacy top spacing on older iOS versions.