authorgravatar for julius0216@outlook.comJulius Marminge <julius0216@outlook.com> 2026-10-07 23:15:05-07:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-10-07 23:15:05-07:00
log97a65a3121c11801a877897928efc51ef40fa49f
tree6e07cb07aab139199f216316c0358f09b3f4dd35
parent57f96e69d39669f47e7310062222cd67adbb4912
signature Signed by PGP key B5690EEEBB952194

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

3 files changed, 32 insertions(+), 25 deletions(-)

apps/mobile/modules/t3-review-diff/ios/T3ReviewDiffView.swift+18-11
...@@ -563,7 +563,7 @@ public final class T3ReviewDiffView: ExpoView, UIScrollViewDelegate {...@@ -563,7 +563,7 @@ public final class T3ReviewDiffView: ExpoView, UIScrollViewDelegate {
563 ])563 ])
564 }564 }
565565
566 // Source files fill the screen so UIKit can sample content below the glass bar.566 // Code surfaces fill the screen so UIKit can sample content below the glass bar.
567 // Keep the same visible row when header or safe-area reservations change.567 // Keep the same visible row when header or safe-area reservations change.
568 func setContentInsetTop(_ inset: CGFloat) {568 func setContentInsetTop(_ inset: CGFloat) {
569 scrollView.automaticallyAdjustsScrollIndicatorInsets = false569 scrollView.automaticallyAdjustsScrollIndicatorInsets = false
...@@ -571,6 +571,7 @@ public final class T3ReviewDiffView: ExpoView, UIScrollViewDelegate {...@@ -571,6 +571,7 @@ public final class T3ReviewDiffView: ExpoView, UIScrollViewDelegate {
571 let delta = inset - scrollView.contentInset.top571 let delta = inset - scrollView.contentInset.top
572 guard delta != 0 else { return }572 guard delta != 0 else { return }
573 scrollView.contentInset.top = inset573 scrollView.contentInset.top = inset
574 contentView.topContentInset = inset
574 scrollView.verticalScrollIndicatorInsets.top = inset575 scrollView.verticalScrollIndicatorInsets.top = inset
575 scrollView.contentOffset.y -= delta576 scrollView.contentOffset.y -= delta
576 updateViewportFrame()577 updateViewportFrame()
...@@ -618,8 +619,12 @@ public final class T3ReviewDiffView: ExpoView, UIScrollViewDelegate {...@@ -618,8 +619,12 @@ public final class T3ReviewDiffView: ExpoView, UIScrollViewDelegate {
618 if let scrollAnchor,619 if let scrollAnchor,
619 let headerOffset = contentView.fileHeaderOffset(forFileId: scrollAnchor.fileId) {620 let headerOffset = contentView.fileHeaderOffset(forFileId: scrollAnchor.fileId) {
620 let targetOffset = headerOffset - scrollAnchor.screenY621 let targetOffset = headerOffset - scrollAnchor.screenY
621 let maxOffset = max(scrollView.contentSize.height - scrollView.bounds.height, 0)622 let minOffset = -scrollView.contentInset.top
622 let clampedOffset = min(max(targetOffset, 0), maxOffset)623 let maxOffset = max(
624 scrollView.contentSize.height - scrollView.bounds.height + scrollView.contentInset.bottom,
625 minOffset
626 )
627 let clampedOffset = min(max(targetOffset, minOffset), maxOffset)
623 scrollView.setContentOffset(CGPoint(x: 0, y: clampedOffset), animated: false)628 scrollView.setContentOffset(CGPoint(x: 0, y: clampedOffset), animated: false)
624 updateViewportFrame()629 updateViewportFrame()
625 }630 }
...@@ -735,7 +740,8 @@ public final class T3ReviewDiffView: ExpoView, UIScrollViewDelegate {...@@ -735,7 +740,8 @@ public final class T3ReviewDiffView: ExpoView, UIScrollViewDelegate {
735 // The top of the combined diff is the explicit "All files" destination.740 // The top of the combined diff is the explicit "All files" destination.
736 // Treat it as a first-class selection instead of immediately resolving the741 // Treat it as a first-class selection instead of immediately resolving the
737 // first file header and undoing the navigator's optimistic selection.742 // first file header and undoing the navigator's optimistic selection.
738 if scrollView.contentOffset.y <= 0.5 {743 let visibleTop = scrollView.contentOffset.y + scrollView.contentInset.top
744 if visibleTop <= 0.5 {
739 guard lastVisibleFileId != nil else {745 guard lastVisibleFileId != nil else {
740 return746 return
741 }747 }
...@@ -744,7 +750,7 @@ public final class T3ReviewDiffView: ExpoView, UIScrollViewDelegate {...@@ -744,7 +750,7 @@ public final class T3ReviewDiffView: ExpoView, UIScrollViewDelegate {
744 return750 return
745 }751 }
746752
747 guard let fileId = contentView.visibleFileId(atVerticalOffset: scrollView.contentOffset.y),753 guard let fileId = contentView.visibleFileId(atVerticalOffset: visibleTop),
748 fileId != lastVisibleFileId else {754 fileId != lastVisibleFileId else {
749 return755 return
750 }756 }
...@@ -1025,6 +1031,7 @@ private final class ReviewDiffContentView: UIView, UIGestureRecognizerDelegate {...@@ -1025,6 +1031,7 @@ private final class ReviewDiffContentView: UIView, UIGestureRecognizerDelegate {
1025 }1031 }
1026 }1032 }
1027 var verticalOffset: CGFloat = 01033 var verticalOffset: CGFloat = 0
1034 var topContentInset: CGFloat = 0
1028 var theme = ReviewDiffNativeTheme.resolve("light") {1035 var theme = ReviewDiffNativeTheme.resolve("light") {
1029 didSet {1036 didSet {
1030 tokenColorsByHex.removeAll()1037 tokenColorsByHex.removeAll()
...@@ -1495,7 +1502,7 @@ private final class ReviewDiffContentView: UIView, UIGestureRecognizerDelegate {...@@ -1495,7 +1502,7 @@ private final class ReviewDiffContentView: UIView, UIGestureRecognizerDelegate {
14951502
1496 let rowIndex = stickyHeader.rowIndex1503 let rowIndex = stickyHeader.rowIndex
1497 let headerTop = rowOffsets[rowIndex]1504 let headerTop = rowOffsets[rowIndex]
1498 guard headerTop < verticalOffset else {1505 guard headerTop < verticalOffset + topContentInset else {
1499 return nil1506 return nil
1500 }1507 }
15011508
...@@ -1505,12 +1512,12 @@ private final class ReviewDiffContentView: UIView, UIGestureRecognizerDelegate {...@@ -1505,12 +1512,12 @@ private final class ReviewDiffContentView: UIView, UIGestureRecognizerDelegate {
1505 : nil1512 : nil
1506 let pushedY: CGFloat1513 let pushedY: CGFloat
1507 if let nextHeaderRowIndex {1514 if let nextHeaderRowIndex {
1508 pushedY = min(0, rowOffsets[nextHeaderRowIndex] - verticalOffset - style.fileHeaderHeight)1515 pushedY = min(topContentInset, rowOffsets[nextHeaderRowIndex] - verticalOffset - style.fileHeaderHeight)
1509 } else {1516 } else {
1510 pushedY = 01517 pushedY = topContentInset
1511 }1518 }
15121519
1513 guard pushedY > -style.fileHeaderHeight else {1520 guard pushedY > topContentInset - style.fileHeaderHeight else {
1514 return nil1521 return nil
1515 }1522 }
15161523
...@@ -1533,7 +1540,7 @@ private final class ReviewDiffContentView: UIView, UIGestureRecognizerDelegate {...@@ -1533,7 +1540,7 @@ private final class ReviewDiffContentView: UIView, UIGestureRecognizerDelegate {
1533 while lowerBound < upperBound {1540 while lowerBound < upperBound {
1534 let midpoint = (lowerBound + upperBound) / 21541 let midpoint = (lowerBound + upperBound) / 2
1535 let rowIndex = fileHeaderRowIndices[midpoint]1542 let rowIndex = fileHeaderRowIndices[midpoint]
1536 if rowOffsets[rowIndex] <= verticalOffset {1543 if rowOffsets[rowIndex] <= verticalOffset + topContentInset {
1537 lowerBound = midpoint + 11544 lowerBound = midpoint + 1
1538 } else {1545 } else {
1539 upperBound = midpoint1546 upperBound = midpoint
...@@ -1550,7 +1557,7 @@ private final class ReviewDiffContentView: UIView, UIGestureRecognizerDelegate {...@@ -1550,7 +1557,7 @@ private final class ReviewDiffContentView: UIView, UIGestureRecognizerDelegate {
1550 func scrollAnchor(forFileId fileId: String) -> ReviewDiffScrollAnchor? {1557 func scrollAnchor(forFileId fileId: String) -> ReviewDiffScrollAnchor? {
1551 if let stickyHeader = stickyFileHeaderTarget(),1558 if let stickyHeader = stickyFileHeaderTarget(),
1552 resolvedFileId(for: stickyHeader.row) == fileId {1559 resolvedFileId(for: stickyHeader.row) == fileId {
1553 return ReviewDiffScrollAnchor(fileId: fileId, screenY: 0)1560 return ReviewDiffScrollAnchor(fileId: fileId, screenY: topContentInset)
1554 }1561 }
15551562
1556 guard let headerOffset = fileHeaderOffset(forFileId: fileId) else {1563 guard let headerOffset = fileHeaderOffset(forFileId: fileId) else {
apps/mobile/src/Stack.tsx+2-3
...@@ -158,8 +158,7 @@ const GLASS_HEADER_OPTIONS: AppScreenOptions = {...@@ -158,8 +158,7 @@ const GLASS_HEADER_OPTIONS: AppScreenOptions = {
158 unstable_navigationItemStyle: NATIVE_LIQUID_GLASS_SUPPORTED ? "editor" : undefined,158 unstable_navigationItemStyle: NATIVE_LIQUID_GLASS_SUPPORTED ? "editor" : undefined,
159};159};
160160
161// SOLID: opaque sheet-colored header for surfaces whose content scrolls internally161// SOLID: opaque sheet-colored header for surfaces that manage their own viewport.
162// (terminal, review) — there is nothing for glass to sample there.
163const SOLID_HEADER_OPTIONS: AppScreenOptions = {162const SOLID_HEADER_OPTIONS: AppScreenOptions = {
164 headerBackButtonDisplayMode: "minimal",163 headerBackButtonDisplayMode: "minimal",
165 headerBackTitle: "",164 headerBackTitle: "",
...@@ -724,7 +723,7 @@ const RootStackConfig = createWorkspaceStackNavigator({...@@ -724,7 +723,7 @@ const RootStackConfig = createWorkspaceStackNavigator({
724 ThreadReview: createNativeStackScreen({723 ThreadReview: createNativeStackScreen({
725 screen: ReviewSheet,724 screen: ReviewSheet,
726 linking: `${THREAD_LINKING_PREFIX}/review`,725 linking: `${THREAD_LINKING_PREFIX}/review`,
727 options: SOLID_HEADER_OPTIONS,726 options: GLASS_HEADER_OPTIONS,
728 }),727 }),
729 ThreadReviewComment: createNativeStackScreen({728 ThreadReviewComment: createNativeStackScreen({
730 screen: ReviewCommentComposerSheet,729 screen: ReviewCommentComposerSheet,
apps/mobile/src/features/review/ReviewSheet.tsx+12-11
...@@ -29,6 +29,7 @@ import {...@@ -29,6 +29,7 @@ import {
29 View,29 View,
30} from "react-native";30} from "react-native";
31import { useSafeAreaInsets } from "react-native-safe-area-context";31import { useSafeAreaInsets } from "react-native-safe-area-context";
32import { useNativeColumnLayoutMetrics } from "../../native/native-layout-metrics";
3233
33import { AppText as Text } from "../../components/AppText";34import { AppText as Text } from "../../components/AppText";
34import { SymbolView } from "../../components/AppSymbol";35import { SymbolView } from "../../components/AppSymbol";
...@@ -156,7 +157,6 @@ function ReviewHeader(...@@ -156,7 +157,6 @@ function ReviewHeader(
156 );157 );
157}158}
158159
159const REVIEW_HEADER_SPACING = 0;
160const SHOWCASE_ENABLED = process.env.EXPO_PUBLIC_SHOWCASE === "1";160const SHOWCASE_ENABLED = process.env.EXPO_PUBLIC_SHOWCASE === "1";
161161
162const ReviewNotice = memo(function ReviewNotice(props: { readonly notice: string }) {162const ReviewNotice = memo(function ReviewNotice(props: { readonly notice: string }) {
...@@ -461,10 +461,11 @@ export function ReviewSheet(props: ReviewSheetProps) {...@@ -461,10 +461,11 @@ export function ReviewSheet(props: ReviewSheetProps) {
461 const { draftMessage } = useThreadDraftForThread({ environmentId, threadId });461 const { draftMessage } = useThreadDraftForThread({ environmentId, threadId });
462 const reviewCache = useReviewCacheForThread({ environmentId, threadId });462 const reviewCache = useReviewCacheForThread({ environmentId, threadId });
463 const { selectedThreadCwd } = useSelectedThreadWorktree();463 const { selectedThreadCwd } = useSelectedThreadWorktree();
464 // With a solid (non-overlay) header the content lays out below the header464 const columnMetrics = useNativeColumnLayoutMetrics();
465 // natively, so no manual top inset is needed. (Android renders its own465 const safeArea = columnMetrics?.safeArea ?? insets;
466 // in-flow AndroidScreenHeader, so it needs no inset either.)466 const topContentInset = Platform.OS === "ios" ? safeArea.top : 0;
467 const topContentInset = 0;467 const bottomContentInset =
468 Platform.OS === "ios" ? safeArea.bottom + 8 : Math.max(insets.bottom, 18) + 18;
468469
469 useEffect(() => {470 useEffect(() => {
470 showAuxiliaryPane("inspector");471 showAuxiliaryPane("inspector");
...@@ -760,7 +761,7 @@ export function ReviewSheet(props: ReviewSheetProps) {...@@ -760,7 +761,7 @@ export function ReviewSheet(props: ReviewSheetProps) {
760 >761 >
761 <View762 <View
762 className="min-w-0 flex-1"763 className="min-w-0 flex-1"
763 style={{ paddingTop: topContentInset + REVIEW_HEADER_SPACING }}764 style={listHeader ? { paddingTop: topContentInset } : undefined}
764 >765 >
765 {listHeader}766 {listHeader}
766 <View className="min-w-0 flex-1" collapsable={false}>767 <View className="min-w-0 flex-1" collapsable={false}>
...@@ -777,6 +778,8 @@ export function ReviewSheet(props: ReviewSheetProps) {...@@ -777,6 +778,8 @@ export function ReviewSheet(props: ReviewSheetProps) {
777 collapsedCommentIdsJson={nativeBridge.collapsedCommentIdsJson}778 collapsedCommentIdsJson={nativeBridge.collapsedCommentIdsJson}
778 contentResetKey={`${reviewCache.threadKey}:${selectedSection.id}`}779 contentResetKey={`${reviewCache.threadKey}:${selectedSection.id}`}
779 contentWidth={NATIVE_REVIEW_DIFF_CONTENT_WIDTH}780 contentWidth={NATIVE_REVIEW_DIFF_CONTENT_WIDTH}
781 contentInsetTop={listHeader ? 0 : topContentInset}
782 contentInsetBottom={bottomContentInset}
780 nativeViewRef={nativeReviewDiffViewRef}783 nativeViewRef={nativeReviewDiffViewRef}
781 rowHeight={nativeReviewDiffStyle.rowHeight}784 rowHeight={nativeReviewDiffStyle.rowHeight}
782 rowsJson={nativeBridge.rowsJson}785 rowsJson={nativeBridge.rowsJson}
...@@ -804,12 +807,10 @@ export function ReviewSheet(props: ReviewSheetProps) {...@@ -804,12 +807,10 @@ export function ReviewSheet(props: ReviewSheetProps) {
804 : undefined807 : undefined
805 }808 }
806 contentInsetAdjustmentBehavior="never"809 contentInsetAdjustmentBehavior="never"
807 contentInset={{ top: topContentInset, bottom: Math.max(insets.bottom, 18) + 18 }}810 contentInset={{ top: topContentInset, bottom: bottomContentInset }}
808 contentOffset={{ x: 0, y: -topContentInset }}811 contentOffset={{ x: 0, y: -topContentInset }}
809 scrollIndicatorInsets={{812 automaticallyAdjustsScrollIndicatorInsets={false}
810 top: topContentInset,813 scrollIndicatorInsets={{ top: topContentInset, bottom: bottomContentInset }}
811 bottom: Math.max(insets.bottom, 18) + 18,
812 }}
813 showsVerticalScrollIndicator={false}814 showsVerticalScrollIndicator={false}
814 className="flex-1"815 className="flex-1"
815 refreshControl={816 refreshControl={