mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-09-21 10:03:29 +00:00
xarmian
9fb6ac006b
Replaces TASK-755's bespoke listing-only scroll-restoration code
with a reusable createScrollRestoration helper built on
SvelteKit's snapshot API, applied across every workspace top-
level page (item detail, collection listing, workspace home,
activity, starred, library, conventions, playbooks list/detail,
roles).
## The bug
On any workspace page, navigating away and back left the user
near the top of the page even though they had scrolled down. The
listing's old TASK-755 workaround also didn't work in practice
once the layout's .main-content overflow-y:auto landed (it had
been targeting window.scrollY which is permanently 0).
## The fix
new `web/src/lib/scroll/restore.svelte.ts`:
createScrollRestoration({ ready, persistKey? }) returns
{ snapshot } that the page re-exports as SvelteKit's snapshot
contract.
Layered restoration strategy:
1. SvelteKit snapshot (per-history-entry sessionStorage) for
back/forward.
2. localStorage fallback for cross-tab / workspace-switcher
goto() (no popstate) restoration, re-fires per persistKey
change.
3. Per-key restoredKey one-shot so routes that reuse a
component instance across URLs get fresh restoration on
each new entry.
4. snapshotKey tracks the SvelteKit-claimed key so LS
fallback yields to a popstate snapshot.restore that beats
the effect.
5. ready() gate: caller-provided predicate must return true
before we attempt to scroll, with the contract that for
routes which reload on URL change the caller verifies
content-vs-URL identity (e.g. item.slug === itemSlug ||
issue-id === itemSlug). itemUrlId() prefers refs over
slugs so the issue-id branch is the dominant URL shape.
6. Retry loop with scrollHeight-stability gate (~250ms) and
a 2s budget. Per-frame re-scroll handles Tiptap rendering
content across many frames and async property-card fields.
7. User-input bail via wheel/touchmove/keydown listeners,
NOT a scrollY diff. The browser's default
overflow-anchor: auto adjusts scrollTop when content layout
shifts; that browser-driven change isn't user input and
mustn't trigger the bail.
8. Scroll target is .main-content (the app's actual overflow
container set by the root layout), NOT window. Window's
scrollY/scrollTo is a no-op for this app's chrome.
## Per-page integration
Each workspace page calls createScrollRestoration() with a
ready() predicate appropriate to its loading shape and a
pathname-keyed persistKey. The collection listing's persistKey
deliberately excludes ?search and showArchived (filter toggles
call goto({replaceState}) and would otherwise jump scroll
mid-interaction).
## Code path summary
- web/src/lib/scroll/restore.svelte.ts — new helper (~460 lines
with extensive design notes).
- workspace +page.svelte and 9 other route files — thin call
sites adding ~10-30 lines each.
- [collection]/+page.svelte — removes ~140 lines of TASK-755's
bespoke localStorage + double-RAF code in favor of the
helper.
Net: +637 / -149.
## Verification
- make check passes (golangci-lint, go test, npm run build).
- svelte-check 0 errors.
- Manual repro of the canonical scenario (item → wiki-link
child → back) confirmed working including the
multi-section ChildItems layout that exposed the
scroll-anchoring bail bug.
## Development trail (squashed from 11 commits)
This commit is the final state of an unusually long iteration:
Codex was consulted 5 times in a review loop and produced a
sequence of correct-but-insufficient fixes (self-cancelling
effect, lifetime-scoped guard, stale-content race, slug/ref
match, snapshot/LS race, per-key reset) all of which were
operating on the wrong measurement: window.scrollY. Once
diagnostic console.logs were added (round 9) the actual problem
fell out in two rounds — wrong scroll target, then wrong bail
signal. Lesson: when behaviour doesn't match logic, instrument
before iterating.