diff --git a/apps/mobile/src/components/agents/use-session-auto-scroll-state.test.ts b/apps/mobile/src/components/agents/use-session-auto-scroll-state.test.ts index 5c78227dc8..1cd7d1027b 100644 --- a/apps/mobile/src/components/agents/use-session-auto-scroll-state.test.ts +++ b/apps/mobile/src/components/agents/use-session-auto-scroll-state.test.ts @@ -4,6 +4,7 @@ import { isSessionListAtBottom, SESSION_LIST_BOTTOM_THRESHOLD_PX, shouldFollowSessionContentSize, + shouldFollowSessionViewportResize, shouldRetrySessionAutoScroll, shouldScheduleSessionAutoScroll, } from '@/components/agents/use-session-auto-scroll-state'; @@ -253,3 +254,55 @@ describe('shouldFollowSessionContentSize', () => { ).toBe(false); }); }); + +describe('shouldFollowSessionViewportResize', () => { + it('permits a viewport-resize follow scroll while a programmatic scroll is still in flight', () => { + // The fixed status row (working indicator / session status indicator) + // mounts outside the list, so the viewport shrinks while the offset stays + // put. During streaming that resize lands inside the 150ms programmatic + // window, and gating on `!isAutoScrolling` would leave the newest row + // below the fold — drawn over the status row, because the list does not + // clip. + expect( + shouldFollowSessionViewportResize({ + isUserScrolling: false, + shouldAutoScroll: true, + didViewportHeightChange: true, + }) + ).toBe(true); + }); + + it('blocks the follow when the viewport height has not actually changed', () => { + // A redundant layout pass (same height) must keep the guarded scheduler + // instead of taking the bypass, so it cannot stack scrolls on top of a + // programmatic scroll already in flight. + expect( + shouldFollowSessionViewportResize({ + isUserScrolling: false, + shouldAutoScroll: true, + didViewportHeightChange: false, + }) + ).toBe(false); + }); + + it('blocks the follow while the user is actively dragging or in momentum fling', () => { + expect( + shouldFollowSessionViewportResize({ + isUserScrolling: true, + shouldAutoScroll: true, + didViewportHeightChange: true, + }) + ).toBe(false); + }); + + it('blocks the follow when the user has scrolled away from the bottom', () => { + // A keyboard or row resize must never yank a reader who scrolled back. + expect( + shouldFollowSessionViewportResize({ + isUserScrolling: false, + shouldAutoScroll: false, + didViewportHeightChange: true, + }) + ).toBe(false); + }); +}); diff --git a/apps/mobile/src/components/agents/use-session-auto-scroll-state.ts b/apps/mobile/src/components/agents/use-session-auto-scroll-state.ts index 0c3ed9067f..19a30efcb9 100644 --- a/apps/mobile/src/components/agents/use-session-auto-scroll-state.ts +++ b/apps/mobile/src/components/agents/use-session-auto-scroll-state.ts @@ -123,3 +123,40 @@ export function shouldFollowSessionContentSize({ } return true; } + +/** + * Decide whether a transcript viewport resize should trigger a follow scroll + * to the latest message. The fixed status rows (the working indicator, the + * session status indicator) live OUTSIDE the list, so they shrink the list's + * viewport while its scroll offset stays put: the newest row is then left + * below the fold and — because the list does not clip its overflow + * (`removeClippedSubviews={false}`, see session-message-list.tsx) — is drawn + * under the transparent status row. Re-pin on the resize. + * + * Like `shouldFollowSessionContentSize`, this does NOT gate on + * `isAutoScrolling`: the resize lands during the streaming follow window, and + * gating on it would drop exactly the correction this exists for. The + * user-facing guards still apply, and an unchanged viewport height does not + * take this path (the caller keeps the guarded scheduler for it), so a + * redundant layout pass cannot stack scrolls. + */ +export function shouldFollowSessionViewportResize({ + isUserScrolling, + shouldAutoScroll, + didViewportHeightChange, +}: { + isUserScrolling: boolean; + shouldAutoScroll: boolean; + didViewportHeightChange: boolean; +}): boolean { + if (!shouldAutoScroll) { + return false; + } + if (isUserScrolling) { + return false; + } + if (!didViewportHeightChange) { + return false; + } + return true; +} diff --git a/apps/mobile/src/components/agents/use-session-list-auto-scroll.mounted.test.tsx b/apps/mobile/src/components/agents/use-session-list-auto-scroll.mounted.test.tsx new file mode 100644 index 0000000000..7fe8ca8e64 --- /dev/null +++ b/apps/mobile/src/components/agents/use-session-list-auto-scroll.mounted.test.tsx @@ -0,0 +1,150 @@ +import { createElement } from 'react'; +import { type FlashListRef } from '@shopify/flash-list'; +import { + type LayoutChangeEvent, + type NativeScrollEvent, + type NativeSyntheticEvent, +} from 'react-native'; +import { act, TestRenderer } from '@/test/renderer'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +import { useSessionListAutoScroll } from './use-session-list-auto-scroll'; + +// The follow policy only needs the scroll-animation preference; the battery and +// OS hooks behind the real policy are irrelevant here. +vi.mock('@/lib/a11y/motion', () => ({ + useMotionPolicy: () => ({ reducedMotion: false, scrollAnimated: false }), +})); + +type AutoScrollApi = ReturnType>; + +function layoutEvent(height: number): LayoutChangeEvent { + const event = { nativeEvent: { layout: { x: 0, y: 0, width: 400, height } } }; + return event as unknown as LayoutChangeEvent; +} + +function scrollEvent( + contentHeight: number, + viewportHeight: number, + offsetY: number +): NativeSyntheticEvent { + const event = { + nativeEvent: { + contentOffset: { x: 0, y: offsetY }, + contentSize: { width: 400, height: contentHeight }, + layoutMeasurement: { width: 400, height: viewportHeight }, + }, + }; + return event as unknown as NativeSyntheticEvent; +} + +function mountAutoScroll(itemCount: number) { + const probe: { current: AutoScrollApi | null } = { current: null }; + + function Probe({ count }: Readonly<{ count: number }>) { + probe.current = useSessionListAutoScroll({ itemCount: count, resetKey: 'session-1' }); + return createElement('View', null); + } + + act(() => { + TestRenderer.create(createElement(Probe, { count: itemCount })); + }); + + const api = probe.current; + if (api === null) { + throw new Error('the auto-scroll probe did not mount'); + } + + const scrollToEnd = vi.fn(); + const handle: { scrollToEnd: typeof scrollToEnd } = { scrollToEnd }; + api.listRef.current = handle as unknown as FlashListRef; + return { scrollToEnd, api }; +} + +function closeProgrammaticScrollWindow() { + // The mount follow arms a 150ms reset window and an 80ms safety-net retry; + // the retry re-arms the reset, so the window only closes after both. + act(() => { + vi.advanceTimersByTime(500); + }); +} + +describe('useSessionListAutoScroll viewport resize', () => { + afterEach(() => { + vi.useRealTimers(); + }); + + it('re-pins the tail when the fixed status row shrinks the list inside the streaming follow window', () => { + // Reproduces the session-answered defect: the working indicator / session + // status row lives outside the list, so mounting it shrinks the viewport + // while the offset stays put. The resize lands inside the 150ms + // programmatic-scroll window a streaming content-size follow just opened; + // the guarded scheduler drops it, the newest row is left below the fold, + // and the list (which does not clip) draws it over the status row. + vi.useFakeTimers(); + const { scrollToEnd, api } = mountAutoScroll(1); + + // The first layout records the pre-resize viewport height. + act(() => { + api.handleListLayout(layoutEvent(600)); + }); + closeProgrammaticScrollWindow(); + scrollToEnd.mockClear(); + + // A streamed row grows the content and starts a follow scroll. + act(() => { + api.handleContentSizeChange(400, 2000); + }); + expect(scrollToEnd).toHaveBeenCalledTimes(1); + + scrollToEnd.mockClear(); + + // Inside that window the status row mounts and the list gets shorter. + act(() => { + api.handleListLayout(layoutEvent(500)); + }); + + expect(scrollToEnd).toHaveBeenCalledTimes(1); + }); + + it('never yanks a reader who scrolled away from the bottom', () => { + vi.useFakeTimers(); + const { scrollToEnd, api } = mountAutoScroll(1); + + act(() => { + api.handleListLayout(layoutEvent(600)); + }); + closeProgrammaticScrollWindow(); + // Far from the content end: the follow is off. + act(() => { + api.handleScroll(scrollEvent(4000, 600, 0)); + }); + scrollToEnd.mockClear(); + + act(() => { + api.handleListLayout(layoutEvent(500)); + }); + + expect(scrollToEnd).not.toHaveBeenCalled(); + }); + + it('never yanks a viewport resize during a drag', () => { + vi.useFakeTimers(); + const { scrollToEnd, api } = mountAutoScroll(1); + + act(() => { + api.handleListLayout(layoutEvent(600)); + }); + closeProgrammaticScrollWindow(); + act(() => { + api.handleScrollBeginDrag(); + }); + scrollToEnd.mockClear(); + + act(() => { + api.handleListLayout(layoutEvent(500)); + }); + + expect(scrollToEnd).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/mobile/src/components/agents/use-session-list-auto-scroll.ts b/apps/mobile/src/components/agents/use-session-list-auto-scroll.ts index 58aadcf4ca..cc7595efd6 100644 --- a/apps/mobile/src/components/agents/use-session-list-auto-scroll.ts +++ b/apps/mobile/src/components/agents/use-session-list-auto-scroll.ts @@ -1,12 +1,17 @@ import { type FlashListRef } from '@shopify/flash-list'; import { useCallback, useEffect, useRef, useState } from 'react'; -import { type NativeScrollEvent, type NativeSyntheticEvent } from 'react-native'; +import { + type LayoutChangeEvent, + type NativeScrollEvent, + type NativeSyntheticEvent, +} from 'react-native'; import { getInitialSessionListAutoScrollVisibility, isSessionListAtBottom, SESSION_LIST_BOTTOM_THRESHOLD_PX, shouldFollowSessionContentSize, + shouldFollowSessionViewportResize, shouldRetrySessionAutoScroll, shouldScheduleSessionAutoScroll, } from '@/components/agents/use-session-auto-scroll-state'; @@ -76,6 +81,9 @@ export function useSessionListAutoScroll({ // (see the reset effect) while a drag's claim outranks the link. const sendTakeoverRef = useRef(false); const lastContentHeightRef = useRef(0); + // The list's own height, tracked so a viewport resize (the fixed status row + // mounting outside the list) can re-pin the tail. See `handleListLayout`. + const lastViewportHeightRef = useRef(0); const autoScrollResetTimeoutRef = useRef | null>(null); const autoScrollRetryTimeoutRef = useRef | null>(null); const userScrollingTimeoutRef = useRef | null>(null); @@ -352,9 +360,8 @@ export function useSessionListAutoScroll({ // to the bottom. Gating on `!isAutoScrolling` here would silently // drop every streaming update that lands inside the debounce // window. Bypass `scheduleScrollToLatestMessage` (which keeps - // the `!isAutoScrolling` guard for the initial itemCount / - // handleListLayout triggers) and trigger the programmatic scroll - // directly. + // the `!isAutoScrolling` guard for the initial itemCount trigger) + // and trigger the programmatic scroll directly. if ( shouldFollowSessionContentSize({ isUserScrolling: isUserScrollingRef.current, @@ -368,17 +375,41 @@ export function useSessionListAutoScroll({ [scrollToLatestMessage] ); - const handleListLayout = useCallback(() => { - if ( - shouldScheduleSessionAutoScroll({ - isAutoScrolling: isAutoScrollingRef.current, - isUserScrolling: isUserScrollingRef.current, - shouldAutoScroll: shouldAutoScrollRef.current, - }) - ) { - scheduleScrollToLatestMessage(); - } - }, [scheduleScrollToLatestMessage]); + const handleListLayout = useCallback( + (event: LayoutChangeEvent) => { + const { height } = event.nativeEvent.layout; + const didViewportHeightChange = height !== lastViewportHeightRef.current; + lastViewportHeightRef.current = height; + // A viewport resize is the same hazard as a content-size change: the + // fixed status rows mount OUTSIDE the list, so the list gets shorter + // while its offset stays put and the newest row is left below the fold, + // drawn over the transparent status row. Re-pin the tail directly — + // bypassing `scheduleScrollToLatestMessage`'s `!isAutoScrolling` guard + // for the same reason `handleContentSizeChange` does: the resize lands + // inside the streaming follow window, and the guarded scheduler would + // swallow exactly the correction this exists for. + if ( + shouldFollowSessionViewportResize({ + isUserScrolling: isUserScrollingRef.current, + shouldAutoScroll: shouldAutoScrollRef.current, + didViewportHeightChange, + }) + ) { + scrollToLatestMessage(); + return; + } + if ( + shouldScheduleSessionAutoScroll({ + isAutoScrolling: isAutoScrollingRef.current, + isUserScrolling: isUserScrollingRef.current, + shouldAutoScroll: shouldAutoScrollRef.current, + }) + ) { + scheduleScrollToLatestMessage(); + } + }, + [scheduleScrollToLatestMessage, scrollToLatestMessage] + ); const handleKeyboardShow = useCallback(() => { // Reuse the guarded scheduler so a keyboard opening never yanks the list