From cd1029f53a5becd9ee7e870dcb973f07e48c52b5 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Fri, 23 Oct 2026 17:34:29 -0700 Subject: [PATCH] fix(mobile): Home and pull request lists let UIKit own their top inset Home, the pull request list, pull request detail and reviewers set their top inset by hand from useSafeAreaInsets and pinned a resting offset in JS. On the iPhone Duo, iOS 27 minimizes the navigation bar on scroll down by default, which shrinks the safe area every frame; each change round- tripped through JS, moved the content a frame late, and UIKit read that as a reversed scroll that restored the bar. Home's sidebar stuttered in place. These lists now use contentInsetAdjustmentBehavior "automatic", like the thread feed, so UIKit moves the inset and offset in the same frame as the bar, through folds and column resizes too. That removes the manual contentInset props and useRestingTopOffset; bottom padding subtracts the safe area UIKit now adds. Co-Authored-By: Claude Opus 5.5 --- apps/mobile/src/features/home/HomeScreen.tsx | 16 +--- .../PullRequestReviewersScreen.tsx | 11 +-- .../pull-requests/PullRequestRouteScreen.tsx | 17 +---- .../pull-requests/PullRequestsScreen.tsx | 19 ++--- .../src/native/useRestingTopOffset.test.tsx | 73 ------------------- apps/mobile/src/native/useRestingTopOffset.ts | 29 -------- 6 files changed, 16 insertions(+), 149 deletions(-) delete mode 100644 apps/mobile/src/native/useRestingTopOffset.test.tsx delete mode 100644 apps/mobile/src/native/useRestingTopOffset.ts diff --git a/apps/mobile/src/features/home/HomeScreen.tsx b/apps/mobile/src/features/home/HomeScreen.tsx index 389054aa1035..2de56b7c391c 100644 --- a/apps/mobile/src/features/home/HomeScreen.tsx +++ b/apps/mobile/src/features/home/HomeScreen.tsx @@ -45,7 +45,6 @@ import type { SavedRemoteConnection } from "../../lib/connection"; import { scopedProjectKey, scopedThreadKey } from "../../lib/scopedEntities"; import { NativePrimaryColumnContext } from "../../native/v5-workspace-context"; import { nativeHeaderScrollEdgeEffects } from "../../native/scrollEdgeEffects"; -import { useRestingTopOffset } from "../../native/useRestingTopOffset"; import { useNativeColumnLayoutMetrics, useNativeLayoutMetrics, @@ -218,9 +217,7 @@ function HomeTopContentSpacer() { } function HomeScrollView(props: ComponentProps) { - const insets = useSafeAreaInsets(); const primaryColumn = use(NativePrimaryColumnContext); - const restingOffset = useRestingTopOffset(insets.top, props); if (Platform.OS !== "ios") return ; // v5 needs the actual content scroll view registered with its screen controller. @@ -235,14 +232,8 @@ function HomeScrollView(props: ComponentProps) { > ); @@ -885,8 +876,9 @@ export function HomeScreen(props: HomeScreenProps) { ListEmptyComponent={v2ListEmpty} style={{ flex: 1 }} automaticallyAdjustsScrollIndicatorInsets={Platform.OS === "ios"} - contentInsetAdjustmentBehavior="never" - contentInset={Platform.OS === "ios" ? { top: insets.top } : undefined} + // UIKit owns the bar's inset, so a minimizing bar and a resized column move + // the list in the same frame as the bar. + contentInsetAdjustmentBehavior="automatic" contentInsetStartAdjustment={Platform.OS === "ios" ? insets.top : 0} initialScrollOffset={ initialScrollOffset > 0 @@ -905,7 +897,7 @@ export function HomeScreen(props: HomeScreenProps) { (contentSideInsets?.right ?? 0) + (primaryColumn ? 8 : 0) + androidListGutter, paddingBottom: Platform.OS === "ios" - ? iosBottomClearance + ? iosBottomClearance - (columnMetrics?.safeArea.bottom ?? insets.bottom) : (workspace?.bottomClearance ?? Math.max(insets.bottom, 16) + (Platform.OS === "android" ? fabClearance : 88)), diff --git a/apps/mobile/src/features/pull-requests/PullRequestReviewersScreen.tsx b/apps/mobile/src/features/pull-requests/PullRequestReviewersScreen.tsx index af471abf2c86..ab609d42456b 100644 --- a/apps/mobile/src/features/pull-requests/PullRequestReviewersScreen.tsx +++ b/apps/mobile/src/features/pull-requests/PullRequestReviewersScreen.tsx @@ -20,7 +20,6 @@ import { ScrollViewMarker } from "react-native-screens"; import { AppText as Text, AppTextInput as TextInput } from "../../components/ui/AppText"; import { EmptyState } from "../../components/ui/EmptyState"; import { Button } from "../../components/ui/Button"; -import { useRestingTopOffset } from "../../native/useRestingTopOffset"; import { SymbolView } from "../../components/ui/AppSymbol"; import { NativeStackScreenOptions } from "../../native/StackHeader"; import { useNativeLayoutMetrics } from "../../native/native-layout-metrics"; @@ -35,17 +34,9 @@ import { PullRequestAvatar } from "./PullRequestPresentation"; import { pullRequestTarget, type PullRequestParams } from "./usePullRequestDiff"; function ReviewerScrollView(props: ScrollViewProps) { - const insets = useSafeAreaInsets(); - const restingOffset = useRestingTopOffset(insets.top, props); return ( - + ); } diff --git a/apps/mobile/src/features/pull-requests/PullRequestRouteScreen.tsx b/apps/mobile/src/features/pull-requests/PullRequestRouteScreen.tsx index a513fd424ced..84f9ea16f4bb 100644 --- a/apps/mobile/src/features/pull-requests/PullRequestRouteScreen.tsx +++ b/apps/mobile/src/features/pull-requests/PullRequestRouteScreen.tsx @@ -56,7 +56,6 @@ import { View, } from "react-native"; import { NativeScrollViewMarker as ScrollViewMarker } from "../../native/NativeScrollViewMarker"; -import { useRestingTopOffset } from "../../native/useRestingTopOffset"; import { useSafeAreaInsets } from "react-native-safe-area-context"; import { AppText as Text } from "../../components/ui/AppText"; @@ -117,16 +116,9 @@ import { usePullRequestEnvironmentFallback } from "./usePullRequestEnvironmentFa const EMPTY_IDS: readonly string[] = []; function PullRequestTabScrollView(props: ScrollViewProps) { - const insets = useSafeAreaInsets(); - const restingOffset = useRestingTopOffset(insets.top, props); return ( - + ); } @@ -572,10 +564,9 @@ export function PullRequestRouteScreen({ route }: StaticScreenProps - + ); } @@ -266,8 +258,9 @@ export function PullRequestsScreen( showsVerticalScrollIndicator={Platform.OS !== "android"} recycleItems renderScrollComponent={Platform.OS === "ios" ? PullRequestScrollView : undefined} - contentInsetAdjustmentBehavior="never" - contentInset={Platform.OS === "ios" ? { top: insets.top } : undefined} + // UIKit owns the bar's inset, so a minimizing bar and a resized column move + // the list in the same frame as the bar. + contentInsetAdjustmentBehavior="automatic" // A plain offset: LegendList adds the list header to an index-based restore, which // landed every return one header lower. initialScrollOffset={ @@ -275,7 +268,9 @@ export function PullRequestsScreen( } contentInsetStartAdjustment={Platform.OS === "ios" ? insets.top : 0} contentContainerStyle={{ - paddingBottom: Math.max(insets.bottom, workspace.bottomClearance), + paddingBottom: + Math.max(insets.bottom, workspace.bottomClearance) - + (Platform.OS === "ios" ? insets.bottom : 0), }} onScroll={(event) => { workspace.recordScrollOffset( diff --git a/apps/mobile/src/native/useRestingTopOffset.test.tsx b/apps/mobile/src/native/useRestingTopOffset.test.tsx deleted file mode 100644 index 7be7e1d82c3b..000000000000 --- a/apps/mobile/src/native/useRestingTopOffset.test.tsx +++ /dev/null @@ -1,73 +0,0 @@ -import { act, useLayoutEffect } from "react"; -import { createRoot, type Root } from "react-dom/client"; -import { afterEach, describe, expect, it } from "vite-plus/test"; - -import { useRestingTopOffset } from "./useRestingTopOffset"; - -let root: Root | null = null; -let latest: ReturnType | null = null; - -function Probe(props: { readonly topInset: number; readonly restoredOffset?: number }) { - const restingOffset = useRestingTopOffset( - props.topInset, - props.restoredOffset === undefined ? {} : { contentOffset: { x: 0, y: props.restoredOffset } }, - ); - useLayoutEffect(() => { - latest = restingOffset; - }); - return null; -} - -function render(topInset: number, restoredOffset?: number) { - (globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; - root ??= createRoot(document.createElement("div")); - act(() => root!.render()); -} - -function scrollTo(y: number) { - const event = { nativeEvent: { contentOffset: { x: 0, y } } } as Parameters< - ReturnType["onScroll"] - >[0]; - act(() => latest!.onScroll(event)); -} - -function offset() { - return latest!.contentOffset.y; -} - -afterEach(() => { - act(() => root?.unmount()); - root = null; - latest = null; -}); - -describe("useRestingTopOffset", () => { - it("keeps an untouched list below the header once the column's inset arrives", () => { - // The folded iPhone Duo's window has no top inset; the header's arrives after mount. - render(0); - render(54); - expect(offset()).toBe(-54); - }); - - it("does not move a list the reader scrolled when the inset changes", () => { - render(54); - scrollTo(300); - render(90); - expect(offset()).toBe(-54); - }); - - it("follows the inset again once the list is back at the top", () => { - render(54); - scrollTo(300); - render(90); - scrollTo(-90); - render(54); - expect(offset()).toBe(-54); - }); - - it("holds a restored position through an inset change", () => { - render(0, 1_200); - render(54); - expect(offset()).toBe(1_200); - }); -}); diff --git a/apps/mobile/src/native/useRestingTopOffset.ts b/apps/mobile/src/native/useRestingTopOffset.ts deleted file mode 100644 index 33fc6bd09b47..000000000000 --- a/apps/mobile/src/native/useRestingTopOffset.ts +++ /dev/null @@ -1,29 +0,0 @@ -import { useState } from "react"; -import type { NativeScrollEvent, NativeSyntheticEvent, ScrollViewProps } from "react-native"; - -/** - * A scroll view with a manual top inset rests at `-topInset`, just below the header. The inset - * can change after mount: a column's safe area starts as the window's until UIKit measures it, - * and rotation or folding changes it again. While the view rests below the header it follows the - * inset; once scrolled away it holds the last resting offset, because RN re-applies a changed - * `contentOffset` prop and would move the reader. - */ -export function holdRestingTopOffset(held: number | null, scrollY: number, topInset: number) { - return Math.abs(scrollY + topInset) <= 1 ? null : (held ?? -topInset); -} - -export function useRestingTopOffset( - topInset: number, - props: Pick, -) { - // A list restoring a saved position mounts away from its resting offset. - const [held, setHeld] = useState(props.contentOffset?.y ?? null); - return { - contentOffset: { x: 0, y: held ?? -topInset }, - onScroll: (event: NativeSyntheticEvent) => { - const scrollY = event.nativeEvent.contentOffset.y; - setHeld((current) => holdRestingTopOffset(current, scrollY, topInset)); - props.onScroll?.(event); - }, - }; -}