Skip to content

Commit cb57696

Browse files
fix(intro-screen-native): catch the trailing drag whatever order it arrives in
The Android e2e flow failed deterministically (both the first attempt and the retry) at `Assert that "Active slide: 3" is visible`. The CI video shows the widget reach slide 3, then snap back to slide 1 and increment the change counter, so the trailing drag the previous guard was written for still got through. It got through because the guard required evidence that a programmatic scroll had landed (settledScrollTarget), and that evidence is not always recorded: the settling list can emit its trailing scrollBeginDrag BEFORE the scroll's own momentum end, and onScrollBeginDrag clears the pending target, so the branch that would have set settledScrollTarget never runs. The guard was then skipped and the stale offset was treated as a swipe. Drop the bookkeeping and rely on the geometry instead. The list is pagingEnabled, so a drag settles at most one page from where it began; a momentum end further away than that cannot be a finger regardless of what came before it. Covered by a test reproducing the CI ordering, which fails without the change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent ccedcd9 commit cb57696

2 files changed

Lines changed: 42 additions & 21 deletions

File tree

packages/pluggableWidgets/intro-screen-native/src/SwipeableContainer.tsx

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -80,10 +80,6 @@ export const SwipeableContainer = (props: SwipeableContainerProps): ReactElement
8080
// wherever the user left it. Used to recognise momentum events that contradict the
8181
// scroll we just asked for; see onMomentumScrollEnd.
8282
const pendingScrollTarget = useRef<number | null>(null);
83-
// Slide the last programmatic scroll settled on. Android can follow that scroll with a
84-
// second drag+momentum pair reporting the offset the list LEFT, which looks exactly like
85-
// a user swipe back; see onMomentumScrollEnd.
86-
const settledScrollTarget = useRef<number | null>(null);
8783

8884
const rtlSafeIndex = useCallback(
8985
(i: number): number => (isAndroidRTL ? props.slides.length - 1 - i : i),
@@ -95,7 +91,6 @@ export const SwipeableContainer = (props: SwipeableContainerProps): ReactElement
9591
setActiveIndex(pageNum);
9692
if (flashList && flashList.current) {
9793
pendingScrollTarget.current = pageNum;
98-
settledScrollTarget.current = null;
9994
flashList.current.scrollToOffset({
10095
offset: rtlSafeIndex(pageNum) * width,
10196
animated
@@ -360,10 +355,6 @@ export const SwipeableContainer = (props: SwipeableContainerProps): ReactElement
360355
if (pendingTarget !== null) {
361356
if (newIndex === pendingTarget) {
362357
pendingScrollTarget.current = null;
363-
// Remember where the scroll landed: on Android the settling list can emit a
364-
// SECOND drag + momentum pair afterwards, and that one carries the offset it
365-
// moved away from.
366-
settledScrollTarget.current = pendingTarget;
367358
}
368359
return;
369360
}
@@ -372,14 +363,18 @@ export const SwipeableContainer = (props: SwipeableContainerProps): ReactElement
372363
return;
373364
}
374365

375-
// The list is pagingEnabled, so one drag moves exactly one page. A momentum end that
376-
// jumps more than a page right after a programmatic scroll settled is therefore the
377-
// tail of that scroll, reporting the offset it moved AWAY from — not a swipe. Android
378-
// emits it with the drag flag set (the settling list, not a finger, starts the drag),
379-
// so wasUserDragging cannot tell the two apart, and trusting it rewrote the attribute
380-
// to the slide being left: pressing NEXT on slide 2 wrote 3, then 1. A one-page swipe
381-
// still reports normally.
382-
if (settledScrollTarget.current !== null && Math.abs(newIndex - activeIndex) > 1) {
366+
// The list is pagingEnabled, so a drag settles at most one page from where it began.
367+
// Any momentum end further away than that is the tail of a programmatic scroll,
368+
// reporting an offset the list has already left — not a swipe. Android emits it with
369+
// the drag flag set (the settling list, not a finger, begins the drag), so
370+
// wasUserDragging cannot tell the two apart, and trusting it rewrote the attribute to
371+
// a slide that is not on screen: pressing NEXT on slide 2 wrote 3, then 1.
372+
//
373+
// This deliberately does not depend on having seen the programmatic scroll land. That
374+
// trailing drag can arrive BEFORE the scroll's own momentum end, and onScrollBeginDrag
375+
// clears pendingScrollTarget, so there is no record left to gate on — the earlier
376+
// version of this guard checked for one and was skipped in exactly that ordering.
377+
if (Math.abs(newIndex - activeIndex) > 1) {
383378
return;
384379
}
385380

packages/pluggableWidgets/intro-screen-native/src/__tests__/SwipeableContainer.remount.spec.tsx

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -221,17 +221,43 @@ describe("SwipeableContainer remount behaviour", () => {
221221
fireEvent.press(getByTestId("intro$buttonNext"));
222222
onSlideChange.mockClear();
223223

224-
// The user grabs the list before the scroll to slide 3 settles and swipes back to
225-
// slide 1. Taking over invalidates the pending target, so this must be reported.
224+
// The user grabs the list before the scroll to slide 3 settles and swipes back one
225+
// page, landing on slide 2. Taking over invalidates the pending target, so this must
226+
// be reported even though the scroll it interrupted never reported a momentum end.
227+
act(() => {
228+
fireEvent(getByTestId("intro"), "scrollBeginDrag", {});
229+
fireEvent(getByTestId("intro"), "momentumScrollEnd", {
230+
nativeEvent: { contentOffset: { x: WIDTH } }
231+
});
232+
});
233+
234+
expect(onSlideChange).toHaveBeenCalledWith(1, 2);
235+
expect(activeSlide.setValue).toHaveBeenLastCalledWith(new Big(2));
236+
});
237+
238+
it("ignores a trailing drag that arrives before the programmatic scroll reports landing", () => {
239+
// The ordering that defeated the previous guard, taken from the Android CI video: on the
240+
// re-entered page NEXT moves slide 2 -> 3, and the settling list emits its trailing
241+
// scrollBeginDrag BEFORE the scroll's own momentum end. That drag clears the pending
242+
// target, so nothing records that a programmatic scroll ever landed — the old guard
243+
// required such a record and was skipped, sending the attribute 3 -> 1 and reporting a
244+
// change, which is what the flow's "Active slide: 3" assertion saw as "Active slide: 1".
245+
const { getByTestId, activeSlide, onSlideChange } = setup(2);
246+
247+
fireEvent.press(getByTestId("intro$buttonNext"));
248+
expect(activeSlide.setValue).toHaveBeenLastCalledWith(new Big(3));
249+
226250
act(() => {
227251
fireEvent(getByTestId("intro"), "scrollBeginDrag", {});
228252
fireEvent(getByTestId("intro"), "momentumScrollEnd", {
229253
nativeEvent: { contentOffset: { x: 0 } }
230254
});
231255
});
232256

233-
expect(onSlideChange).toHaveBeenCalledWith(0, 2);
234-
expect(activeSlide.setValue).toHaveBeenLastCalledWith(new Big(1));
257+
expect(activeSlide.setValue).toHaveBeenLastCalledWith(new Big(3));
258+
expect(onSlideChange).toHaveBeenCalledTimes(1);
259+
expect(onSlideChange).toHaveBeenCalledWith(2, 1);
260+
expect(getByTestId("intro$buttonDone")).toBeTruthy();
235261
});
236262

237263
it("ignores a stale programmatic scroll that reports the pre-scroll offset", () => {

0 commit comments

Comments
 (0)