diff --git a/mobile/src/session/MobileNativeChatView.test.ts b/mobile/src/session/MobileNativeChatView.test.ts index 5226f483d27..38b722a633c 100644 --- a/mobile/src/session/MobileNativeChatView.test.ts +++ b/mobile/src/session/MobileNativeChatView.test.ts @@ -207,6 +207,11 @@ describe('MobileNativeChatView', () => { scrollToEnd.mockClear() await update({ messages: [message], hasMore: true, loadingEarlier: true }) + // The spinner swapping in resizes the header — that layout pass must not + // consume the hold, since the page has not arrived yet. + const spinnerList = renderer!.root.find((node) => node.type === 'FlatList') + act(() => spinnerList.props.onContentSizeChange()) + await update({ messages: [older, message], hasMore: false, loadingEarlier: false }) const list = renderer!.root.find((node) => node.type === 'FlatList') expect(list.props.maintainVisibleContentPosition).toEqual({ minIndexForVisible: 0 }) @@ -219,6 +224,75 @@ describe('MobileNativeChatView', () => { } }) + it('holds position when an unrelated render lands before the layout pass', async () => { + vi.useFakeTimers() + try { + await render({ messages: [message], hasMore: true }) + await act(async () => vi.runAllTimersAsync()) + scrollToEnd.mockClear() + + await update({ messages: [message], hasMore: true, loadingEarlier: true }) + await update({ messages: [older, message], hasMore: false, loadingEarlier: false }) + // A streaming tick or keystroke re-renders before the native content-size + // event lands — the hold has to outlive it, or the anchoring is undone. + await update({ messages: [older, message], hasMore: false, loadingEarlier: false }) + const list = renderer!.root.find((node) => node.type === 'FlatList') + act(() => list.props.onContentSizeChange()) + await act(async () => vi.runAllTimersAsync()) + + expect(scrollToEnd).not.toHaveBeenCalled() + } finally { + vi.useRealTimers() + } + }) + + it('drops an already-armed tail-follow when a page starts before it fires', async () => { + vi.useFakeTimers() + try { + await render({ messages: [message], hasMore: true }) + await act(async () => vi.runAllTimersAsync()) + scrollToEnd.mockClear() + + // A message lands, arming the 60ms tail-follow; the reader taps "Load + // earlier" inside that window, so the scroll must be abandoned. + const appended = { ...message, id: 'appended' } + await update({ messages: [message, appended], hasMore: true }) + await act(async () => vi.advanceTimersByTimeAsync(30)) + await update({ messages: [message, appended], hasMore: true, loadingEarlier: true }) + await act(async () => vi.advanceTimersByTimeAsync(60)) + + expect(scrollToEnd).not.toHaveBeenCalled() + } finally { + vi.useRealTimers() + } + }) + + it('releases the hold when a page prepends nothing', async () => { + vi.useFakeTimers() + try { + await render({ messages: [message], hasMore: true }) + await act(async () => vi.runAllTimersAsync()) + scrollToEnd.mockClear() + + // A failed/empty page clears `loadingEarlier` without prepending, so no + // layout pass arrives to consume the hold. It must expire on its own. + await update({ messages: [message], hasMore: true, loadingEarlier: true }) + await update({ messages: [message], hasMore: true, loadingEarlier: false }) + await act(async () => vi.runAllTimersAsync()) + scrollToEnd.mockClear() + + const appended = { ...message, id: 'appended' } + await update({ messages: [message, appended], hasMore: true }) + const list = renderer!.root.find((node) => node.type === 'FlatList') + act(() => list.props.onContentSizeChange()) + await act(async () => vi.runAllTimersAsync()) + + expect(scrollToEnd).toHaveBeenCalled() + } finally { + vi.useRealTimers() + } + }) + it('still follows the tail once the history page has settled', async () => { vi.useFakeTimers() try { @@ -228,14 +302,18 @@ describe('MobileNativeChatView', () => { // the chat permanently unable to follow the agent. await update({ messages: [message], hasMore: true, loadingEarlier: true }) await update({ messages: [older, message], hasMore: false, loadingEarlier: false }) - await act(async () => vi.runAllTimersAsync()) + const settled = renderer!.root.find((node) => node.type === 'FlatList') + act(() => settled.props.onContentSizeChange()) + // Stay well inside the hold's expiry so this proves the page's own layout + // pass released it, not the never-latch safety net. + await act(async () => vi.advanceTimersByTimeAsync(100)) scrollToEnd.mockClear() const appended = { ...message, id: 'appended' } await update({ messages: [older, message, appended], hasMore: false }) const list = renderer!.root.find((node) => node.type === 'FlatList') act(() => list.props.onContentSizeChange()) - await act(async () => vi.runAllTimersAsync()) + await act(async () => vi.advanceTimersByTimeAsync(100)) expect(scrollToEnd).toHaveBeenCalled() } finally { diff --git a/mobile/src/session/MobileNativeChatView.tsx b/mobile/src/session/MobileNativeChatView.tsx index 21763f0d670..f8fbb49a6f9 100644 --- a/mobile/src/session/MobileNativeChatView.tsx +++ b/mobile/src/session/MobileNativeChatView.tsx @@ -21,6 +21,7 @@ import { type MobileNativeChatPendingItem } from './mobile-native-chat-render-data' import { useMobileNativeChatAskDismiss } from './use-mobile-native-chat-ask-dismiss' +import { useMobileNativeChatHistoryHold } from './use-mobile-native-chat-history-hold' import { useMobileNativeChatPinchGesture } from './use-mobile-native-chat-pinch-gesture' import { MobileAgentWorkingIndicator } from './MobileAgentWorkingIndicator' import type { PendingNativeChatImage } from './mobile-native-chat-image-attachment' @@ -153,10 +154,8 @@ export function MobileNativeChatView({ const sendScrollTimerRef = useRef | null>(null) // A short transcript leaves the reader both at the top and `atBottom`, so the // tail-follow paths would scroll away the history that just paged in. - const keepHistoryPositionRef = useRef(Boolean(loadingEarlier)) - // Snapshot, not a live ref read: effects flush before native layout, so by the - // time `onContentSizeChange` runs the ref is already cleared. - const keepHistoryPosition = keepHistoryPositionRef.current + const { isHeld: isHistoryHeld, release: releaseHistoryHold } = + useMobileNativeChatHistoryHold(loadingEarlier) const { fontScale, pinchGesture } = useMobileNativeChatPinchGesture() useEffect( () => () => { @@ -182,17 +181,18 @@ export function MobileNativeChatView({ // we don't yank the user away while they read history. (Also fires on keyboard // close, which is harmless while atBottom.) useEffect(() => { - if (data.length === 0 || !atBottom || keepHistoryPositionRef.current) { + if (data.length === 0 || !atBottom || isHistoryHeld()) { return } - const t = setTimeout(() => listRef.current?.scrollToEnd({ animated: true }), 60) + // Re-check on fire: a page can start inside this 60ms window, and a scroll + // armed before the tap must not outlive the hold it would defeat. + const t = setTimeout(() => { + if (!isHistoryHeld()) { + listRef.current?.scrollToEnd({ animated: true }) + } + }, 60) return () => clearTimeout(t) - }, [data.length, atBottom, keyboardInset]) - // Must stay below the tail-follow effect: the prepend lands in the same commit - // that clears `loadingEarlier`, and that effect has to still see the hold. - useEffect(() => { - keepHistoryPositionRef.current = Boolean(loadingEarlier) - }, [loadingEarlier]) + }, [data.length, atBottom, keyboardInset, isHistoryHeld]) const handleSend = useCallback( async (text: string): Promise => { @@ -290,7 +290,14 @@ export function MobileNativeChatView({ // the load-earlier header offset internally. maintainVisibleContentPosition={{ minIndexForVisible: 0 }} onContentSizeChange={() => { - if (data.length > 0 && atBottom && !keepHistoryPosition) { + // The prepend's own layout pass consumes the hold. Reading the + // latch (not a render snapshot) is what makes this survive a + // streaming tick landing before the native event. + if (isHistoryHeld()) { + releaseHistoryHold() + return + } + if (data.length > 0 && atBottom) { listRef.current?.scrollToEnd({ animated: false }) } }} diff --git a/mobile/src/session/use-mobile-native-chat-history-hold.ts b/mobile/src/session/use-mobile-native-chat-history-hold.ts new file mode 100644 index 00000000000..c3dce020a27 --- /dev/null +++ b/mobile/src/session/use-mobile-native-chat-history-hold.ts @@ -0,0 +1,78 @@ +import { useCallback, useEffect, useRef } from 'react' + +/** Never let the hold outlive a paging cycle: a page that prepends nothing + * (failed, empty, or resolved onto a swapped session) produces no layout pass + * to consume it, and a stuck `loadingEarlier` would otherwise disable + * tail-follow for good. Re-armed on each transition, so a slow round trip just + * re-holds when the page lands. */ +const HISTORY_HOLD_MAX_MS = 500 + +export type MobileNativeChatHistoryHold = { + /** True while a paging cycle's layout passes are still expected. */ + isHeld: () => boolean + /** Consume the hold — call from the layout pass that the prepend produced. */ + release: () => void +} + +/** + * Suppress tail-follow across a `loadEarlier` cycle. + * + * The hold is a latch, not a per-render snapshot of `loadingEarlier`: a + * streaming tick or keystroke rendering between the prepend commit and the + * native content-size event would drop a snapshot, and the tail-follow would + * then scroll away the history that just paged in. + * + * It covers the whole cycle, not just delivery — swapping the spinner into the + * header resizes it, and that layout pass would otherwise scroll the reader to + * the bottom the moment they tap "Load earlier". + */ +export function useMobileNativeChatHistoryHold( + loadingEarlier: boolean | undefined +): MobileNativeChatHistoryHold { + const heldRef = useRef(false) + const inFlightRef = useRef(false) + const expiryRef = useRef | null>(null) + + const clearExpiry = useCallback(() => { + if (expiryRef.current) { + clearTimeout(expiryRef.current) + expiryRef.current = null + } + }, []) + + const hold = useCallback(() => { + heldRef.current = true + clearExpiry() + expiryRef.current = setTimeout(() => { + heldRef.current = false + expiryRef.current = null + }, HISTORY_HOLD_MAX_MS) + }, [clearExpiry]) + + const isHeld = useCallback(() => heldRef.current, []) + // Each layout pass consumes one hold; the spinner's pass is re-held when the + // page is delivered, so it costs nothing to let it consume this one. + const release = useCallback(() => { + heldRef.current = false + clearExpiry() + }, [clearExpiry]) + + useEffect(() => { + if (loadingEarlier) { + inFlightRef.current = true + hold() + return + } + if (!inFlightRef.current) { + return + } + inFlightRef.current = false + hold() + }, [loadingEarlier, hold]) + + useEffect(() => () => clearExpiry(), [clearExpiry]) + + // Both callbacks are stable, so a consumer can depend on them individually + // without this object's identity mattering. + return { isHeld, release } +}