Skip to content

fix: attach the rebuilt sticky transform node, and stop flooring averaged size estimates - #524

Open
byldrma3 wants to merge 2 commits into
LegendApp:mainfrom
byldrma3:fix/sticky-position-remount-and-estimate-rounding
Open

fix: attach the rebuilt sticky transform node, and stop flooring averaged size estimates#524
byldrma3 wants to merge 2 commits into
LegendApp:mainfrom
byldrma3:fix/sticky-position-remount-and-estimate-rounding

Conversation

@byldrma3

Copy link
Copy Markdown

Why

Two independent bugs, both in size/position estimation.

1. Sticky headers detach from their section after a data change — addresses #445, which is still reproducing on 3.3.3 according to the last few reports in that thread.

PositionViewSticky builds its transform by interpolating animatedScrollY over an input/output range derived from the container's position. When position changes — which is exactly what happens on any data change that shifts a section — the memo produces a brand new interpolation node, but the Animated.View stays mounted and keeps the node it attached on mount. The header then keeps following the range computed for its previous position, so it paints at the wrong offset and looks detached from its section.

That also explains why every workaround in #445 is some variant of "put a key on the list": remounting is currently the only way to get Animated to pick up the new node.

2. Averaged size estimates are biased low.

roundSize floors to the nearest eighth of a pixel. That's the conservative choice for a size you actually measured, but getItemSize also uses it for averaged estimates, and those get summed across every unmeasured item. Flooring loses up to 1/8px per item in one direction only, so the error accumulates instead of cancelling out: with an average of 80.1, 1000 items estimate to 80,000 instead of 80,100 — 100px of content size that isn't there. The comment on roundSize already says "round to nearest … to avoid accumulating rounding errors", which is what the estimate path wants.

How

  • PositionView.native.tsx — key the sticky Animated.View on position, so a changed position remounts the view and Animated attaches the freshly built interpolation node. Scoped to the sticky branch only; PositionViewState and PositionViewAnimated apply position through style and are unaffected.
  • helpers.ts / getItemSize.ts — add roundEstimatedSize (nearest eighth) and use it for the two averaged-estimate paths. roundSize keeps flooring for measured sizes, so updateItemSizes and useContainerMeasurement are untouched.

Two things worth flagging:

  • Keying remounts the sticky item's subtree when its position changes. That's bounded — position changes on data/layout changes, not per scroll frame, since scrolling itself is handled by the transform — but if you'd rather not remount user content, the deeper fix is to keep the interpolation stable and drive the offset separately. Happy to rework it that way if you prefer.
  • I left integrations/reanimated.tsx alone; it has its own sticky implementation and I can't verify it against the same repro.

Test Plan

  • bun test1586 pass, 0 fail (1583 before, 3 added)
  • bunx biome check ./src ./__tests__ — clean
  • bun run tsc:src — clean

New PositionView.native test asserts that when containerPosition changes, the interpolation node is rebuilt and the view remounts. On main it fails showing exactly the bug: the style already carries the new inputRange/outputRange while the view is never remounted, so Animated is still driving the old node.

Expected number of calls: 2
Received number of calls: 1
(fail) PositionView.native > rebuilds the sticky transform node when the container position changes

New getItemSize tests cover the rounding: an 80.1 average resolves to 80.125 instead of 80, and summing 1000 estimates stays within a few px of the true total instead of drifting 100px low.

Beyond the unit tests, both fixes have been running as a patch on 3.3.3 in an app that uses SectionList with stickyHeaderIndices over data that changes — which is how I hit #445 in the first place.

The sticky transform interpolates animatedScrollY over a range derived from the
container position, so a new interpolation node is built whenever that position
changes. Animated keeps the node it attached on mount, so the header kept
following the range of its previous position and drifted away from its section
after any data change.

Key the Animated.View on position so the new node gets attached.
roundSize floors to the nearest eighth, which is right for measured sizes but
biases every averaged estimate low. Those estimates are summed across all
unmeasured items, so the error accumulates over long lists.

Add roundEstimatedSize, which rounds to the nearest eighth, and use it for the
average and average-snapshot paths in getItemSize.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 875e0dac83

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

// Keying on position remounts the view and lets Animated attach the new node.
return (
<Animated.View ref={refView} style={viewStyle} {...rest}>
<Animated.View key={`sticky-pos:${position}`} ref={refView} style={viewStyle} {...rest}>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve sticky header state across position updates

When a native sticky header keeps the same itemKey but its position changes, such as after rows above it are inserted, removed, or remeasured, this key makes React unmount and recreate the entire header subtree. That means uncontrolled inputs, focus, animations, or local component state inside a sticky section header are lost even when the row itself was not recycled; the fix should avoid remounting user content just to attach the rebuilt Animated interpolation node.

Useful? React with 👍 / 👎.

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.

1 participant