fix(desktop): keep history prepend anchored on the visible row#1113
Closed
tlongwell-block wants to merge 3 commits into
Closed
fix(desktop): keep history prepend anchored on the visible row#1113tlongwell-block wants to merge 3 commits into
tlongwell-block wants to merge 3 commits into
Conversation
Co-authored-by: npub1mprnacetjua2xx3p5eddmhxyk6wv929ymm5py8kd2xfxurxahspqqlgyta <d8473ee32b973aa31a21a65adddcc4b69cc2a8a4dee8121ecd51926e0cddbc02@sprout-oss.stage.blox.sqprod.co> Signed-off-by: npub1mprnacetjua2xx3p5eddmhxyk6wv929ymm5py8kd2xfxurxahspqqlgyta <d8473ee32b973aa31a21a65adddcc4b69cc2a8a4dee8121ecd51926e0cddbc02@sprout-oss.stage.blox.sqprod.co>
…anchor invariant
Adds the regression gate for the channel scroll-jump fix in the previous
commit. Two pieces:
1. **Mock-bridge fixture in `e2eBridge.ts`**: a `history-jump` channel
seeded with 240 sequential messages, enough to force at least one
older-fetch round-trip past the initial 200-message load. A new
`historyDelayMs` config knob (wired through `helpers/bridge.ts`)
delays only the older-history `REQ`/`until` path — initial loads
stay snappy — so the test has a deterministic window to snapshot the
visible anchor row after the sentinel fires but before the prepended
rows commit.
2. **`messaging.spec.ts` invariant test** ("older-history prepend keeps
the visible row pinned"): real `mouse.wheel` events scroll near the
top sentinel, the anchor row is snapshotted by `[data-message-id]`
rect against the timeline container, the prepend is awaited via
first-id-changed + count-grew, and drift is asserted ≤ 4px after
the layout-effect restore settles. Accounts for `scrollTop` drift
between snapshots so it doesn't false-pass when `scrollBy` is wired
correctly but the user kept wheeling.
Also adds `scroll-prepend-stability.spec.ts` as a nightly-gated /
manual real-relay smoke artifact. Excluded from both the `smoke` and
`integration` `testMatch` arrays in `playwright.config.ts` so it never
runs in the default suite. Run manually with
`pnpm -C desktop exec playwright test scroll-prepend-stability.spec.ts`
against a relay started with `BUZZ_RECONCILE_CHANNELS=1`. The
top-of-file docblock spells out why the mock-bridge case is the
canonical gate (deterministic, no seed dependency) and why a green
Chromium run is necessary but not sufficient (WKWebView-specific
stale-`scrollTop` hazard requires manual macOS pass).
Verification: positive test green against the fixed build (~3s).
Negative test (restore neutered) fails as expected — confirms the gate
catches the regression. Both tsc and biome clean.
Co-authored-by: npub17jjz49l9jjmhhk7cac63j8yt9z555n9cw8vk7v5jz4vzw4ppld5qgj57cc <f4a42a97e594b77bdbd8ee35191c8b28a94a4cb871d96f32921558275421fb68@sprout-oss.stage.blox.sqprod.co>
Signed-off-by: npub17jjz49l9jjmhhk7cac63j8yt9z555n9cw8vk7v5jz4vzw4ppld5qgj57cc <f4a42a97e594b77bdbd8ee35191c8b28a94a4cb871d96f32921558275421fb68@sprout-oss.stage.blox.sqprod.co>
If `fetchOlder` resolves successfully without a committed prepend (zero new rows, all-duplicates page, or an early-returned fetch that no-ops), the layout effect never fires — its deps (`renderedFirstMessageId`, `renderedMessageCount`) don't change — and `pendingRestoreRef` stays set. The IntersectionObserver's `observe()` guards on `pendingRestoreRef.current` and silently refuses to re-arm, wedging older-history loading until the user navigates away. Most empty-page cases are already covered indirectly: `useFetchOlderMessages` flips `hasOlderMessages` to false when the relay returns fewer than the batch size or no new oldest, which triggers the outer effect's cleanup path. The wedge described above is the narrow case where the fetch resolves with no prepend AND `hasOlderMessages` stays true — e.g., a transient empty page from the relay during pagination. Convert the fetch handler from `.catch()` to a unified `.then(onFulfilled, onRejected)`. On fulfilled, wait one animation frame to give React time to commit any real prepend (the layout effect runs synchronously inside that commit and clears `pendingRestoreRef`). If our captured pending is still the live one after the frame, treat it as a no-commit fetch: clear pending and call `resumeObservingRef.current` to re-arm. Identity check (`pendingRestoreRef.current !== restorePending`) prevents a stale rAF from clobbering a fresh pending created by a subsequent intersect after a real commit re-armed the observer mid-frame. Reject path is unchanged in behavior. Follow-up to #1113 per Eva's PR review. Co-authored-by: npub17jjz49l9jjmhhk7cac63j8yt9z555n9cw8vk7v5jz4vzw4ppld5qgj57cc <f4a42a97e594b77bdbd8ee35191c8b28a94a4cb871d96f32921558275421fb68@sprout-oss.stage.blox.sqprod.co> Signed-off-by: npub17jjz49l9jjmhhk7cac63j8yt9z555n9cw8vk7v5jz4vzw4ppld5qgj57cc <f4a42a97e594b77bdbd8ee35191c8b28a94a4cb871d96f32921558275421fb68@sprout-oss.stage.blox.sqprod.co>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
Scrolling up in a channel to load older history yanks the viewport to the
oldest just-loaded message instead of keeping the row the user was reading
pinned to the same screen position. Recurring regression; previous "fixes"
relied on math against
container.scrollTop, which is stale on macOSWKWebView mid-wheel (root cause documented in
RESEARCH/BUZZ_SCROLL_PREPEND_2026-06-14.mdand Element/Matrixdocs/scrolling.md).The fix (two parts, in
useLoadOlderOnScroll.ts+MessageTimeline.tsx)Anchor on a real visible DOM node, not on a
scrollTopvalue.When the top sentinel intersects,
findVisibleMessageAnchorwalks[data-message-id]rows and picks the one whose top-offset is closestto
min(12px, containerHeight / 2)— the row the user is most likelyreading. We snapshot
{ id, topOffset, fallbackScrollHeight }in aref while we wait for the prepended rows to commit.
Restore via relative
scrollBy(delta), noscrollTopread.In a layout-effect keyed on the actual rendered first-id + count
changing (i.e., the prepend has committed), we read the anchor row's
new top-offset and call
restoreScrollBy(nextTopOffset - savedTopOffset).This is the difference between "passes headless Chromium" and
"actually fixed on macOS" — no stale read, no read-then-write.
MessageTimeline.tsxswitches the consumer fromrestoreScrollPosition(absolute write) to
restoreScrollBy(relative), and changes thefetch-older spinner from an in-flow block to an absolute overlay so the
spinner's own mount doesn't shove the topmost message down and confuse
the anchor math.
useTimelineScrollManager.tsadds therestoreScrollByprimitive(
container.scrollBy(0, delta)) alongside the existing scroll API.The regression gate
Mock-bridge invariant test in
messaging.spec.ts("older-historyprepend keeps the visible row pinned"):
history-jumpmock channel with 240 messages — enough toforce at least one older-fetch round-trip past the 200-msg initial
load.
historyDelayMsconfig knob delays only the older-historyREQwith
until(not initial loads), giving a deterministic snapshotwindow after the sentinel fires but before the prepend commits.
mouse.wheelevents, snapshots the anchor row by[data-message-id]rect against the timeline container, awaitsprepend commit via first-id-changed + count-grew, then asserts
≤ 4px drift after layout-effect restore.
scrollTopdrift between snapshots so it doesn'tfalse-pass when
scrollByis wired correctly but the user keptwheeling.
A nightly-gated
scroll-prepend-stability.spec.tsis included formanual real-relay smoke (excluded from both
smokeandintegrationtestMatchinplaywright.config.ts). Run manually withpnpm -C desktop exec playwright test scroll-prepend-stability.spec.tsagainst a relay started with
BUZZ_RECONCILE_CHANNELS=1.Verified
void delta) fails as expected —confirms the gate catches the regression.
pnpm typecheck+pnpm check(biome) clean.What's NOT done in this PR
A green CI run here is necessary, not sufficient. @tlongwell-block —
pulling this branch on macOS and confirming the jump is gone is the
load-bearing pass before this is "shipped."
fetchOlderresolves with zero new rows,hasCommittedPrependnever goes true, the layout effect early-returnsleaving
pendingRestoreRefset, and the observer never re-arms — olderloading silently wedges. Happy path is covered; this is a follow-up
commit on the same PR (resume observing if the fetch resolves with no
commit). Landed as
da1606c2— converts the fetch handler to.then(onFulfilled, onRejected)and uses a single-rAF settle fence withan identity check on the captured pending to re-arm only on a true
no-commit, never racing a real commit's restore.