diff --git a/__tests__/core/checkFinishedScroll.test.ts b/__tests__/core/checkFinishedScroll.test.ts index f331d07a..e9b5fb01 100644 --- a/__tests__/core/checkFinishedScroll.test.ts +++ b/__tests__/core/checkFinishedScroll.test.ts @@ -4,8 +4,10 @@ import "../setup"; import { interruptUserScroll } from "../../src/core/cancelScroll"; import { checkFinishedScroll, checkFinishedScrollFallback } from "../../src/core/checkFinishedScroll"; import { doScrollTo } from "../../src/core/doScrollTo.native"; +import { finishScrollTo } from "../../src/core/finishScrollTo"; import { getScrollRequestTracker } from "../../src/core/scrollRequestTracker"; import { Platform } from "../../src/platform/Platform"; +import { requestAdjust } from "../../src/utils/requestAdjust"; import { createMockContext } from "../__mocks__/createMockContext"; describe("checkFinishedScrollFallback", () => { @@ -725,6 +727,139 @@ describe("checkFinishedScrollFallback", () => { expect(ctx.state.didFinishInitialScroll).toBe(true); }); + it("returns a silent Android retry to its target when the scroll finishes before the retry frame", () => { + Platform.OS = "android"; + const frames: FrameRequestCallback[] = []; + globalThis.requestAnimationFrame = ((callback: FrameRequestCallback) => { + frames.push(callback); + return frames.length; + }) as typeof globalThis.requestAnimationFrame; + const originalCancelAnimationFrame = globalThis.cancelAnimationFrame; + globalThis.cancelAnimationFrame = ((handle: number) => { + frames[handle - 1] = () => {}; + }) as typeof globalThis.cancelAnimationFrame; + const scrollToCalls: Array<{ animated: boolean; x: number; y: number }> = []; + const ctx = createMockContext( + {}, + { + didContainersLayout: true, + hasScrolled: false, + initialScrollSession: { + completion: { + didDispatchNativeScroll: true, + watchdog: { + targetOffset: 220, + }, + }, + kind: "offset", + previousDataLength: 0, + } as any, + refScroller: { + current: { + scrollTo: (params: { animated: boolean; x: number; y: number }) => scrollToCalls.push(params), + }, + } as any, + scroll: 220, + scrollingTo: { + animated: false, + index: 99, + isInitialScroll: true, + offset: 220, + targetOffset: 220, + viewOffset: 0, + } as any, + scrollPending: 220, + }, + ); + + try { + checkFinishedScrollFallback(ctx); + flushTimers(1); + expect(scrollToCalls).toEqual([{ animated: false, x: 0, y: 219 }]); + + // Another completion check finishes the scroll while the retry frame is still pending. + finishScrollTo(ctx); + for (const frame of frames.splice(0)) { + frame(0); + } + + // The list returns to its target once, instead of staying one point away from it. + expect(scrollToCalls).toEqual([ + { animated: false, x: 0, y: 219 }, + { animated: false, x: 0, y: 220 }, + ]); + expect(ctx.state.scrollingTo).toBeUndefined(); + } finally { + globalThis.cancelAnimationFrame = originalCancelAnimationFrame; + } + }); + + it("returns a silent Android retry to the target an adjustment moved while the nudge was pending", () => { + Platform.OS = "android"; + const frames: FrameRequestCallback[] = []; + globalThis.requestAnimationFrame = ((callback: FrameRequestCallback) => { + frames.push(callback); + return frames.length; + }) as typeof globalThis.requestAnimationFrame; + const originalCancelAnimationFrame = globalThis.cancelAnimationFrame; + globalThis.cancelAnimationFrame = ((handle: number) => { + frames[handle - 1] = () => {}; + }) as typeof globalThis.cancelAnimationFrame; + const scrollToCalls: Array<{ animated: boolean; x: number; y: number }> = []; + const ctx = createMockContext( + {}, + { + didContainersLayout: true, + hasScrolled: false, + initialScrollSession: { + completion: { + didDispatchNativeScroll: true, + watchdog: { + targetOffset: 220, + }, + }, + kind: "offset", + previousDataLength: 0, + } as any, + refScroller: { + current: { + scrollTo: (params: { animated: boolean; x: number; y: number }) => scrollToCalls.push(params), + }, + } as any, + scroll: 220, + scrollingTo: { + animated: false, + index: 99, + isInitialScroll: true, + offset: 220, + targetOffset: 220, + viewOffset: 0, + } as any, + scrollPending: 220, + }, + ); + + try { + checkFinishedScrollFallback(ctx); + flushTimers(1); + expect(scrollToCalls).toEqual([{ animated: false, x: 0, y: 219 }]); + + // Rows above the target measure smaller while the nudge is pending, which moves the target up. + requestAdjust(ctx, -36, "item-size"); + for (const frame of frames.splice(0)) { + frame(0); + } + + // The retry returns to the moved target instead of the offset recorded before the adjustment. + expect(scrollToCalls).toEqual([ + { animated: false, x: 0, y: 219 }, + { animated: false, x: 0, y: 184 }, + ]); + } finally { + globalThis.cancelAnimationFrame = originalCancelAnimationFrame; + } + }); + it("does not finish a silent iOS bootstrap dispatch before native scroll is observed", () => { Platform.OS = "ios"; const scrollToCalls: Array<{ animated: boolean; x: number; y: number }> = []; diff --git a/__tests__/utils/requestAdjust.oldarch.cases.ts b/__tests__/utils/requestAdjust.oldarch.cases.ts index fb8123a9..7b26c239 100644 --- a/__tests__/utils/requestAdjust.oldarch.cases.ts +++ b/__tests__/utils/requestAdjust.oldarch.cases.ts @@ -50,4 +50,34 @@ describe("requestAdjust old-architecture cases", () => { expect(ctx.state.ignoreScrollFromMVCP).toEqual({ lt: 175 }); expect(scheduledDelay).toBe(250); }); + + it("reveals the list after a queued Android data-change workaround runs", () => { + Platform.OS = "android"; + const ctx = createMockContext( + { readyToRender: false }, + { + didContainersLayout: true, + didFinishInitialScroll: true, + props: { horizontal: false }, + scroll: 100, + scrollAdjustHandler: { + getAdjust: () => 0, + requestAdjust: mock(), + } as any, + }, + ); + const originalRAF = globalThis.requestAnimationFrame; + const frames: Array<() => void> = []; + globalThis.requestAnimationFrame = ((callback: () => void) => frames.push(callback)) as any; + + try { + requestAdjust(ctx, 150, "data"); + frames[0](); + } finally { + globalThis.requestAnimationFrame = originalRAF; + } + + expect(doScrollTo.mock.calls.at(-1)?.[1]).toEqual({ horizontal: false, isAdjustment: true, offset: 250 }); + expect(ctx.values.get("readyToRender")).toBe(true); + }); }); diff --git a/__tests__/utils/requestAdjust.test.ts b/__tests__/utils/requestAdjust.test.ts index 501a0463..b2228b8f 100644 --- a/__tests__/utils/requestAdjust.test.ts +++ b/__tests__/utils/requestAdjust.test.ts @@ -1,10 +1,13 @@ import { afterEach, beforeEach, describe, expect, it, spyOn } from "bun:test"; import "../setup"; // Import global test setup +import { doScrollTo } from "../../src/core/doScrollTo.native"; +import { initialScrollWatchdog } from "../../src/core/initialScrollSession"; import { Platform } from "../../src/platform/Platform"; import type { StateContext } from "../../src/state/state"; import type { InternalState } from "../../src/types.internal"; import { requestAdjust } from "../../src/utils/requestAdjust"; +import { setInitialRenderState } from "../../src/utils/setInitialRenderState"; import { createMockContext } from "../__mocks__/createMockContext"; describe("requestAdjust", () => { @@ -185,6 +188,66 @@ describe("requestAdjust", () => { expect(scrollAdjustHandlerCalls[0]).toBe(25); }); + it("keeps the list hidden until an adjustment queued before it was ready is applied", () => { + // An item above the initial scroll target measured smaller than its estimate while the list was hidden + mockState.didFinishInitialScroll = false; + mockCtx.values.set("readyToRender", false); + const adjustCountsAtLoad: number[] = []; + mockState.props.onLoad = () => adjustCountsAtLoad.push(scrollAdjustHandlerCalls.length); + + requestAdjust(mockCtx, -45, "item-size"); + setInitialRenderState(mockCtx, { didInitialScroll: true }); + + expect(mockCtx.values.get("readyToRender")).toBe(false); + + rafCallbacks[0](); + + expect(scrollAdjustHandlerCalls).toEqual([-45]); + expect(mockCtx.values.get("readyToRender")).toBe(true); + expect(adjustCountsAtLoad).toEqual([1]); + }); + + it("becomes ready after the last of several queued adjustments", () => { + mockState.didContainersLayout = mockState.didFinishInitialScroll = true; + mockCtx.values.set("readyToRender", false); + + requestAdjust(mockCtx, 10); + requestAdjust(mockCtx, 5); + rafCallbacks[0](); + + expect(mockCtx.values.get("readyToRender")).toBe(false); + + rafCallbacks[1](); + + expect(scrollAdjustHandlerCalls).toEqual([10, 5]); + expect(mockCtx.values.get("readyToRender")).toBe(true); + }); + + it("skips a deferred adjustment that a later native scroll already includes", () => { + // Positions move right away, so a native scroll issued before the frame already targets the + // adjusted offset. Moving the anchor as well would shift the list a second time. + mockCtx.values.set("readyToRender", false); + mockState.refScroller = { current: { scrollTo: () => {} } } as any; + + requestAdjust(mockCtx, -36, "item-size"); + doScrollTo(mockCtx, { offset: mockState.scroll }); + rafCallbacks[0](); + + expect(scrollAdjustHandlerCalls).toHaveLength(0); + expect(mockState.adjustingFromInitialMount).toBe(0); + }); + + it("applies a deferred adjustment when no native scroll lands before its frame", () => { + mockCtx.values.set("readyToRender", false); + mockState.refScroller = { current: { scrollTo: () => {} } } as any; + + doScrollTo(mockCtx, { offset: mockState.scroll }); + requestAdjust(mockCtx, -36, "item-size"); + rafCallbacks[0](); + + expect(scrollAdjustHandlerCalls).toEqual([-36]); + }); + it("should handle undefined readyToRender as falsy", () => { mockState.didContainersLayout = mockState.didFinishInitialScroll = true; mockCtx.values.delete("readyToRender"); @@ -196,6 +259,43 @@ describe("requestAdjust", () => { }); }); + describe("in-flight initial scroll targets", () => { + const startInitialScroll = () => { + mockState.scrollingTo = { + animated: false, + index: 37, + isInitialScroll: true, + offset: 1618, + targetOffset: 1578, + viewOffset: 40, + } as any; + mockState.initialScrollSession = { + completion: { watchdog: { startScroll: 0, targetOffset: 1578 } }, + kind: "bootstrap", + } as any; + }; + + it("moves a native initial scroll target with the adjustment", () => { + // Retries reissue the stored target, which has to follow the content like state.scroll does. + startInitialScroll(); + + requestAdjust(mockCtx, -36, "item-size"); + + expect(mockState.scrollingTo).toMatchObject({ offset: 1582, targetOffset: 1542 }); + expect(initialScrollWatchdog.get(mockState)?.targetOffset).toBe(1542); + }); + + it("leaves web initial scroll targets to the index-based retargeting", () => { + Platform.OS = "web"; + startInitialScroll(); + + requestAdjust(mockCtx, -36, "item-size"); + + expect(mockState.scrollingTo).toMatchObject({ offset: 1618, targetOffset: 1578 }); + expect(initialScrollWatchdog.get(mockState)?.targetOffset).toBe(1578); + }); + }); + describe("MVCP ignore logic", () => { const requestDataAdjust = (positionDiff: number) => requestAdjust(mockCtx, positionDiff, "data"); diff --git a/src/core/cancelImperativeScroll.ts b/src/core/cancelImperativeScroll.ts index daa37545..5f63208a 100644 --- a/src/core/cancelImperativeScroll.ts +++ b/src/core/cancelImperativeScroll.ts @@ -2,7 +2,9 @@ import { type StateContext, set$ } from "@/state/state"; type InternalState = StateContext["state"]; -export function cancelScrollCompletionChecks({ scheduledWork }: InternalState) { +export function cancelScrollCompletionChecks(state: InternalState) { + const { scheduledWork } = state; + state.silentInitialScrollReturn = undefined; scheduledWork.cancel("checkFinishedScrollFrame"); scheduledWork.cancel("checkFinishedScrollRetryFrame"); scheduledWork.cancel("checkFinishedScrollFallback"); diff --git a/src/core/checkFinishedScroll.ts b/src/core/checkFinishedScroll.ts index 43a8a031..5dd3495a 100644 --- a/src/core/checkFinishedScroll.ts +++ b/src/core/checkFinishedScroll.ts @@ -143,6 +143,10 @@ function checkFinishedScrollFrame(ctx: StateContext) { } function scrollToFallbackOffset(ctx: StateContext, offset: number, animated = false) { + if (Platform.OS !== "web") { + // Like doScrollTo, a retry targets an offset that already includes the adjustments requested so far. + ctx.state.nativeScrollEpoch = (ctx.state.nativeScrollEpoch ?? 0) + 1; + } ctx.state.refScroller.current?.scrollTo({ animated, ...getNativeScrollPoint(ctx, offset) }); } @@ -230,10 +234,13 @@ export function checkFinishedScrollFallback(ctx: StateContext) { : targetOffset + SILENT_INITIAL_SCROLL_TARGET_EPSILON; initialScrollCompletion.markSilentInitialScrollRetry(state); scrollToFallbackOffset(ctx, jiggleOffset); - state.scheduledWork.frame( - () => scrollToFallbackOffset(ctx, targetOffset), - "checkFinishedScrollRetryFrame", - ); + // Adjustments can move the target while the nudge is pending, so return to its current offset. + const returnToTarget = () => { + state.silentInitialScrollReturn = undefined; + scrollToFallbackOffset(ctx, initialScrollWatchdog.get(state)?.targetOffset ?? targetOffset); + }; + state.silentInitialScrollReturn = returnToTarget; + state.scheduledWork.frame(returnToTarget, "checkFinishedScrollRetryFrame"); scheduleFallbackCheck(SILENT_INITIAL_SCROLL_RETRY_DELAY_MS); } else if (shouldRetryUnalignedEndScroll) { scrollToFallbackOffset(ctx, completionState.clampedTargetOffset, !!isStillScrollingTo.animated); diff --git a/src/core/doScrollTo.native.ts b/src/core/doScrollTo.native.ts index 80a8b5da..1f69de8c 100644 --- a/src/core/doScrollTo.native.ts +++ b/src/core/doScrollTo.native.ts @@ -17,6 +17,8 @@ export function doScrollTo(ctx: StateContext, params: DoScrollToParams) { if (!params.isAdjustment) { // Explicit requests own their momentum; MVCP re-assertions keep the gesture's ownership. state.userScrollContentSize = undefined; + // The offset is computed from the current positions, so it already includes every adjustment requested so far. + state.nativeScrollEpoch = (state.nativeScrollEpoch ?? 0) + 1; } scroller.scrollTo({ animated: isAnimated, ...getNativeScrollPoint(ctx, offset, horizontal) }); diff --git a/src/core/finishScrollTo.ts b/src/core/finishScrollTo.ts index f932a8e4..1a61e81e 100644 --- a/src/core/finishScrollTo.ts +++ b/src/core/finishScrollTo.ts @@ -9,6 +9,9 @@ import type { StateContext } from "@/state/state"; export function finishScrollTo(ctx: StateContext) { const state = ctx.state; if (state?.scrollingTo) { + // A silent-dispatch retry nudges the native offset away and returns on the next frame. Another check can + // finish the scroll first, so return now instead of leaving the list one point off its target. + state.silentInitialScrollReturn?.(); cancelScrollCompletionChecks(state); // An older active scroll can finish while its replacement still waits for layout. // Its completion must not settle or release the newer request's ownership. diff --git a/src/types.internal.ts b/src/types.internal.ts index b77d7214..70f2a6d6 100644 --- a/src/types.internal.ts +++ b/src/types.internal.ts @@ -252,6 +252,10 @@ export interface InternalState { // Content size at release while a user gesture awaits settlement. userScrollContentSize?: number; layoutAdjustmentEpoch?: number; + /** Counts absolute native scrolls, so an adjustment deferred before one of them is not applied again. */ + nativeScrollEpoch?: number; + /** Moves a silently dispatched initial scroll back onto its target after the retry nudged it away. */ + silentInitialScrollReturn?: () => void; endFollowInterrupted?: boolean; visibleContentAnchor?: { key: string; offset: number; sizeOnly?: true }; maintainingScrollAtEnd?: MaintainingScrollAtEndState; diff --git a/src/utils/requestAdjust.ts b/src/utils/requestAdjust.ts index 3a9f44fa..ce0598b1 100644 --- a/src/utils/requestAdjust.ts +++ b/src/utils/requestAdjust.ts @@ -1,7 +1,9 @@ import { IsNewArchitecture } from "@/constants-platform"; +import { initialScrollWatchdog } from "@/core/initialScrollSession"; import { Platform } from "@/platform/Platform"; import { peek$, type StateContext } from "@/state/state"; import type { ScrollAdjustmentSource } from "@/types.internal"; +import { setInitialRenderState } from "@/utils/setInitialRenderState"; export function requestAdjust(ctx: StateContext, positionDiff: number, source?: ScrollAdjustmentSource) { const state = ctx.state; @@ -16,10 +18,6 @@ export function requestAdjust(ctx: StateContext, positionDiff: number, source?: ctx.doScrollTo!({ horizontal: state.props.horizontal, isAdjustment: true, offset: state.scroll }); } else { state.scrollAdjustHandler.requestAdjust(positionDiff); - - if (state.adjustingFromInitialMount) { - state.adjustingFromInitialMount--; - } } }; state.scroll += positionDiff; @@ -28,6 +26,7 @@ export function requestAdjust(ctx: StateContext, positionDiff: number, source?: state.userScrollContentSize += positionDiff; } state.scrollForNextCalculateItemsInView = undefined; + shiftNativeInitialScrollTarget(state, positionDiff); const readyToRender = peek$(ctx, "readyToRender"); @@ -71,7 +70,38 @@ export function requestAdjust(ctx: StateContext, positionDiff: number, source?: } } else { state.adjustingFromInitialMount = (state.adjustingFromInitialMount || 0) + 1; - requestAnimationFrame(doit); + const nativeScrollEpoch = state.nativeScrollEpoch; + requestAnimationFrame(() => { + // A native scroll issued in the meantime already landed on the adjusted offset. Moving the + // anchor as well would apply the adjustment a second time. + if (state.nativeScrollEpoch === nativeScrollEpoch) { + doit(); + } + state.adjustingFromInitialMount = (state.adjustingFromInitialMount || 1) - 1; + if (!state.adjustingFromInitialMount) { + setInitialRenderState(ctx, {}); + } + }); } } } + +/** + * Native retries of an initial scroll reuse the offset recorded when it was dispatched. The adjustment moves + * state.scroll with the content, so the recorded target has to move with it. Web retargets from the index instead. + */ +function shiftNativeInitialScrollTarget(state: StateContext["state"], positionDiff: number) { + const scrollingTo = state.scrollingTo; + if (Platform.OS === "web" || !scrollingTo?.isInitialScroll || scrollingTo.animated) { + return; + } + + scrollingTo.offset += positionDiff; + if (scrollingTo.targetOffset !== undefined) { + scrollingTo.targetOffset += positionDiff; + } + const watchdog = initialScrollWatchdog.get(state); + if (watchdog) { + watchdog.targetOffset += positionDiff; + } +} diff --git a/src/utils/setInitialRenderState.ts b/src/utils/setInitialRenderState.ts index b47ed2b0..f813da26 100644 --- a/src/utils/setInitialRenderState.ts +++ b/src/utils/setInitialRenderState.ts @@ -56,7 +56,11 @@ export function setInitialRenderState( state.didFinishInitialScroll = true; } - const isReadyToRender = Boolean(state.didContainersLayout && state.didFinishInitialScroll); + // An adjustment requested before the list was ready reaches the scroll view on the next frame. Revealing the + // list earlier would show items at their unadjusted offset for a frame. + const isReadyToRender = Boolean( + state.didContainersLayout && state.didFinishInitialScroll && !state.adjustingFromInitialMount, + ); if (isReadyToRender && !peek$(ctx, "readyToRender")) { set$(ctx, "readyToRender", true); setAdaptiveRender(ctx, "normal", "ready");