Skip to content

fix(mobile): align diff scrolling with glass headers #17085

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.

Sign up for GitHub

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
juliusmarminge merged 1 commit into from
Oct 8, 2026
Merged

fix(mobile): align diff scrolling with glass headers #17085

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 18 additions & 11 deletions apps/mobile/modules/t3-review-diff/ios/T3ReviewDiffView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -563,14 +563,15 @@ public final class T3ReviewDiffView: ExpoView, UIScrollViewDelegate {
])
}

// Source files fill the screen so UIKit can sample content below the glass bar.
// Code surfaces fill the screen so UIKit can sample content below the glass bar.
// Keep the same visible row when header or safe-area reservations change.
func setContentInsetTop(_ inset: CGFloat) {
scrollView.automaticallyAdjustsScrollIndicatorInsets = false
let inset = max(0, inset)
let delta = inset - scrollView.contentInset.top
guard delta != 0 else { return }
scrollView.contentInset.top = inset
contentView.topContentInset = inset
scrollView.verticalScrollIndicatorInsets.top = inset
scrollView.contentOffset.y -= delta
updateViewportFrame()
Expand Down Expand Up @@ -618,8 +619,12 @@ public final class T3ReviewDiffView: ExpoView, UIScrollViewDelegate {
if let scrollAnchor,
let headerOffset = contentView.fileHeaderOffset(forFileId: scrollAnchor.fileId) {
let targetOffset = headerOffset - scrollAnchor.screenY
let maxOffset = max(scrollView.contentSize.height - scrollView.bounds.height, 0)
let clampedOffset = min(max(targetOffset, 0), maxOffset)
let minOffset = -scrollView.contentInset.top
let maxOffset = max(
scrollView.contentSize.height - scrollView.bounds.height + scrollView.contentInset.bottom,
minOffset
)
let clampedOffset = min(max(targetOffset, minOffset), maxOffset)
scrollView.setContentOffset(CGPoint(x: 0, y: clampedOffset), animated: false)
updateViewportFrame()
}
Expand Down Expand Up @@ -735,7 +740,8 @@ public final class T3ReviewDiffView: ExpoView, UIScrollViewDelegate {
// The top of the combined diff is the explicit "All files" destination.
// Treat it as a first-class selection instead of immediately resolving the
// first file header and undoing the navigator's optimistic selection.
if scrollView.contentOffset.y <= 0.5 {
let visibleTop = scrollView.contentOffset.y + scrollView.contentInset.top
if visibleTop <= 0.5 {
guard lastVisibleFileId != nil else {
return
}
Expand All @@ -744,7 +750,7 @@ public final class T3ReviewDiffView: ExpoView, UIScrollViewDelegate {
return
}

guard let fileId = contentView.visibleFileId(atVerticalOffset: scrollView.contentOffset.y),
guard let fileId = contentView.visibleFileId(atVerticalOffset: visibleTop),
fileId != lastVisibleFileId else {
return
}
Expand Down Expand Up @@ -1025,6 +1031,7 @@ private final class ReviewDiffContentView: UIView, UIGestureRecognizerDelegate {
}
}
var verticalOffset: CGFloat = 0
var topContentInset: CGFloat = 0
var theme = ReviewDiffNativeTheme.resolve("light") {
didSet {
tokenColorsByHex.removeAll()
Expand Down Expand Up @@ -1495,7 +1502,7 @@ private final class ReviewDiffContentView: UIView, UIGestureRecognizerDelegate {

let rowIndex = stickyHeader.rowIndex
let headerTop = rowOffsets[rowIndex]
guard headerTop < verticalOffset else {
guard headerTop < verticalOffset + topContentInset else {
return nil
}

Expand All @@ -1505,12 +1512,12 @@ private final class ReviewDiffContentView: UIView, UIGestureRecognizerDelegate {
: nil
let pushedY: CGFloat
if let nextHeaderRowIndex {
pushedY = min(0, rowOffsets[nextHeaderRowIndex] - verticalOffset - style.fileHeaderHeight)
pushedY = min(topContentInset, rowOffsets[nextHeaderRowIndex] - verticalOffset - style.fileHeaderHeight)
} else {
pushedY = 0
pushedY = topContentInset
}

guard pushedY > -style.fileHeaderHeight else {
guard pushedY > topContentInset - style.fileHeaderHeight else {
return nil
}

Expand All @@ -1533,7 +1540,7 @@ private final class ReviewDiffContentView: UIView, UIGestureRecognizerDelegate {
while lowerBound < upperBound {
let midpoint = (lowerBound + upperBound) / 2
let rowIndex = fileHeaderRowIndices[midpoint]
if rowOffsets[rowIndex] <= verticalOffset {
if rowOffsets[rowIndex] <= verticalOffset + topContentInset {
lowerBound = midpoint + 1
} else {
upperBound = midpoint
Expand All @@ -1550,7 +1557,7 @@ private final class ReviewDiffContentView: UIView, UIGestureRecognizerDelegate {
func scrollAnchor(forFileId fileId: String) -> ReviewDiffScrollAnchor? {
if let stickyHeader = stickyFileHeaderTarget(),
resolvedFileId(for: stickyHeader.row) == fileId {
return ReviewDiffScrollAnchor(fileId: fileId, screenY: 0)
return ReviewDiffScrollAnchor(fileId: fileId, screenY: topContentInset)
}

guard let headerOffset = fileHeaderOffset(forFileId: fileId) else {
Expand Down
5 changes: 2 additions & 3 deletions apps/mobile/src/Stack.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,7 @@ const GLASS_HEADER_OPTIONS: AppScreenOptions = {
unstable_navigationItemStyle: NATIVE_LIQUID_GLASS_SUPPORTED ? "editor" : undefined,
};

// SOLID: opaque sheet-colored header for surfaces whose content scrolls internally
// (terminal, review) — there is nothing for glass to sample there.
// SOLID: opaque sheet-colored header for surfaces that manage their own viewport.
const SOLID_HEADER_OPTIONS: AppScreenOptions = {
headerBackButtonDisplayMode: "minimal",
headerBackTitle: "",
Expand Down Expand Up @@ -724,7 +723,7 @@ const RootStackConfig = createWorkspaceStackNavigator({
ThreadReview: createNativeStackScreen({
screen: ReviewSheet,
linking: `${THREAD_LINKING_PREFIX}/review`,
options: SOLID_HEADER_OPTIONS,
options: GLASS_HEADER_OPTIONS,
}),
ThreadReviewComment: createNativeStackScreen({
screen: ReviewCommentComposerSheet,
Expand Down
23 changes: 12 additions & 11 deletions apps/mobile/src/features/review/ReviewSheet.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import {
View,
} from "react-native";
import { useSafeAreaInsets } from "react-native-safe-area-context";
import { useNativeColumnLayoutMetrics } from "../../native/native-layout-metrics";

import { AppText as Text } from "../../components/AppText";
import { SymbolView } from "../../components/AppSymbol";
Expand Down Expand Up @@ -156,7 +157,6 @@ function ReviewHeader(
);
}

const REVIEW_HEADER_SPACING = 0;
const SHOWCASE_ENABLED = process.env.EXPO_PUBLIC_SHOWCASE === "1";

const ReviewNotice = memo(function ReviewNotice(props: { readonly notice: string }) {
Expand Down Expand Up @@ -461,10 +461,11 @@ export function ReviewSheet(props: ReviewSheetProps) {
const { draftMessage } = useThreadDraftForThread({ environmentId, threadId });
const reviewCache = useReviewCacheForThread({ environmentId, threadId });
const { selectedThreadCwd } = useSelectedThreadWorktree();
// With a solid (non-overlay) header the content lays out below the header
// natively, so no manual top inset is needed. (Android renders its own
// in-flow AndroidScreenHeader, so it needs no inset either.)
const topContentInset = 0;
const columnMetrics = useNativeColumnLayoutMetrics();
const safeArea = columnMetrics?.safeArea ?? insets;
const topContentInset = Platform.OS === "ios" ? safeArea.top : 0;
const bottomContentInset =
Platform.OS === "ios" ? safeArea.bottom + 8 : Math.max(insets.bottom, 18) + 18;

useEffect(() => {
showAuxiliaryPane("inspector");
Expand Down Expand Up @@ -760,7 +761,7 @@ export function ReviewSheet(props: ReviewSheetProps) {
>
<View
className="min-w-0 flex-1"
style={{ paddingTop: topContentInset + REVIEW_HEADER_SPACING }}
style={listHeader ? { paddingTop: topContentInset } : undefined}
>
{listHeader}
<View className="min-w-0 flex-1" collapsable={false}>
Expand All @@ -777,6 +778,8 @@ export function ReviewSheet(props: ReviewSheetProps) {
collapsedCommentIdsJson={nativeBridge.collapsedCommentIdsJson}
contentResetKey={`${reviewCache.threadKey}:${selectedSection.id}`}
contentWidth={NATIVE_REVIEW_DIFF_CONTENT_WIDTH}
contentInsetTop={listHeader ? 0 : topContentInset}
contentInsetBottom={bottomContentInset}
nativeViewRef={nativeReviewDiffViewRef}
rowHeight={nativeReviewDiffStyle.rowHeight}
rowsJson={nativeBridge.rowsJson}
Expand Down Expand Up @@ -804,12 +807,10 @@ export function ReviewSheet(props: ReviewSheetProps) {
: undefined
}
contentInsetAdjustmentBehavior="never"
contentInset={{ top: topContentInset, bottom: Math.max(insets.bottom, 18) + 18 }}
contentInset={{ top: topContentInset, bottom: bottomContentInset }}
contentOffset={{ x: 0, y: -topContentInset }}
scrollIndicatorInsets={{
top: topContentInset,
bottom: Math.max(insets.bottom, 18) + 18,
}}
automaticallyAdjustsScrollIndicatorInsets={false}
scrollIndicatorInsets={{ top: topContentInset, bottom: bottomContentInset }}
showsVerticalScrollIndicator={false}
className="flex-1"
refreshControl={
Expand Down
Loading