fix(editor): восстанавливать позицию чтения только после стабилизации вёрстки (остаточный дрыг при reload, #266) #306

Closed
vvzvlad wants to merge 1 commits from feat/scroll-restore-stable-wait into develop
Owner

Follow-up to the merged title-autofocus fix (#301). Убирает остаточный «дрыг» восстановления позиции при перезагрузке. Причина и решение подтверждены эмпирически через Chrome DevTools на живом инстансе.

Проблема

После фикса заголовка (#301) дёрганье уменьшилось, но осталось. Замер таймлайна на живой странице (сохранено 12000):

Время Что scrollY высота
0.9с restore #1 scrollTo(12000) 0 → едет 27864 (не финал)
scroll-anchoring тащит 20896 32005
2.6с restore #2 scrollTo(12000) едет ещё не финал
осел 18006 32185 (финал)

Механизм restore проверял только «хватает ли высоты, чтобы доскроллить до цели» (maxScroll >= targetY), но не «устоялась ли вёрстка». Документ рендерится прогрессивно (высота 17729→32185, со схлопыванием на свопе), поэтому restore стрелял дважды и рано, scroll-anchoring тащил позицию, второй restore дёргал обратно → дрыг + промах ~6000px.

Решение

restoreScrollPosition теперь опрашивает высоту документа и восстанавливает один раз, когда высота стабильна HEIGHT_STABLE_MS (400мс) И цель достижима; MAX_RESTORE_WAIT_MS (5с) — единственный fallback с клампом. Убран общий бюджет restoreStartRef; идемпотентность — через guard pollTimerRef !== null (работающий опрос подавляет второй триггер). Два триггера (useScrollRestoreOnSwap: ранний on-mount для offline-пути + пост-своп) сохранены.

Shadow-симуляция нового автомата на живой странице (наблюдение без скролла) подтвердила: fires один раз, точно в цель (12000), вместо двух-с-дрейфом.

Компромисс

На прогрессивно-рендерящихся страницах позиция появляется, когда вёрстка устоится (~0.5–2с) — одним плавным переходом, вместо раннего-но-дёрганого и часто неточного restore. Мгновенно-и-точно на догружающемся контенте невозможно без резервирования размеров/anchor-based restore (отдельный follow-up).

Файлы

  • hooks/use-scroll-position.ts — restore ждёт стабильной высоты.
  • hooks/use-scroll-position.test.ts — тесты под новый тайминг; (d3) «ждёт, пока высота перестанет меняться»; (k) переписан, чтобы пиннить guard идемпотентности (проверено мутацией).
  • page-editor.test.tsx — тесты двух-триггерной обвязки под новый тайминг.

Тесты

npx vitest run (обе сюиты) → 20 passed; полный клиент-сьют → 825 passed + 1 expected-fail. tsc чист. (k) мутационно-проверен (снятие guard'а → тест краснеет).

Ревью

Делегированный code-review — APPROVE WITH SUGGESTIONS (продакшн-код без блокеров); суджесты (устаревший (k) + комментарии) устранены.

🤖 Generated with Claude Code

Follow-up to the merged title-autofocus fix (#301). Убирает **остаточный** «дрыг» восстановления позиции при перезагрузке. Причина и решение подтверждены эмпирически через Chrome DevTools на живом инстансе. ## Проблема После фикса заголовка (#301) дёрганье уменьшилось, но осталось. Замер таймлайна на живой странице (сохранено `12000`): | Время | Что | scrollY | высота | |------|-----|--------|------| | 0.9с | restore #1 `scrollTo(12000)` | 0 → едет | **27864** (не финал) | | — | scroll-anchoring тащит | **20896** | 32005 | | 2.6с | restore #2 `scrollTo(12000)` | едет | ещё не финал | | — | осел | **18006** | 32185 (финал) | Механизм restore проверял только «хватает ли высоты, чтобы доскроллить до цели» (`maxScroll >= targetY`), но **не** «устоялась ли вёрстка». Документ рендерится прогрессивно (высота 17729→32185, со схлопыванием на свопе), поэтому restore стрелял **дважды и рано**, scroll-anchoring тащил позицию, второй restore дёргал обратно → дрыг + промах ~6000px. ## Решение `restoreScrollPosition` теперь опрашивает высоту документа и восстанавливает **один раз**, когда высота **стабильна** `HEIGHT_STABLE_MS` (400мс) И цель достижима; `MAX_RESTORE_WAIT_MS` (5с) — единственный fallback с клампом. Убран общий бюджет `restoreStartRef`; идемпотентность — через guard `pollTimerRef !== null` (работающий опрос подавляет второй триггер). Два триггера (`useScrollRestoreOnSwap`: ранний on-mount для offline-пути + пост-своп) сохранены. **Shadow-симуляция нового автомата на живой странице** (наблюдение без скролла) подтвердила: fires **один раз**, точно в цель (12000), вместо двух-с-дрейфом. ## Компромисс На прогрессивно-рендерящихся страницах позиция появляется, когда вёрстка устоится (~0.5–2с) — одним плавным переходом, вместо раннего-но-дёрганого и часто неточного restore. Мгновенно-и-точно на догружающемся контенте невозможно без резервирования размеров/anchor-based restore (отдельный follow-up). ## Файлы - `hooks/use-scroll-position.ts` — restore ждёт стабильной высоты. - `hooks/use-scroll-position.test.ts` — тесты под новый тайминг; `(d3)` «ждёт, пока высота перестанет меняться»; `(k)` переписан, чтобы пиннить guard идемпотентности (проверено мутацией). - `page-editor.test.tsx` — тесты двух-триггерной обвязки под новый тайминг. ## Тесты `npx vitest run` (обе сюиты) → **20 passed**; полный клиент-сьют → **825 passed + 1 expected-fail**. `tsc` чист. `(k)` мутационно-проверен (снятие guard'а → тест краснеет). ## Ревью Делегированный code-review — **APPROVE WITH SUGGESTIONS** (продакшн-код без блокеров); суджесты (устаревший `(k)` + комментарии) устранены. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
vvzvlad added the review/needs label 2026-07-03 16:57:17 +03:00
vvzvlad added 1 commit 2026-07-03 16:57:18 +03:00
Follow-up to the merged title-autofocus fix (#301). Confirmed via Chrome DevTools
on the live app: a residual reload jitter remained — the document renders
progressively (measured height 17729 -> 32185, collapsing mid-swap), and the
restore fired TOO EARLY (twice, at partial heights) because it only checked
"is the target reachable", not "has the layout settled". While the doc grew,
scroll-anchoring dragged the position and the second restore yanked it back
(the jitter), landing ~6000px off.

- restoreScrollPosition now polls the document height and restores ONCE the
  height has been stable for HEIGHT_STABLE_MS (400ms) AND the target is
  reachable; the MAX_RESTORE_WAIT_MS (5s) timeout is the only fallback that
  clamps. Removed the restoreStartRef shared budget; idempotency is now the
  `pollTimerRef !== null` guard (a running wait suppresses a second trigger).
- The two-trigger wiring (early on-mount for the offline path + post-swap) is
  unchanged; both call the now-settle-waiting, idempotent restore.
- A shadow simulation on the live page confirmed the new logic fires once,
  accurately (vs the old two-fire-plus-drift).
- Tests updated for the stable-height timing; (k) rewritten to pin the
  idempotency guard (mutation-verified); (d3) added for "waits until height
  stops changing".

Tradeoff: on progressively-rendering pages the position now appears once the
layout settles (~0.5-2s) in one smooth move, instead of an early-but-jittery,
often-inaccurate restore.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Author
Owner

Заменён на #308: там stable-wait лечил симптом с задержкой, а новый PR бьёт в корень (резерв высоты на свопе, без задержки) и валидирован на живой странице. Закрываю.

Заменён на #308: там stable-wait лечил симптом с задержкой, а новый PR бьёт в корень (резерв высоты на свопе, без задержки) и валидирован на живой странице. Закрываю.
vvzvlad closed this pull request 2026-07-03 17:31:37 +03:00
Collaborator

Ревью — #306 (editor: восстановление scroll-позиции после стабилизации вёрстки, #266), round 1, head 882a6bb0, base develop

Scope: реальная дельта PR — ТОЛЬКО коммит 882a6bb0 поверх смердженного #304 (родитель 88d96c41); 3 файла (use-scroll-position.ts +91, .test +112, page-editor.test +74). Предки (#304/#301) не ревьюились — уже одобрены.

Вердикт: PASS — фикс тайминга корректен и хорошо покрыт, объективка зелёная. Ничего для кодера. (NB: живой «дрыг» в браузере я не проверял — нет браузера; оценка по логике + тестам. Эмпирическую первопричину PR описал через DevTools-таймлайн — по коду фикс её адресует.)

Полный веер (stability, regressions, test-coverage, coherence, conventions+simplification) — все LGTM. Объективка запущена мной (детач 882a6bb0): client tsc --noEmit0 ошибок; vitest use-scroll-position + page-editor2 files, 20 tests passed.

Подтверждено по коду + прогоны

  • Суть фикса верна. Раньше restore стрелял на первом же тике maxScroll>=targetY — mid-render, пока высота ещё росла/схлопывалась на static→live свопе, и scroll-anchoring утаскивал offset (двойной ранний restore = остаточный дрыг). Стало: гейт (settled && reachable) || timedOut, где settled = now-stableSince >= HEIGHT_STABLE_MS(400) и stableSince сбрасывается на ЛЮБОЕ изменение высоты (if (height!==lastHeight){lastHeight=height; stableSince=now}). Стреляет ОДИН раз, после того как вёрстка устоялась. Первопричина адресована.
  • Termination/bounded. Если высота никогда не стабилизируется — timedOut = now-start >= MAX_RESTORE_WAIT_MS(5000) клампит к furthest reachable (top=min(target,max(maxScroll,0))). Poll всегда либо перепланирует один таймер, либо завершается. Не заклинивает.
  • Конкурентность. Ре-триггер во время running poll теперь NO-OP (if (pollTimerRef.current!==null) return, вместо cancel+restart). Одновременно бежит ровно один poll; in-flight poll читает scrollHeight свежим каждый tick → сам подхватывает live-layout, так что подавление второго триггера ничего не теряет. restoreStartRef (шаренный бюджет) удалён чисто, start локальный per-run — корректно, раз concurrent-poll невозможен.
  • Пути целы. Offline (live-editor не пришёл → early-триггер восстанавливает на static-layout, post-swap дремлет под !showStatic && editor); swap (early-poll подхватывает live-layout или post-swap стартует свежий poll); redundancy-guard abs(scrollY-top)>1 делает пере-ассерт no-op когда уже на месте. Пред-инварианты целы: #hash-guard wins, targetY<=0 skip, storage-throw safe, оба useLayoutEffect с прежними деп-ами. userInteracted-bail останавливает poll и блокирует последующие. Cleanup на unmount отменяет pollTimerRef (SPA-навигация mid-poll не стрельнёт по новой странице).
  • Тесты не-вакуозны. (d3) высота меняется каждый tick → не устаивается (против старого «стрельни на reachable» падает синхронно), затем steady 400мс+ → стреляет к фикс-target; (k) переписан на «ре-триггер во время wait не стартует второй poll» (ровно 1 scroll; мутант без гварда → 2); guard-тест (early settles once + && editor гейт); end-to-end «ждёт settle». Риск-путь timeout-clamp ПОКРЫТ нетронутым (d2) (target 5000, высота const 1000 → только timedOut-ветка → top:200). Замена старого (k) на новый — намеренная (шаренный бюджет удалён как механизм).
  • Conventions/simpl: HEIGHT_STABLE_MS в стиле файла, tryRestore→tick уместно, dead-code/vestigial-комментов нет, доки обновлены под settle-механику.
## Ревью — #306 (editor: восстановление scroll-позиции после стабилизации вёрстки, #266), round 1, head `882a6bb0`, base develop Scope: реальная дельта PR — ТОЛЬКО коммит `882a6bb0` поверх смердженного #304 (родитель `88d96c41`); 3 файла (use-scroll-position.ts +91, .test +112, page-editor.test +74). Предки (#304/#301) не ревьюились — уже одобрены. **Вердикт: PASS** — фикс тайминга корректен и хорошо покрыт, объективка зелёная. Ничего для кодера. (NB: живой «дрыг» в браузере я не проверял — нет браузера; оценка по логике + тестам. Эмпирическую первопричину PR описал через DevTools-таймлайн — по коду фикс её адресует.) Полный веер (stability, regressions, test-coverage, coherence, conventions+simplification) — все LGTM. **Объективка запущена мной** (детач `882a6bb0`): client `tsc --noEmit` → **0 ошибок**; `vitest use-scroll-position + page-editor` → **2 files, 20 tests passed**. ### Подтверждено по коду + прогоны - **Суть фикса верна.** Раньше restore стрелял на первом же тике `maxScroll>=targetY` — mid-render, пока высота ещё росла/схлопывалась на static→live свопе, и scroll-anchoring утаскивал offset (двойной ранний restore = остаточный дрыг). Стало: гейт `(settled && reachable) || timedOut`, где `settled = now-stableSince >= HEIGHT_STABLE_MS(400)` и `stableSince` сбрасывается на ЛЮБОЕ изменение высоты (`if (height!==lastHeight){lastHeight=height; stableSince=now}`). Стреляет ОДИН раз, после того как вёрстка устоялась. Первопричина адресована. - **Termination/bounded.** Если высота никогда не стабилизируется — `timedOut = now-start >= MAX_RESTORE_WAIT_MS(5000)` клампит к furthest reachable (`top=min(target,max(maxScroll,0))`). Poll всегда либо перепланирует один таймер, либо завершается. Не заклинивает. - **Конкурентность.** Ре-триггер во время running poll теперь NO-OP (`if (pollTimerRef.current!==null) return`, вместо cancel+restart). Одновременно бежит ровно один poll; in-flight poll читает `scrollHeight` свежим каждый tick → сам подхватывает live-layout, так что подавление второго триггера ничего не теряет. `restoreStartRef` (шаренный бюджет) удалён чисто, `start` локальный per-run — корректно, раз concurrent-poll невозможен. - **Пути целы.** Offline (live-editor не пришёл → early-триггер восстанавливает на static-layout, post-swap дремлет под `!showStatic && editor`); swap (early-poll подхватывает live-layout или post-swap стартует свежий poll); redundancy-guard `abs(scrollY-top)>1` делает пере-ассерт no-op когда уже на месте. Пред-инварианты целы: #hash-guard wins, `targetY<=0` skip, storage-throw safe, оба useLayoutEffect с прежними деп-ами. userInteracted-bail останавливает poll и блокирует последующие. Cleanup на unmount отменяет pollTimerRef (SPA-навигация mid-poll не стрельнёт по новой странице). - **Тесты не-вакуозны.** (d3) высота меняется каждый tick → не устаивается (против старого «стрельни на reachable» падает синхронно), затем steady 400мс+ → стреляет к фикс-target; (k) переписан на «ре-триггер во время wait не стартует второй poll» (ровно 1 scroll; мутант без гварда → 2); guard-тест (early settles once + `&& editor` гейт); end-to-end «ждёт settle». Риск-путь timeout-clamp ПОКРЫТ нетронутым (d2) (target 5000, высота const 1000 → только timedOut-ветка → `top:200`). Замена старого (k) на новый — намеренная (шаренный бюджет удалён как механизм). - Conventions/simpl: `HEIGHT_STABLE_MS` в стиле файла, `tryRestore→tick` уместно, dead-code/vestigial-комментов нет, доки обновлены под settle-механику. <!-- state:review reviewed_head=882a6bb03253... round=1 verdict=pass -->
agent_reviewer added review/approved and removed review/needs labels 2026-07-03 17:31:57 +03:00

Pull request closed

Sign in to join this conversation.