Skip to content

Commit a0c4fbf

Browse files
committed
fix(scroll): keep autoscroll to bottom when item sizes settle
useBoundDetection skips the autoscroll check when the content size changes within 100ms of the last bounds check, so an active scroll is not fought. The check is dropped rather than deferred, and the taller content then pushes the bottom out of reach, so the next checkBounds clears the pending autoscroll and nothing ever brings it back. Items measuring to their real height a frame after they mount is exactly that case, which is why a list with dynamic item heights stops sticking to the bottom: an image finishing its load or a message wrapping onto another line is enough to lose it. Defer the check to the moment the scroll goes quiet instead of dropping it, and run the autoscroll that was owed. The deferred run is cancelled if the scroll offset moved in the meantime, which is what separates the two cases: content growing below the viewport leaves the offset untouched, while a user who takes over changes it and keeps ownership of the position. The deferred run goes through runAutoScrollToBottomCheck rather than resurrecting the pending flag, so scrollToEnd stays suppressed while offset projection is disabled and no stale flag is left behind for a later data change to fire. Fixes #1903
1 parent 435b514 commit a0c4fbf

3 files changed

Lines changed: 239 additions & 22 deletions

File tree

.claude/skills/review-and-test/SKILL.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -338,13 +338,21 @@ Run through relevant entries after any fix or review. This is the single source
338338
- [ ] `firstItemOffset` after fix — confirm it equals `ListHeaderComponent` height/width
339339
- [ ] `measureParentSize(view)` returns `x=0, y=0` on RN 0.84 Fabric — the #2017 bug may only manifest on other RN versions
340340

341+
### Autoscroll to bottom (`maintainVisibleContentPosition.autoscrollToBottomThreshold`)
342+
- [ ] Item heights settling after the row mounts (image loads, text wraps) still keeps the list pinned to the bottom
343+
- [ ] New content arriving mid-scroll still lands at the bottom once the scroll stops
344+
- [ ] Scrolling up while content is still growing is NOT yanked back down
345+
- [ ] `scrollToIndex` in flight is still not hijacked by the autoscroll (`isOffsetProjectionEnabled` guard)
346+
341347
### Performance
342348
- [ ] Benchmark screen shows no FPS regression (use `ManualBenchmarkExample`)
343349

344350
---
345351

346352
## Common Issues
347353

354+
- **A `not.toHaveBeenCalled()` test can pass without ever reaching the code it names** — negative assertions are vacuous by default. Mutation-test each guard separately (delete one, confirm exactly the test that names it goes red), or drop a temporary `console.log` in the branch to prove the test enters it. A test that stays green when its guard is deleted is testing nothing.
355+
- **A time-guarded bug hides under `jest.runAllTimers()`**`runAllTimers` advances the fake clock past every `Date.now()` guard in the code (e.g. `useBoundDetection`'s 100ms quiet window), so a bug that only happens inside that window silently passes. Drive those cases with `jest.advanceTimersByTime(16)` and only drain with `runAllTimers` at the end.
348356
- **Tests pass but device shows bug** — did you `yarn build` and relaunch? The dist/ folder may be stale
349357
- **Switched branches but behavior didn't change**`dist/` is NOT rebuilt on branch switch. You MUST run `yarn build` after every `git checkout`. Verify with `grep` in `dist/` that the expected code is present before testing.
350358
- **RTL looks wrong but LTR is fine** — did you set `forceRTL(true)` in `index.js` and do a full kill+relaunch?

src/__tests__/RecyclerView.test.tsx

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -514,4 +514,137 @@ describe("RecyclerView", () => {
514514
expect(scrollToEndSpy).toHaveBeenCalled();
515515
});
516516
});
517+
describe("autoscroll to bottom when item sizes change", () => {
518+
const { measureItemLayout } = jest.requireMock(
519+
"../recyclerview/utils/measureLayout"
520+
) as { measureItemLayout: jest.Mock };
521+
522+
// jest.clearAllMocks() keeps implementations, so a grown item size would
523+
// otherwise leak in from an earlier test and leave nothing left to grow.
524+
beforeEach(() => {
525+
measureItemLayout.mockImplementation(() => ({
526+
x: 0,
527+
y: 0,
528+
width: 399,
529+
height: 100,
530+
}));
531+
});
532+
533+
const itemCount = 30;
534+
const itemHeight = 100;
535+
const windowHeight = 899;
536+
const contentHeight = itemCount * itemHeight;
537+
const bottomOffset = contentHeight - windowHeight;
538+
539+
const scrollTo = (root: ReturnType<typeof render>, y: number) => {
540+
const scrollable = root.findWhere((node: any) => node.props.onScroll);
541+
if (!scrollable) throw new Error("Could not find scrollable component");
542+
543+
const onScroll: any = scrollable.prop("onScroll" as never);
544+
root.act(() => {
545+
onScroll({
546+
nativeEvent: {
547+
contentOffset: { x: 0, y },
548+
contentSize: { width: 399, height: contentHeight },
549+
layoutMeasurement: { width: 399, height: windowHeight },
550+
},
551+
});
552+
});
553+
};
554+
555+
// Renders a chat style list, parks it at the given offset and returns a spy
556+
// on the scroll the autoscroll would issue.
557+
const renderStickyBottomList = (offset: number = bottomOffset) => {
558+
const data = Array.from({ length: itemCount }, (_, i) => i);
559+
const ref = createRef<FlashListRef<number>>();
560+
const result = render(
561+
<FlashList
562+
ref={ref}
563+
data={data}
564+
extraData={1}
565+
keyExtractor={(item) => String(item)}
566+
maintainVisibleContentPosition={{
567+
autoscrollToBottomThreshold: 0.2,
568+
animateAutoScrollToBottom: false,
569+
}}
570+
renderItem={({ item }) => <Text>{item}</Text>}
571+
/>
572+
);
573+
jest.runAllTimers();
574+
scrollTo(result, offset);
575+
jest.runAllTimers();
576+
577+
const scrollToEndSpy = jest.fn();
578+
const nativeScrollRef = ref.current?.getNativeScrollRef() as any;
579+
expect(nativeScrollRef).toBeTruthy();
580+
nativeScrollRef.scrollToEnd = scrollToEndSpy;
581+
582+
return { result, scrollToEndSpy };
583+
};
584+
585+
// Items settle to their real height with no data change - what an image
586+
// finishing its load or a message wrapping onto another line does a frame
587+
// after the row mounts.
588+
const growItems = (result: ReturnType<typeof render>) => {
589+
measureItemLayout.mockImplementation(() => ({
590+
x: 0,
591+
y: 0,
592+
width: 399,
593+
height: 300,
594+
}));
595+
result.setProps({ extraData: 2 });
596+
};
597+
598+
it("still autoscrolls when items grow right after a scroll", () => {
599+
const { result, scrollToEndSpy } = renderStickyBottomList();
600+
601+
// The growth lands inside the window where an active scroll suppresses
602+
// the autoscroll check.
603+
scrollTo(result, bottomOffset);
604+
jest.advanceTimersByTime(16);
605+
growItems(result);
606+
jest.advanceTimersByTime(16);
607+
expect(scrollToEndSpy).not.toHaveBeenCalled();
608+
609+
// Once the scroll goes quiet the deferred autoscroll runs.
610+
jest.runAllTimers();
611+
expect(scrollToEndSpy).toHaveBeenCalled();
612+
});
613+
614+
it("autoscrolls right away when items grow well after the last scroll", () => {
615+
const { result, scrollToEndSpy } = renderStickyBottomList();
616+
617+
scrollTo(result, bottomOffset);
618+
jest.advanceTimersByTime(200);
619+
growItems(result);
620+
jest.runAllTimers();
621+
622+
expect(scrollToEndSpy).toHaveBeenCalled();
623+
});
624+
625+
it("does not autoscroll if the user scrolls away while the growth settles", () => {
626+
const { result, scrollToEndSpy } = renderStickyBottomList();
627+
628+
scrollTo(result, bottomOffset);
629+
jest.advanceTimersByTime(16);
630+
growItems(result);
631+
632+
// The user drags up before the deferred autoscroll gets its turn.
633+
scrollTo(result, 200);
634+
jest.runAllTimers();
635+
636+
expect(scrollToEndSpy).not.toHaveBeenCalled();
637+
});
638+
639+
it("does not autoscroll when the list was not near the bottom", () => {
640+
const { result, scrollToEndSpy } = renderStickyBottomList(0);
641+
642+
scrollTo(result, 0);
643+
jest.advanceTimersByTime(16);
644+
growItems(result);
645+
jest.runAllTimers();
646+
647+
expect(scrollToEndSpy).not.toHaveBeenCalled();
648+
});
649+
});
517650
});

src/recyclerview/hooks/useBoundDetection.ts

Lines changed: 98 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,25 @@ import { useCallback, useEffect, useMemo, useRef } from "react";
33
import { RecyclerViewManager } from "../RecyclerViewManager";
44
import { CompatScroller } from "../components/CompatScroller";
55

6-
import { useUnmountAwareAnimationFrame } from "./useUnmountAwareCallbacks";
6+
import {
7+
useUnmountAwareAnimationFrame,
8+
useUnmountAwareTimeout,
9+
} from "./useUnmountAwareCallbacks";
10+
11+
/**
12+
* How long the scroll has to stay quiet before a content size change is allowed
13+
* to trigger the autoscroll to bottom. Autoscrolling in the middle of an active
14+
* scroll fights the user, so the check waits for this gap instead.
15+
*/
16+
const AUTOSCROLL_QUIET_WINDOW = 100;
17+
18+
/**
19+
* Scroll offsets can come back off by a sub pixel without anything having
20+
* actually moved, so a deferred autoscroll treats a difference this small as
21+
* "nobody scrolled". A real drag covers far more than this in
22+
* AUTOSCROLL_QUIET_WINDOW.
23+
*/
24+
const AUTOSCROLL_OFFSET_TOLERANCE = 1;
725

826
/**
927
* Hook to detect when the scroll position reaches near the start or end of the list
@@ -28,9 +46,14 @@ export function useBoundDetection<T>(
2846
const pendingAutoscrollToBottom = useRef(false);
2947

3048
const lastCheckBoundsTime = useRef(Date.now());
49+
// Holds an autoscroll that was owed when the content size changed mid scroll,
50+
// along with the scroll offset at that moment so a real user scroll during the
51+
// wait can cancel it.
52+
const deferredAutoscroll = useRef<{ offset: number } | undefined>(undefined);
3153

3254
const { data } = recyclerViewManager.props;
3355
const { requestAnimationFrame } = useUnmountAwareAnimationFrame();
56+
const { setTimeout } = useUnmountAwareTimeout();
3457

3558
const windowHeight = recyclerViewManager.hasLayout()
3659
? recyclerViewManager.getWindowSize().height
@@ -140,27 +163,36 @@ export function useBoundDetection<T>(
140163
}
141164
}, [recyclerViewManager]);
142165

143-
const runAutoScrollToBottomCheck = useCallback(() => {
144-
// Suppress MVCP autoscroll while a programmatic scrollToIndex is in
145-
// flight. FlashList disables offset projection at the start of
146-
// scrollToIndex and reenables it ~200-300ms after settling. Without
147-
// this guard, the sticky pendingAutoscrollToBottom ref races against
148-
// scrollToIndex and fires scrollToEnd mid flight.
149-
if (!recyclerViewManager.isOffsetProjectionEnabled) {
150-
return;
151-
}
152-
if (pendingAutoscrollToBottom.current) {
153-
pendingAutoscrollToBottom.current = false;
154-
requestAnimationFrame(() => {
155-
const shouldAnimate =
156-
recyclerViewManager.props.maintainVisibleContentPosition
157-
?.animateAutoScrollToBottom ?? true;
158-
scrollViewRef.current?.scrollToEnd({
159-
animated: shouldAnimate && !recyclerViewManager.ignoreScrollEvents,
166+
/**
167+
* @param force - run the autoscroll even though checkBounds has since cleared
168+
* pendingAutoscrollToBottom. Used by the deferred path, where the content
169+
* growing below the viewport is what put the bottom out of reach in the first
170+
* place.
171+
*/
172+
const runAutoScrollToBottomCheck = useCallback(
173+
(force = false) => {
174+
// Suppress MVCP autoscroll while a programmatic scrollToIndex is in
175+
// flight. FlashList disables offset projection at the start of
176+
// scrollToIndex and reenables it ~200-300ms after settling. Without
177+
// this guard, the sticky pendingAutoscrollToBottom ref races against
178+
// scrollToIndex and fires scrollToEnd mid flight.
179+
if (!recyclerViewManager.isOffsetProjectionEnabled) {
180+
return;
181+
}
182+
if (force || pendingAutoscrollToBottom.current) {
183+
pendingAutoscrollToBottom.current = false;
184+
requestAnimationFrame(() => {
185+
const shouldAnimate =
186+
recyclerViewManager.props.maintainVisibleContentPosition
187+
?.animateAutoScrollToBottom ?? true;
188+
scrollViewRef.current?.scrollToEnd({
189+
animated: shouldAnimate && !recyclerViewManager.ignoreScrollEvents,
190+
});
160191
});
161-
});
162-
}
163-
}, [requestAnimationFrame, scrollViewRef, recyclerViewManager]);
192+
}
193+
},
194+
[requestAnimationFrame, scrollViewRef, recyclerViewManager]
195+
);
164196

165197
// Reset end reached state when data changes
166198
useMemo(() => {
@@ -174,16 +206,60 @@ export function useBoundDetection<T>(
174206
runAutoScrollToBottomCheck();
175207
}, [data, runAutoScrollToBottomCheck, windowHeight, windowWidth]);
176208

209+
/**
210+
* Waits out AUTOSCROLL_QUIET_WINDOW and then runs the autoscroll that the
211+
* content size change was owed, as long as nothing actually scrolled in the
212+
* meantime.
213+
*/
214+
const scheduleAutoScrollRetry = useCallback(() => {
215+
if (!pendingAutoscrollToBottom.current || deferredAutoscroll.current) {
216+
return;
217+
}
218+
deferredAutoscroll.current = {
219+
offset: recyclerViewManager.getAbsoluteLastScrollOffset(),
220+
};
221+
222+
setTimeout(() => {
223+
const deferred = deferredAutoscroll.current;
224+
deferredAutoscroll.current = undefined;
225+
if (!deferred) {
226+
return;
227+
}
228+
// An offset that moved means the user took over, and checkBounds has
229+
// already recorded whether they are still near the bottom - leave the
230+
// decision to it rather than yanking them back down. An offset that did
231+
// not move means nothing is scrolling, which is both the case autoscroll
232+
// exists for and proof that there is no scroll left to fight.
233+
if (
234+
Math.abs(
235+
recyclerViewManager.getAbsoluteLastScrollOffset() - deferred.offset
236+
) > AUTOSCROLL_OFFSET_TOLERANCE
237+
) {
238+
return;
239+
}
240+
runAutoScrollToBottomCheck(true);
241+
}, AUTOSCROLL_QUIET_WINDOW);
242+
}, [recyclerViewManager, runAutoScrollToBottomCheck, setTimeout]);
243+
177244
// Since content changes frequently, we try and avoid doing the auto scroll during active scrolls
178245
useEffect(() => {
179-
if (Date.now() - lastCheckBoundsTime.current >= 100) {
246+
if (Date.now() - lastCheckBoundsTime.current >= AUTOSCROLL_QUIET_WINDOW) {
180247
runAutoScrollToBottomCheck();
248+
return;
181249
}
250+
// The content changed while a scroll was still settling. Giving up here
251+
// loses the autoscroll for good: the taller content pushes the bottom out
252+
// of reach, so the next checkBounds clears the pending autoscroll and
253+
// nothing brings it back. Items measuring to their real height right after
254+
// new content arrives is exactly that case, which is why a list with
255+
// dynamic item heights stops sticking to the bottom.
256+
scheduleAutoScrollRetry();
182257
}, [
183258
contentHeight,
184259
contentWidth,
185260
recyclerViewManager.firstItemOffset,
186261
runAutoScrollToBottomCheck,
262+
scheduleAutoScrollRetry,
187263
]);
188264

189265
return {

0 commit comments

Comments
 (0)