mirror of
https://github.com/stablyai/orca.git
synced 2026-10-20 08:03:28 +00:00
fix(mobile): hold the history position for the whole paging cycle
The hold was a per-render snapshot of `loadingEarlier`, so it survived exactly one commit. Any render landing between the prepend and its native content-size event -- a streaming tick at 50ms, a keystroke, an agentWorking flip -- rebuilt `onContentSizeChange` with the hold already cleared, and that handler then scrolled to the bottom, undoing the anchoring maintainVisibleContentPosition had just performed. Reproduced with a single unrelated re-render inserted before the layout pass. Re-key it as a latch owned by a small hook: armed when a page is requested, re-armed when it is delivered, and consumed by the layout pass the prepend produces. An interleaved render can no longer drop it, and the effect-ordering dependency the old design relied on is gone. Arming on request also covers a hole the snapshot never did: swapping the spinner into the header resizes it, and that layout pass scrolled the reader to the bottom the moment they tapped "Load earlier". A page can prepend nothing -- failed, empty, or resolved onto a swapped session -- and then no layout pass arrives to consume the hold, so it expires on its own; a latched hold would leave the chat permanently unable to follow the agent. Lives in its own hook because the view is at 388/400 effective lines.
This commit is contained in:
@@ -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 {
|
||||
|
||||
@@ -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<ReturnType<typeof setTimeout> | 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<boolean> => {
|
||||
@@ -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 })
|
||||
}
|
||||
}}
|
||||
|
||||
@@ -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<ReturnType<typeof setTimeout> | 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 }
|
||||
}
|
||||
Reference in New Issue
Block a user