Skip to content

fix(template): keep scroll offset when using keepScrolledIndexOnPrepend - #1896

Open
hoebbelsB wants to merge 7 commits into
mainfrom
fix-keep-scrolled-index
Open

hoebbelsB wants to merge 7 commits into
mainfrom
fix-keep-scrolled-index

Conversation

@hoebbelsB

@hoebbelsB hoebbelsB commented Jan 2, 2026

Copy link
Copy Markdown
Member

Extension of #1859, as I couldn't push to the fork.

Fixes #1857.

Problem

keepScrolledIndexOnPrepend compensated the scroll position by counting how many items were inserted ahead of the anchored item. That only holds for a pure prepend.

A real reverse infinite scroller — a chat client loading older messages — also renders a transient "loading older messages" row while the request is in flight, so the update that resolves it is an insert and a remove. Counting insertions mis-compensates by the height of that row and the list jumps.

Approach

Locate the anchored item again via trackBy instead of counting insertions. That nets out inserts, removals and moves ahead of the anchor in a single pass.

The compensation then shifts by the resulting delta rather than scrolling to the top of the new anchor index — the latter drops the anchor's sub-item offset, which is the jump reported in #1857.

Also fixed

Validating the above against a realistic demo surfaced several further defects in the same area. Each has its own regression spec.

Symptom Cause
Empty list, nothing rendered at all The strategies only compute a range once containerRect$ emits, and it was fed exclusively by a ResizeObserver — which never delivers its first entry without a rendering frame (hidden or fully occluded tab). Now seeded with a synchronous measurement on init.
Views stacked at the end of the content, gaps in the viewport A prepend shifts the range while the rendered slice stays identical, so the differ reports no changes and rendering bailed out via NEVER. Now re-renders in place — and announces the batch correctly, since an empty batch made the strategies' position pass fast-forward its cursor past views that still emit afterwards.
Overlapping items and gaps that never recover while scrolling Autosize booked measurements through view.context.index, which goes stale when a second emission rebuilds the size ledger while a render or ResizeObserver pass for the previous dataset is still in flight. Because sizes are cached, that corruption was permanent. Now resolved through the trackBy cache.
Stale loading row frozen on screen waitForScroll blocks rendering until a scroll event arrives, but a target outside the scrollable bounds is clamped by the browser and emits no event — latching isStable to false forever. Now decided against the clamped target.
Viewport pinned to the top, reloading forever When the anchored item itself disappears — the anchor sits on the transient row, which happens whenever the user waits at the very top — the compensation bailed out entirely. Now falls back to the nearest surviving item below it.

Additionally, autosize applies the range shrink independently of the relocation (a transient row that was both the anchor and the last item of the rendered range used to wedge a pending scrollToIndex), and render/measure passes tolerate indices belonging to a superseded dataset instead of dereferencing a removed ledger entry.

Testing

nx run template:component-test82 specs, 23 of them new, across the fixed, dynamic and autosize strategies. Each fix listed above has a spec that was verified to fail against the unfixed code.

Demo

apps/demosVirtual ScrollingReverse Infinite Scroll was reworked to behave like an actual chat client, which is what surfaced most of the defects above:

  • a transient loading row inside the list (toggleable) and a slower simulated backend, so the in-flight state is actually observable
  • history requests driven by user input — wheel upwards, or arriving at the very top — instead of range emissions. Passive scroll changes (initial scroll, prepend compensations, measurement corrections) all report a range starting at 0 and would otherwise chain-load without any interaction. Wheel events are debounced to one trigger per gesture, since trackpad momentum keeps emitting for seconds after the flick.
  • history runs dry at the beginning of the conversation instead of refetching the first page forever

Known limitation

Scrolling upward into items that have never been measured can still produce a visible correction: the anchor walk uses tombstoneSize estimates, and maybeAdjustScrollPosition applies the real geometry a frame later, mid-gesture. With high per-item size variance this reads as rubber-banding near the top.

Setting tombstoneSize close to the real mean item height reduces it substantially, and that is worth documenting for reverse infinite scrollers. Damping corrections while a gesture is in progress would address the remainder, but that is a behavioural change to the scroll strategies and is left for a separate PR.

@github-actions github-actions Bot added the </> Template @rx-angular/template related label Jan 2, 2026
@nx-cloud

nx-cloud Bot commented Jan 2, 2026

Copy link
Copy Markdown

View your CI Pipeline Execution ↗ for commit 1ed5565

Command Status Duration Result
nx affected -t lint build test component-test e... ✅ Succeeded 30s View ↗
nx-cloud record -- npx nx format:check ✅ Succeeded 1s View ↗
nx build docs ✅ Succeeded 29s View ↗
nx build demos --configuration=production ✅ Succeeded 1s View ↗

💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗


☁️ Nx Cloud last updated this comment at 2026-08-03 21:55:18 UTC

@hoebbelsB
hoebbelsB force-pushed the fix-keep-scrolled-index branch from 843a285 to feeb9e1 Compare January 2, 2026 21:07
m-gericke and others added 6 commits July 31, 2026 18:57
This fix prevents items from jumping from the anchor items current offset to it's scrollTop when scrolling upwards.
The scroll strategies only compute a rendered range once `containerRect$`
emits, and that stream was fed exclusively by a `ResizeObserver`. The
observer delivers its first entry only when the browser produces a
rendering frame. A viewport created in a hidden or fully occluded tab
(chrome classifies a covered window as invisible) therefore never
received a container size and rendered nothing at all - permanently,
until the tab became visible again.

Measure the container synchronously in `ngAfterViewInit` and seed
`containerRect$` with it. Layout reads are available regardless of frame
production, and the ResizeObserver keeps the value up to date from there.

Co-Authored-By: Claude Fable 5 <[email protected]>
Prepending items shifts the rendered range by exactly the amount of
inserted items, which leaves the rendered slice identical. The
IterableDiffer reports no changes, so rendering bailed out through
`NEVER` - but every view now maps to a new index and has to be
re-positioned, leaving the list drawn at stale offsets.

Detect that case and re-render the views in place.

Two details matter for the strategies consuming these events:

- the batch handed to `renderingStart$` names every view index, because
  every view emits `viewRendered$` right after. An empty batch makes the
  strategies' position pass fast-forward its running cursor past views
  that still emit afterwards, stacking them at the end of the content -
  visible as a blank viewport with gaps between items.
- `viewRendered$` carries the view container index, not the context
  index. The two diverge as soon as `range.start > 0`.

Co-Authored-By: Claude Fable 5 <[email protected]>
`keepScrolledIndexOnPrepend` compensated the scroll position by counting
how many items were inserted before the anchor. That breaks as soon as
the update is not a pure insert - a chat client showing a transient
"loading older messages" row performs an insert *and* a remove, leaving
the list shifted by the height of that row (#1857).

Locate the anchored item again instead, via `trackBy`. That nets out
inserts, removals and moves ahead of the anchor in one pass, and shifts
by the resulting delta so the anchor keeps its sub-item offset - scrolling
to the top of the new anchor index is what produced the reported jump.

Further correctness fixes found while validating the above:

- when the anchored item itself disappears (the anchor sits *on* the
  transient row, which happens whenever the user waits at the very top),
  fall back to the nearest following survivor rather than bailing out.
  Bailing left the viewport pinned to the top, which in a reverse
  infinite scroller retriggers loading forever.
- autosize resolves the item index of a view through the trackBy cache
  instead of `view.context.index`. A second emission rebuilds the size
  ledger while a render or ResizeObserver pass for the previous dataset
  is still in flight, and the stale index booked measurements onto the
  neighbouring item. Since sizes are cached, that corruption was
  permanent and showed up as overlapping items and gaps while scrolling.
- `waitForScroll` is decided against the scroll target *clamped* to the
  scrollable bounds. A scroll the browser cannot perform emits no scroll
  event, so an out-of-bounds target latched `isStable` to false forever
  and froze rendering with a stale loading row on screen.
- autosize applies the range shrink independently of the relocation, so
  a transient row that was both the anchor and the last item of the
  rendered range no longer wedges a pending `scrollToIndex`.
- render and measure passes tolerate indices of a superseded dataset
  instead of dereferencing a removed ledger entry.

Adds cypress coverage for all of the above across the fixed, dynamic and
autosize strategies, including the render-without-ResizeObserver-frames
and range-shift cases from the preceding two commits.

Co-Authored-By: Claude Fable 5 <[email protected]>
Make the demo behave like an actual chat client so it exercises the
prepend compensation the way real consumers do:

- a transient "loading older messages" row inside the list while a
  history request is in flight, toggleable, plus a slower simulated
  backend so that state is actually observable
- history requests are driven by user input (wheel upwards, or arriving
  at the very top) rather than by range emissions. Passive scroll
  changes - the initial scroll, prepend compensations, measurement
  corrections - all report a range starting at 0 and would otherwise
  chain-load without any interaction. Wheel events are debounced to one
  trigger per gesture, since trackpad momentum keeps emitting for
  seconds after the flick.
- the batch runs dry at the beginning of the conversation instead of
  refetching the first page forever

Co-Authored-By: Claude Fable 5 <[email protected]>
@hoebbelsB
hoebbelsB force-pushed the fix-keep-scrolled-index branch from feeb9e1 to 1ed5565 Compare August 3, 2026 21:49

@nx-cloud nx-cloud 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.

Nx Cloud has identified a flaky task in your failed CI:

🔂 Since the failure was identified as flaky, we triggered a CI rerun by adding an empty commit to this branch.

Nx Cloud View detailed reasoning in Nx Cloud ↗


🎓 Learn more about Self-Healing CI on nx.dev

@G-iele

G-iele commented Aug 31, 2026

Copy link
Copy Markdown

@hoebbelsB when do you plan to merge this PR? ))

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

Labels

</> Template @rx-angular/template related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Virtual Scroll: scroll position jumps when using keepScrolledIndexOnPrepend

3 participants