fix(feed): preserve scroll when returning from posts

[rebelchris]

Changes

Readers could return from a post to the top of the feed when cached content took longer than two seconds to render, or when closing a post reopened through browser Back.

• Use one height-aware restoration helper for both feed navigation and modal close. Closing a modal saves the offset for the new destination history entry and waits until the document can hold it, including after a feed remount.
• Allow at most ten seconds for delayed rendering. Cancel earlier on user input, a changed native scroll position, browser Back/Forward, route changes, or cleanup. Growth after the deadline cannot trigger a jump. This deliberately permits restoration within that ten-second window, rather than waiting indefinitely.
• Observe the body when ResizeObserver is available; otherwise poll every 100 ms within the same deadline. Both paths clean up observers, timers, and event listeners.
• Consolidate history-key handling and saved positions in one store capped at 200 entries. Modal-origin offsets have their own namespace so scrolling the post cannot overwrite them. Missing or evicted modal positions do not force a scroll.
• Add hook regressions with browser-like scroll clamping and make absent posts and indices explicit in the modal navigation types. Both post selectors use the same checked result, including boosted posts.

Events

No new tracking events.

Experiment

No new experiments.

Browser Testing

Ran ten local fixture checks using Chromium 145.0.7632.6: desktop and Pixel 5 emulation, both with 4x CPU throttling. These execute the production restoration helper with real document layout, ResizeObserver, timers, scroll events, and browser history. They do not mock scrolling or layout.

• Before/after: a bare scroll to 5000 on a short document clamps below 5000; the shared helper reaches exactly 5000 when content grows after 2.5 seconds.
• Closing after native browser Back waits for delayed content and restores 5000.
• Native scrolling cancels restoration; document growth after the ten-second deadline does not move the reader.
• Delayed content also restores when ResizeObserver is unavailable.

These browser checks were run during development; their temporary Playwright suite, configuration, and package command are not included in this PR.

No physical-device test was performed. The PR preview was opened, but its home route redirected this browser session to onboarding, so the authenticated application feed has not been verified. The browser results above are local fixtures, not claims about a physical phone or the preview's cached feed.

Validation

• Shared hook, store, and feed component tests: 70 passed.
• Full webapp test suite: 92 suites, 719 tests passed; webapp pretest lint passed.
• Changed shared files: ESLint passed.
• Changed-file strict TypeScript check passed: node ./scripts/typecheck-strict-changed.js. All 13 modal navigation errors are fixed without suppressions.
• Full webapp TypeScript check: 12 existing errors, with the same files, error codes, and messages as main (05d796c).

Preview domain

https://codex-fix-feed-scroll-restoratio.preview.app.daily.dev

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
daily-webapp	Ready	Preview	Sep 17, 2026 9:42am UTC

[rebelchris]

Summary

Two mechanisms are changed here: useScrollRestoration swaps a 2s rAF budget for a ResizeObserver, and usePostModalNavigation starts keying the originating feed offset by history entry. The history-key keying is the right idea and the modal/back traces hold up. Two things I'd want resolved before merge, plus some consolidation asks.

Context I'd flag up front: this is the third pass at the same symptom (#6409, then the 1s → 2s budget now being deleted). Per the PR description no browser or device testing was done, and the tests mock window.scrollTo, ResizeObserver, the router and timers — so nothing in the evidence here demonstrates the fix on the runtime where it fails (a mid-range phone on a cache-restored feed). Could you confirm which device/preview was actually checked, and what the before/after looked like? Every previous iteration of this fix was correct in the diff and wrong in the runtime.

Blocking

• usePostModalNavigation.ts — onCloseModal restores with an unguarded window.scrollTo (inline).
• useScrollRestoration.ts — the restoration attempt is now unbounded in time (inline).

Non-blocking

• Two parallel scroll-position stores and two history-key readers (inline).
• new ResizeObserver without the repo's typeof ResizeObserver === 'undefined' guard (inline).
• Unrelated strict-typing edits inside a fix PR (inline).

Reviewed by AI.

[rebelchris]

Re-review of 163387c

Both blockers are properly resolved, and the shape is better than what I suggested:

• lib/scrollRestoration.ts is now a single height-aware restoration used by both the hook and modal close, so the close path no longer scrolls blind. Saving the feed offset under the destination entry and letting useScrollRestoration pick it up is the right seam.
• The 10s bound is back, with a popstate/native-scroll cancel and a regression test asserting nothing restores after the deadline.
• ResizeObserver is guarded with a bounded polling fallback, matching the rest of the repo.
• The unrelated typing changes are gone, and the store is capped with LRU eviction.

One new finding on the added e2e, plus the verification ask still open. No blockers from me on the hook code itself.

Reviewed by AI.

[rebelchris]

Non-blocking, but I'd drop this file: this reads scrollRestoration.ts off disk, transpiles it at runtime with the typescript compiler API and injects the output as a script tag into a synthetic <div id="feed" style="height:2000px"> fixture. Nothing else in packages/playwright works this way, and it is a pattern that will quietly rot — a future refactor that splits the module, adds an import, or changes the export shape breaks the harness rather than the feature.

More importantly, it does not test what the PR fixes. There is no feed, no post modal, no usePostModalNavigation, no back navigation — it asserts that restoreScrollPosition scrolls to 5000 once a div grows, which the Jest suite already covers with fake timers. The CPU throttling and Pixel 5 project make it look like device verification, but the page under test is a two-element fixture, so it proves nothing about the real feed on a real phone.

Suggested direction: either drop it and keep the Jest coverage, or write it against the actual app (baseURL, open a feed, open a post, close it, assert scrollY) in ./tests so it runs with the existing config.

Reviewed by AI.

[rebelchris]

Non-blocking: a second Playwright config with its own testDir: './regressions' means this suite is outside playwright.config.ts (testDir: './tests'), so neither pnpm test nor the e2e-tests workflow will ever run it — it only runs if somebody remembers pnpm test:scroll-restoration. A regression test nobody runs is not a regression test. If the suite survives the comment above, please fold it into the existing config as a project rather than adding a parallel one.

Reviewed by AI.

[rebelchris]

Nit, no action needed if intentional: on the webapp this both saves the position and starts a restoration, while the asPath change makes useScrollRestoration's effect find the same value and start a second one (the first is cancelled by cancelScrollRestoration inside). Harmless, and I assume the explicit call is here because the extension's router does not drive that effect — worth a line in the description so the redundancy is not read as a bug later.

Small related asymmetry: the key is written from ${webappUrl}posts/${postId} and read back from router.asPath. They agree today because the push sets as to exactly that string, but webappUrl concatenation has bitten this repo before; deriving both sides from one expression would be safer.

Reviewed by AI.

[rebelchris]

Correction on my part, and a red check. typecheck_strict_changed is failing on 163387c:

packages/shared/src/hooks/usePostModalNavigation.ts:97  'item.ad.data.post' is possibly 'undefined'
packages/shared/src/hooks/usePostModalNavigation.ts:198 'item' is possibly 'null'
packages/shared/src/hooks/usePostModalNavigation.ts:268 Type 'undefined' cannot be used as an index type
packages/shared/src/hooks/usePostModalNavigation.ts:296 'openedPostIndex' is possibly 'undefined'
... 13 in total

That is my fault: I asked for the strict-typing edits to be split out as unrelated scope, but typecheck:strict:changed runs strict mode over every changed file, so touching this hook at all makes them mandatory. Please put them back — the optional chaining on item.ad.data.post, the item null guard, selectedPostIndex: number | undefined, and the openedPostIndex === undefined guards in onPrevious/onNext. Withdraw my scope comment; the only part worth keeping from it is the getPost/getPostItem inconsistency, and that resolves itself once the guards are restored.

The substantive review stands: the two blockers are fixed, the e2e harness comments are still open.

Reviewed by AI.