Skip to content

fix: recompute full layout range on initial scroll adjustment - #2444

Open
theRizwan wants to merge 1 commit into
Shopify:mainfrom
theRizwan:fix/initial-scroll-index-layout-order
Open

theRizwan wants to merge 1 commit into
Shopify:mainfrom
theRizwan:fix/initial-scroll-index-layout-order

Conversation

@theRizwan

Copy link
Copy Markdown

Fixes #2307

The problem

applyInitialScrollAdjustment recomputes layouts for [0, initialScrollIndex] and stops there. Items are positioned from their predecessor, so this moves the target item without moving anything after it, and the layout array is left out of order at that boundary.

getVisibleLayouts binary searches that array. binarySearchVisibleIndex documents that it assumes the array is sorted, so once it is not, the search does not return a slightly wrong answer, it returns an unrelated one.

With 600 items of 300px and initialScrollIndex={250}:

seeded      item 250 at 50000    item 251 at 50200
measured    item 250 at 50100    item 251 at 50400
corrected   item 250 at 75000    item 251 at 50400   <- steps backwards

The search for offset 75000 then returns item 333, which matches what the issue reports on device.

The gap only opens when items are taller than the 200px the average window starts at. Below that the corrective pass moves items to a lower position, which leaves a forward gap and keeps the array sorted, which is why smaller items never show the bug.

The change

Recompute to the end of the list instead of stopping at initialScrollIndex.

This is the same range the layout managers already use when window size changes, and Masonry already recomputes to the end regardless of the range it is given. _recomputeLayouts has tail repair for partial recomputes, but this call site goes to the public recomputeLayouts and bypasses it, and its condition compares the last item rather than the boundary item so it would not fire here anyway.

I kept the change at the call site because the ordering only breaks when the prefix is corrected against a changed average, and positions chain, so anything after the target genuinely has to be recomputed. Happy to move it into _recomputeLayouts as a boundary check instead if you would rather have it there, though that path is shared with Masonry where offsets are not ordered by index by design.

Test

src/__tests__/initialScrollIndex2307.test.tsx renders the case from the issue with items measuring 300px and asserts item 250 is rendered and item 333 is not. It fails on main and passes with this change.

Full suite is green, 188 tests across 15 suites, and type check passes.

@theRizwan

Copy link
Copy Markdown
Author

I have signed the CLA!

pupaxxo pushed a commit to one-am-it/react-native-simple-image-slider that referenced this pull request Sep 8, 2026
… photo

The mispark is upstream, not ours: Shopify/flash-list#2307, open and P1.
`applyInitialScrollAdjustment` recomputes layouts only as far as the opening index,
which leaves the layout array out of order at that boundary — and the binary search
in `getVisibleLayouts` assumes it is sorted, so it resolves an unrelated item. It
needs items larger than the 200pt default estimate to bite, which a full-width slide
always is, and it arrived in 2.3.1 with the reorder that reads the target offset
after recomputing rather than before.

Measured here on the example's gallery, twenty-two slides at 420pt opening on index
14: the scroll is *correct* — `offset 5880`, and `getLayout(14).x` is also 5880 — but
the content is 6380 against a true 9240 and the item resolved under that offset is 21.
Every signal from outside reads healthy, which is why this looked like a timing
problem for so long.

This carries the one line from the upstream fix, Shopify/flash-list#2444:

    recomputeLayouts(0, initialScrollIndex)
    → recomputeLayouts(0, this.getDataLength() - 1)

Nothing in `src/` changes. The slider stays exactly as published, and the patch goes
when a release carries the fix.

The version is also locked now. `example/package.json` asked for 2.3.2 while the
lockfile still resolved 2.0.2, so a clean install produced the version where the
defect does not exist — the reproduction would have quietly stopped reproducing. The
root devDependency moves to 2.3.2 as well, so the library is typechecked against the
version its consumers actually get.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flashlist v2 initialScrollIndex renders wrong items when item size exceeds 200

1 participant