Skip to content

Commit d67febf

Browse files
authored
fix(mobile): hold transcript rows until their position is final (#9396)
* fix(mobile): hold transcript rows until their position is final The agent chat transcript painted its rows before FlashList applied its bottom-start adjustment and scrolled to the newest row, so every row (and the time marker above them) moved by the transcript's whole overflow a beat after the screen had settled. `useSessionListOpenSettle` reports whether the rows have stopped measuring, and `SessionTranscriptOpenGate` keeps them mounted but hidden (`opacity: 0`, which also drops them from the design gate's frame readings) behind the loading skeleton until then. The tail follow and the resume schedule are unchanged; an anchored open reveals at once. New `app:transcript-markers` seed for a transcript whose markers sit above every row. * fix(mobile): settle the transcript gate per session and after real measurements Review follow-ups: the quiet window no longer starts before the list has reported a content height and a viewport (a slow first measurement could otherwise run out the window), a mounted list that switches sessions settles again for the new rows, and the hidden rows are excluded from assistive technology while they are not yet a position the user can read. * fix(mobile): keep the measured viewport when the transcript switches sessions The frame does not change when a mounted list swaps sessions in place, so React Native has no new layout to report. Clearing the measured viewport there left the quiet window unarmable and held the new session's rows behind the skeleton until the cap. Only the content height belongs to a session. The switch test now drives the new session's content size and asserts the quiet-window reveal. * fix(mobile): settle the transcript gate on measurements alone The first reported content height is a measurement in progress, not a verdict: a short transcript can report a fitting height before the rest of its rows arrive, and revealing on that height painted the rows at an offset the list then left. The quiet window now covers every open, including one that fits. The seed fixture keeps its original session id. * test(mobile): record the fixture link in the form the app opens The seed fixture check on main requires the link a fixture records to use the group-prefixed route form, so the transcript-markers seed prints that one.
1 parent 9006f65 commit d67febf

9 files changed

Lines changed: 742 additions & 74 deletions

‎apps/mobile/src/components/agents/child-session-sheet-recovery.mounted.test.tsx‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,10 @@ import { QueryError } from '@/components/query-error';
2828
import { performCopy } from '@/components/agents/use-message-copy';
2929
import { i18n } from '@/i18n';
3030

31+
vi.mock('@/components/agents/session-detail-skeleton', () => ({
32+
SessionSkeletonMessages: 'SessionSkeletonMessages',
33+
}));
34+
3135
vi.mock('@/components/centered-state', () => ({ CenteredState: 'CenteredState' }));
3236
vi.mock('@/components/centered-state-surface', () => ({ StateSurface: 'View' }));
3337
vi.mock('@/components/ui/activity-indicator', () => ({ ActivityIndicator: 'ActivityIndicator' }));

‎apps/mobile/src/components/agents/child-session-sheet-stream-error.mounted.test.tsx‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,10 @@ import { HELD_CHILD_LOAD_RETRY_MS } from './child-session-sheet';
2121
import { i18n } from '@/i18n';
2222
import { QueryError } from '@/components/query-error';
2323

24+
vi.mock('@/components/agents/session-detail-skeleton', () => ({
25+
SessionSkeletonMessages: 'SessionSkeletonMessages',
26+
}));
27+
2428
vi.mock('@/components/centered-state', () => ({ CenteredState: 'CenteredState' }));
2529
vi.mock('@/components/centered-state-surface', () => ({ StateSurface: 'View' }));
2630
vi.mock('@/components/ui/activity-indicator', () => ({ ActivityIndicator: 'ActivityIndicator' }));

‎apps/mobile/src/components/agents/child-session-sheet.mounted.test.tsx‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@ import { QueryError } from '@/components/query-error';
2020
import { i18n } from '@/i18n';
2121
import { ChildSessionModelLabel } from './child-session-model-label';
2222

23+
vi.mock('@/components/agents/session-detail-skeleton', () => ({
24+
SessionSkeletonMessages: 'SessionSkeletonMessages',
25+
}));
26+
2327
vi.mock('@/components/centered-state', () => ({ CenteredState: 'CenteredState' }));
2428
vi.mock('@/components/centered-state-surface', () => ({ StateSurface: 'View' }));
2529
vi.mock('@/components/ui/activity-indicator', () => ({ ActivityIndicator: 'ActivityIndicator' }));

‎apps/mobile/src/components/agents/session-message-list.mounted.test.tsx‎

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,9 @@ vi.mock('@/lib/hooks/use-theme-colors', () => ({
5252
vi.mock('@/components/agents/session-pagination-header', () => ({
5353
SessionPaginationHeader: () => null,
5454
}));
55+
vi.mock('@/components/agents/session-detail-skeleton', () => ({
56+
SessionSkeletonMessages: 'SessionSkeletonMessages',
57+
}));
5558

5659
const baseProps = {
5760
sessionId: 'session-1',
@@ -163,6 +166,120 @@ describe('SessionMessageList', () => {
163166
});
164167
});
165168

169+
describe('SessionMessageList open settle', () => {
170+
function skeletonCount(renderer: TestRenderer.ReactTestRenderer): number {
171+
return renderer.root.findAll(node => Object.is(node.type, 'SessionSkeletonMessages')).length;
172+
}
173+
174+
function fireContentSize(height: number): void {
175+
const handler = flashListProps.current?.onContentSizeChange as
176+
| ((width: number, height: number) => void)
177+
| undefined;
178+
act(() => {
179+
handler?.(400, height);
180+
});
181+
}
182+
183+
function fireLayout(height: number): void {
184+
const handler = flashListProps.current?.onLayout as
185+
| ((event: { nativeEvent: { layout: { height: number } } }) => void)
186+
| undefined;
187+
act(() => {
188+
handler?.({ nativeEvent: { layout: { height } } });
189+
});
190+
}
191+
192+
it('keeps the loading skeleton over the rows until they stop measuring', () => {
193+
vi.useFakeTimers();
194+
const renderer = mountList({ items: ['m1', 'm2', 'm3'] });
195+
expect(skeletonCount(renderer)).toBe(1);
196+
197+
// Content taller than the viewport: the list still has to settle.
198+
fireLayout(655);
199+
fireContentSize(790);
200+
expect(skeletonCount(renderer)).toBe(1);
201+
202+
act(() => {
203+
vi.advanceTimersByTime(150);
204+
});
205+
expect(skeletonCount(renderer)).toBe(0);
206+
});
207+
208+
it('reveals a short transcript after the quiet window, not on its first height', () => {
209+
vi.useFakeTimers();
210+
const renderer = mountList();
211+
fireLayout(655);
212+
fireContentSize(500);
213+
214+
// A height that fits may still be a measurement in progress.
215+
expect(skeletonCount(renderer)).toBe(1);
216+
act(() => {
217+
vi.advanceTimersByTime(150);
218+
});
219+
expect(skeletonCount(renderer)).toBe(0);
220+
});
221+
222+
it('does not reveal before the list has measured anything', () => {
223+
vi.useFakeTimers();
224+
const renderer = mountList({ items: ['m1', 'm2', 'm3'] });
225+
226+
// A slow first measurement must not let the quiet window run out first.
227+
act(() => {
228+
vi.advanceTimersByTime(400);
229+
});
230+
expect(skeletonCount(renderer)).toBe(1);
231+
232+
fireLayout(655);
233+
fireContentSize(790);
234+
act(() => {
235+
vi.advanceTimersByTime(150);
236+
});
237+
expect(skeletonCount(renderer)).toBe(0);
238+
});
239+
240+
it('settles again for a session the mounted list switches to', () => {
241+
vi.useFakeTimers();
242+
const renderer = mountList({ items: ['m1', 'm2', 'm3'] });
243+
fireLayout(655);
244+
fireContentSize(790);
245+
act(() => {
246+
vi.advanceTimersByTime(150);
247+
});
248+
expect(skeletonCount(renderer)).toBe(0);
249+
250+
// The Chat tab follows a conversation that moved onto a new session: the
251+
// same list now paints different rows, without a new layout (the frame did
252+
// not change), so only the content size arrives.
253+
rerenderList(renderer, { sessionId: 'session-2', items: ['n1', 'n2', 'n3'] });
254+
expect(skeletonCount(renderer)).toBe(1);
255+
256+
fireContentSize(810);
257+
act(() => {
258+
vi.advanceTimersByTime(150);
259+
});
260+
expect(skeletonCount(renderer)).toBe(0);
261+
});
262+
263+
it('reveals the rows even if the content never stops measuring', () => {
264+
vi.useFakeTimers();
265+
const renderer = mountList({ items: ['m1', 'm2', 'm3'] });
266+
fireLayout(655);
267+
268+
for (let elapsed = 0; elapsed < 1400; elapsed += 100) {
269+
fireContentSize(800 + elapsed);
270+
act(() => {
271+
vi.advanceTimersByTime(100);
272+
});
273+
}
274+
expect(skeletonCount(renderer)).toBe(1);
275+
276+
act(() => {
277+
vi.advanceTimersByTime(200);
278+
});
279+
expect(skeletonCount(renderer)).toBe(0);
280+
});
281+
});
282+
166283
describe('SessionMessageList older-page auto-scroll', () => {
167284
it('does not yank the viewport when an older page is prepended', () => {
168285
const renderer = mountList({ items: ['m1', 'm2'], hasOlderMessages: true });

‎apps/mobile/src/components/agents/session-message-list.resume-auto-scroll.mounted.test.tsx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,9 @@ vi.mock('@/lib/hooks/use-theme-colors', () => ({
7272
vi.mock('@/components/agents/session-pagination-header', () => ({
7373
SessionPaginationHeader: () => null,
7474
}));
75+
vi.mock('@/components/agents/session-detail-skeleton', () => ({
76+
SessionSkeletonMessages: 'SessionSkeletonMessages',
77+
}));
7578

7679
function resumeItem(id: string): SessionTranscriptItem {
7780
return {

‎apps/mobile/src/components/agents/session-message-list.tsx‎

Lines changed: 87 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,10 @@ import { useSafeAreaInsets } from 'react-native-safe-area-context';
99

1010
import { useSessionListAutoScroll } from '@/components/agents/use-session-list-auto-scroll';
1111
import { useSessionListAnchorReport } from '@/components/agents/use-session-list-anchor-report';
12+
import { useSessionListOpenSettle } from '@/components/agents/use-session-list-open-settle';
1213
import { useSessionListResumeScroll } from '@/components/agents/use-session-list-resume-scroll';
1314
import { SessionPaginationHeader } from '@/components/agents/session-pagination-header';
15+
import { SessionTranscriptOpenGate } from '@/components/agents/session-transcript-open-gate';
1416
import { shouldTriggerOlderMessagesLoad } from '@/components/agents/session-message-list-state';
1517
import {
1618
getSessionTranscriptItemMessageId,
@@ -106,9 +108,7 @@ export function SessionMessageList<T>({
106108
followTailNonce,
107109
}: Readonly<SessionMessageListProps<T>>) {
108110
// Rows are already present when the list mounts (the resume never mounts a
109-
// blank list), so the resume decision is derivable at render: every row's
110-
// anchor id via `getSessionTranscriptItemMessageId`, and whether the anchor
111-
// is one of them.
111+
// blank list), so the resume decision is derivable at render.
112112
const anchorIds = useMemo(
113113
() => items.map(item => getSessionTranscriptItemMessageId(item as SessionTranscriptItem)),
114114
[items]
@@ -157,6 +157,18 @@ export function SessionMessageList<T>({
157157
initialAutoScroll: followTailAtMount,
158158
resumeKey: resumeAnchor,
159159
});
160+
// The transcript stays behind the loading skeleton until its rows stop
161+
// measuring, so the position it is first seen in is the position it keeps.
162+
const {
163+
openSettled,
164+
handleContentSizeChange: handleContentSizeChangeForOpen,
165+
handleListLayout: handleListLayoutForOpen,
166+
} = useSessionListOpenSettle({
167+
sessionId,
168+
followTailAtMount,
169+
onContentSizeChange: handleContentSizeChange,
170+
onLayout: handleListLayout,
171+
});
160172
const colors = useThemeColors();
161173
const { t } = useTranslation();
162174
const { left, right } = useSafeAreaInsets();
@@ -245,11 +257,10 @@ export function SessionMessageList<T>({
245257

246258
// The newest message is kept visible when the viewport shrinks by the follow
247259
// itself: `handleListLayout` re-pins the tail on a viewport resize, and
248-
// FlashList's `startRenderingFromBottom` holds the bottom while the layout
249-
// changes. A keyboard listener here added a second, independent correction —
250-
// the message stack jumped on its own timetable while the composer was still
251-
// moving (video V1 ~19.0s), and no surface beside the provider may read the
252-
// keyboard.
260+
// `startRenderingFromBottom` holds the bottom while the layout changes. A
261+
// keyboard listener here added a second, independent correction — the stack
262+
// jumped as the composer moved (video V1 ~19.0s), and no surface beside the
263+
// provider may read the keyboard.
253264

254265
// Fire `onReachedBottom` only on the false→true transition of
255266
// `isAtBottom`. The previous-value ref prevents a fire on mount (the list
@@ -350,76 +361,78 @@ export function SessionMessageList<T>({
350361
);
351362

352363
return (
353-
<View className="flex-1">
354-
<FlashList<T>
355-
ref={listRef}
356-
style={listStyle}
357-
contentContainerStyle={resolvedContentContainerStyle}
358-
data={items}
359-
keyExtractor={keyExtractor}
360-
getItemType={getItemType}
361-
renderItem={renderItem}
362-
// Transcript rows are tall and parse markdown on mount. The 250 dp
363-
// default draws under half a screen ahead, so a fast fling shows blank
364-
// space until the rows mount. A 2000 dp lookahead hides that.
365-
drawDistance={DRAW_DISTANCE}
366-
// Android Fabric can race clipped-view reattachment with rapid transcript updates.
367-
// Kept explicit: flash-list ≥ 2.3.2 defaults this to false (PR #2202); the pin
368-
// is asserted in src/lib/flash-list-contract.test.ts.
369-
removeClippedSubviews={false}
370-
onScroll={handleScroll}
371-
onScrollBeginDrag={handleScrollBeginDrag}
372-
onScrollEndDrag={handleScrollEndDrag}
373-
onMomentumScrollBegin={handleMomentumScrollBegin}
374-
onMomentumScrollEnd={handleMomentumScrollEnd}
375-
onContentSizeChange={handleContentSizeChange}
376-
onLayout={handleListLayout}
377-
scrollEventThrottle={16}
378-
onStartReached={hasOlderMessages ? handleStartReached : undefined}
379-
onStartReachedThreshold={ON_START_REACHED_THRESHOLD}
380-
// Viewability is only wired when a host asked for the position, so
381-
// every other caller's list stays byte-identical.
382-
{...anchorReport}
383-
maintainVisibleContentPosition={{
384-
// Start rendering from the bottom so the newest message is visible
385-
// on first render. `autoscrollToTopThreshold` is left at its default
386-
// so the viewport only repositions when the user is far enough away
387-
// from the top — preserving the existing auto-follow behavior on
388-
// streaming insertions at the bottom.
389-
startRenderingFromBottom: true,
390-
}}
391-
ListHeaderComponent={listHeaderComponent}
392-
ListFooterComponent={ListFooterComponent}
393-
keyboardDismissMode="interactive"
394-
keyboardShouldPersistTaps="handled"
395-
/>
396-
{/* Floating "scroll to bottom" affordance. Rendered only when the
364+
<SessionTranscriptOpenGate settled={openSettled} sessionId={sessionId}>
365+
<View className="flex-1">
366+
<FlashList<T>
367+
ref={listRef}
368+
style={listStyle}
369+
contentContainerStyle={resolvedContentContainerStyle}
370+
data={items}
371+
keyExtractor={keyExtractor}
372+
getItemType={getItemType}
373+
renderItem={renderItem}
374+
// Transcript rows are tall and parse markdown on mount. The 250 dp
375+
// default draws under half a screen ahead, so a fast fling shows blank
376+
// space until the rows mount. A 2000 dp lookahead hides that.
377+
drawDistance={DRAW_DISTANCE}
378+
// Android Fabric can race clipped-view reattachment with rapid transcript updates.
379+
// Kept explicit: flash-list ≥ 2.3.2 defaults this to false (PR #2202); the pin
380+
// is asserted in src/lib/flash-list-contract.test.ts.
381+
removeClippedSubviews={false}
382+
onScroll={handleScroll}
383+
onScrollBeginDrag={handleScrollBeginDrag}
384+
onScrollEndDrag={handleScrollEndDrag}
385+
onMomentumScrollBegin={handleMomentumScrollBegin}
386+
onMomentumScrollEnd={handleMomentumScrollEnd}
387+
onContentSizeChange={handleContentSizeChangeForOpen}
388+
onLayout={handleListLayoutForOpen}
389+
scrollEventThrottle={16}
390+
onStartReached={hasOlderMessages ? handleStartReached : undefined}
391+
onStartReachedThreshold={ON_START_REACHED_THRESHOLD}
392+
// Viewability is only wired when a host asked for the position, so
393+
// every other caller's list stays byte-identical.
394+
{...anchorReport}
395+
maintainVisibleContentPosition={{
396+
// Start rendering from the bottom so the newest message is visible
397+
// on first render. `autoscrollToTopThreshold` is left at its default
398+
// so the viewport only repositions when the user is far enough away
399+
// from the top — preserving the existing auto-follow behavior on
400+
// streaming insertions at the bottom.
401+
startRenderingFromBottom: true,
402+
}}
403+
ListHeaderComponent={listHeaderComponent}
404+
ListFooterComponent={ListFooterComponent}
405+
keyboardDismissMode="interactive"
406+
keyboardShouldPersistTaps="handled"
407+
/>
408+
{/* Floating "scroll to bottom" affordance. Rendered only when the
397409
user has scrolled past the 100px bottom threshold; the fade
398410
animations match the chat-composer convention. `pointerEvents`
399411
is set on the wrapper so empty space around the button keeps
400412
scrolling the list, while the Pressable itself catches taps. */}
401-
{!isAtBottom ? (
402-
<Animated.View
403-
entering={FadeIn.duration(200)}
404-
exiting={FadeOut.duration(150)}
405-
pointerEvents="box-none"
406-
className="absolute bottom-4 right-4"
407-
// The fixed 16pt (right-4) offset gains the landscape right inset so
408-
// the control clears the sensor area; portrait insets are 0, keeping
409-
// the geometry unchanged (16 == right-4).
410-
style={{ right: 16 + right }}
411-
>
412-
<Pressable
413-
accessibilityRole="button"
414-
accessibilityLabel={t('agentChat.session.scrollToBottom')}
415-
onPress={scrollToLatestAnimated}
416-
hitSlop={2}
417-
className="h-10 w-10 items-center justify-center rounded-full border border-border bg-card shadow-lg shadow-[#00000040] active:opacity-70"
413+
{!isAtBottom ? (
414+
<Animated.View
415+
entering={FadeIn.duration(200)}
416+
exiting={FadeOut.duration(150)}
417+
pointerEvents="box-none"
418+
className="absolute bottom-4 right-4"
419+
// The fixed 16pt (right-4) offset gains the landscape right inset so
420+
// the control clears the sensor area; portrait insets are 0, keeping
421+
// the geometry unchanged (16 == right-4).
422+
style={{ right: 16 + right }}
418423
>
419-
<ChevronDown size={20} color={colors.foreground} />
420-
</Pressable>
421-
</Animated.View>
422-
) : null}
423-
</View>
424+
<Pressable
425+
accessibilityRole="button"
426+
accessibilityLabel={t('agentChat.session.scrollToBottom')}
427+
onPress={scrollToLatestAnimated}
428+
hitSlop={2}
429+
className="h-10 w-10 items-center justify-center rounded-full border border-border bg-card shadow-lg shadow-[#00000040] active:opacity-70"
430+
>
431+
<ChevronDown size={20} color={colors.foreground} />
432+
</Pressable>
433+
</Animated.View>
434+
) : null}
435+
</View>
436+
</SessionTranscriptOpenGate>
424437
);
425438
}

0 commit comments

Comments
 (0)