Merge pull request 'fix(editor): резервировать высоту документа на свопе static→live — скролл переживает своп (корень дрыга, #266)' (#308) from fix/scroll-restore-swap-height into develop
Reviewed-on: #308
This commit was merged in pull request #308.
This commit is contained in:
@@ -0,0 +1,164 @@
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
import { renderHook, act } from "@testing-library/react";
|
||||
import type { RefObject } from "react";
|
||||
import { useSwapHeightReservation } from "./use-swap-height-reservation";
|
||||
|
||||
// Controllable fake requestAnimationFrame. jsdom's rAF is timer-driven and hard
|
||||
// to step deterministically, so we install a manual queue: `tickRaf()` drains the
|
||||
// callbacks scheduled so far (a callback that reschedules enqueues a new one for
|
||||
// the NEXT tick), letting each test advance the release loop frame by frame.
|
||||
let rafQueue: Array<{ id: number; cb: FrameRequestCallback }> = [];
|
||||
let nextRafId = 1;
|
||||
let realRaf: typeof globalThis.requestAnimationFrame;
|
||||
let realCancel: typeof globalThis.cancelAnimationFrame;
|
||||
|
||||
function tickRaf(): void {
|
||||
const current = rafQueue;
|
||||
rafQueue = [];
|
||||
for (const { cb } of current) cb(0);
|
||||
}
|
||||
|
||||
// A mutable stand-in for the live-content container. The hook only reads
|
||||
// `scrollHeight`, so tests drive the release condition by mutating this.
|
||||
function makeMenuRef(): {
|
||||
ref: RefObject<HTMLElement | null>;
|
||||
setScrollHeight: (h: number) => void;
|
||||
} {
|
||||
const el = { scrollHeight: 0 };
|
||||
return {
|
||||
ref: { current: el } as unknown as RefObject<HTMLElement | null>,
|
||||
setScrollHeight: (h: number) => {
|
||||
el.scrollHeight = h;
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
const H = 1000;
|
||||
|
||||
describe("useSwapHeightReservation", () => {
|
||||
beforeEach(() => {
|
||||
rafQueue = [];
|
||||
nextRafId = 1;
|
||||
realRaf = globalThis.requestAnimationFrame;
|
||||
realCancel = globalThis.cancelAnimationFrame;
|
||||
globalThis.requestAnimationFrame = ((cb: FrameRequestCallback) => {
|
||||
const id = nextRafId++;
|
||||
rafQueue.push({ id, cb });
|
||||
return id;
|
||||
}) as typeof globalThis.requestAnimationFrame;
|
||||
globalThis.cancelAnimationFrame = ((id: number) => {
|
||||
rafQueue = rafQueue.filter((e) => e.id !== id);
|
||||
}) as typeof globalThis.cancelAnimationFrame;
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
globalThis.requestAnimationFrame = realRaf;
|
||||
globalThis.cancelAnimationFrame = realCancel;
|
||||
vi.useRealTimers();
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
// (a) reserve-on-swap: the captured height becomes `reservedHeight`, the value
|
||||
// that drives the swap wrapper's minHeight. Captured while static is still up,
|
||||
// then the swap flips showStatic; before any release frame runs the reservation
|
||||
// is held at exactly H.
|
||||
it("(a) holds the captured height as reservedHeight after the swap (drives minHeight)", () => {
|
||||
const { ref, setScrollHeight } = makeMenuRef();
|
||||
setScrollHeight(0); // live content not laid out yet -> release cannot fire.
|
||||
const { result, rerender } = renderHook(
|
||||
({ showStatic }) => useSwapHeightReservation(showStatic, ref),
|
||||
{ initialProps: { showStatic: true } },
|
||||
);
|
||||
|
||||
// Capture happens synchronously at the swap point (static still shown).
|
||||
act(() => {
|
||||
result.current.captureReservation(H);
|
||||
});
|
||||
// The swap flips to the live branch.
|
||||
rerender({ showStatic: false });
|
||||
|
||||
expect(result.current.reservedHeight).toBe(H);
|
||||
});
|
||||
|
||||
// (b) release when the live content is tall enough. Guard is `>=`: with
|
||||
// liveHeight === H the reservation releases. This FAILS if the guard direction
|
||||
// were `<` (liveHeight === H is not `< H`, so it would never release).
|
||||
it("(b) releases once live content reaches the reserved height", () => {
|
||||
const { ref, setScrollHeight } = makeMenuRef();
|
||||
setScrollHeight(0);
|
||||
const { result, rerender } = renderHook(
|
||||
({ showStatic }) => useSwapHeightReservation(showStatic, ref),
|
||||
{ initialProps: { showStatic: true } },
|
||||
);
|
||||
|
||||
act(() => {
|
||||
result.current.captureReservation(H);
|
||||
});
|
||||
rerender({ showStatic: false });
|
||||
expect(result.current.reservedHeight).toBe(H); // still reserved (short live doc)
|
||||
|
||||
// Live editor finishes laying out to the reserved height.
|
||||
setScrollHeight(H);
|
||||
act(() => {
|
||||
tickRaf();
|
||||
});
|
||||
|
||||
expect(result.current.reservedHeight).toBeNull();
|
||||
});
|
||||
|
||||
// (c) cap escape: the live content never reaches the reserved height, so the
|
||||
// height match never fires; the reservation must still release at the 4000ms
|
||||
// cap (no stuck reservation / dead space). This FAILS if there were no cap: the
|
||||
// loop would poll forever while scrollHeight stays below H.
|
||||
it("(c) releases at the 4000ms cap when live content stays too short", () => {
|
||||
// Only fake Date so `Date.now()` (the cap clock) is controllable; leave our
|
||||
// manual rAF queue in place (default fake timers would replace it).
|
||||
vi.useFakeTimers({ toFake: ["Date"] });
|
||||
vi.setSystemTime(0);
|
||||
const { ref, setScrollHeight } = makeMenuRef();
|
||||
setScrollHeight(H - 100); // always shorter than reserved -> height match never fires.
|
||||
const { result, rerender } = renderHook(
|
||||
({ showStatic }) => useSwapHeightReservation(showStatic, ref),
|
||||
{ initialProps: { showStatic: true } },
|
||||
);
|
||||
|
||||
act(() => {
|
||||
result.current.captureReservation(H);
|
||||
});
|
||||
rerender({ showStatic: false });
|
||||
|
||||
// A few frames pass but time has not reached the cap: still reserved.
|
||||
act(() => {
|
||||
tickRaf();
|
||||
});
|
||||
act(() => {
|
||||
tickRaf();
|
||||
});
|
||||
expect(result.current.reservedHeight).toBe(H);
|
||||
|
||||
// Advance past the cap; the next frame releases even though the live content
|
||||
// is still shorter than the reservation.
|
||||
vi.setSystemTime(4001);
|
||||
act(() => {
|
||||
tickRaf();
|
||||
});
|
||||
|
||||
expect(result.current.reservedHeight).toBeNull();
|
||||
});
|
||||
|
||||
// (d) non-swap: without a capture (and while static is shown) there is no
|
||||
// reservation and the release loop never arms, so no rAF is scheduled.
|
||||
it("(d) reserves nothing and arms no loop when the swap never happens", () => {
|
||||
const { ref } = makeMenuRef();
|
||||
const { result } = renderHook(() =>
|
||||
useSwapHeightReservation(true, ref),
|
||||
);
|
||||
|
||||
expect(result.current.reservedHeight).toBeNull();
|
||||
expect(rafQueue.length).toBe(0); // release loop never armed
|
||||
act(() => {
|
||||
tickRaf();
|
||||
});
|
||||
expect(result.current.reservedHeight).toBeNull();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,79 @@
|
||||
import { RefObject, useCallback, useEffect, useState } from "react";
|
||||
|
||||
// Last-resort release deadline. The primary release is the live-content height
|
||||
// match below; this cap only exists so a slow/short live doc can never pin the
|
||||
// reservation forever. It is generous (well past when the live content normally
|
||||
// reaches the reserved height — it renders the SAME content as the static copy)
|
||||
// so a slow load doesn't release mid-render and reintroduce the collapse.
|
||||
const RELEASE_CAP_MS = 4000;
|
||||
|
||||
/**
|
||||
* Reserves the document height across the static -> live editor swap.
|
||||
*
|
||||
* The live editor lays out its content over a few frames, so replacing the
|
||||
* (full-height) static copy with it momentarily shrinks the document; the
|
||||
* browser then clamps window scroll to the top, which yanked the reader off
|
||||
* their restored reading position (and threw their scroll to 0 if they were
|
||||
* scrolling at that moment). Pinning a min-height on the swap wrapper keeps the
|
||||
* document tall through the swap so the scroll position simply survives (#266).
|
||||
* `reservedHeight === null` means no reservation is active.
|
||||
*
|
||||
* The capture is intentionally a CALLBACK the page editor invokes, NOT something
|
||||
* this hook derives by watching `showStatic`. The height MUST be read
|
||||
* synchronously while the static content is still mounted (full natural height),
|
||||
* right before the flip to the live branch. By the time any post-transition
|
||||
* effect here could run, `showStatic` is already false and the wrapper shows the
|
||||
* live/collapsed content, so `offsetHeight` would be wrong. So page-editor calls
|
||||
* `captureReservation(wrapper.offsetHeight)` inside its collab-sync effect,
|
||||
* before `setShowStatic(false)`, preserving that exact timing.
|
||||
*
|
||||
* @param showStatic whether the static (cached) content is still shown.
|
||||
* @param menuContainerRef the live-branch content container. It is a descendant
|
||||
* of the swap wrapper inside the live branch, so its `scrollHeight` is the live
|
||||
* content height (not inflated by the ancestor min-height reservation).
|
||||
*/
|
||||
export function useSwapHeightReservation(
|
||||
showStatic: boolean,
|
||||
menuContainerRef: RefObject<HTMLElement | null>,
|
||||
): {
|
||||
reservedHeight: number | null;
|
||||
captureReservation: (height: number | null) => void;
|
||||
} {
|
||||
const [reservedHeight, setReservedHeight] = useState<number | null>(null);
|
||||
|
||||
// Capture the current (static, full-height) content height BEFORE the swap so
|
||||
// the wrapper can reserve it while the live editor lays out — otherwise the
|
||||
// transient shrink clamps window scroll to the top. The caller reads
|
||||
// `offsetHeight` synchronously at the swap point and hands it here.
|
||||
const captureReservation = useCallback(
|
||||
(height: number | null) => setReservedHeight(height),
|
||||
[],
|
||||
);
|
||||
|
||||
// Release the reserved height once the live editor's content has laid out to
|
||||
// at least the reserved height (so removing the reservation cannot collapse
|
||||
// the document). The primary release is that height match; the cap is only a
|
||||
// last-resort so we never pin forever. A shorter-than-reserved live doc (rare:
|
||||
// stale/longer cache) releases at the cap, leaving only harmless bottom dead
|
||||
// space until then.
|
||||
useEffect(() => {
|
||||
if (showStatic || reservedHeight == null) return;
|
||||
let raf = 0;
|
||||
const startedAt = Date.now();
|
||||
const check = () => {
|
||||
const liveHeight = menuContainerRef.current?.scrollHeight ?? 0;
|
||||
if (
|
||||
liveHeight >= reservedHeight ||
|
||||
Date.now() - startedAt > RELEASE_CAP_MS
|
||||
) {
|
||||
setReservedHeight(null);
|
||||
return;
|
||||
}
|
||||
raf = requestAnimationFrame(check);
|
||||
};
|
||||
raf = requestAnimationFrame(check);
|
||||
return () => cancelAnimationFrame(raf);
|
||||
}, [showStatic, reservedHeight, menuContainerRef]);
|
||||
|
||||
return { reservedHeight, captureReservation };
|
||||
}
|
||||
@@ -79,6 +79,7 @@ import { jwtDecode } from "jwt-decode";
|
||||
import { searchSpotlight } from "@/features/search/constants.ts";
|
||||
import { useEditorScroll } from "./hooks/use-editor-scroll";
|
||||
import { useScrollRestoreOnSwap } from "./hooks/use-scroll-position";
|
||||
import { useSwapHeightReservation } from "./hooks/use-swap-height-reservation";
|
||||
import { EditorLinkMenu } from "@/features/editor/components/link/link-menu";
|
||||
import ColumnsMenu from "@/features/editor/components/columns/columns-menu.tsx";
|
||||
import { TransclusionLookupProvider } from "@/features/editor/components/transclusion/transclusion-lookup-context";
|
||||
@@ -449,6 +450,22 @@ export default function PageEditor({
|
||||
const hasConnectedOnceRef = useRef(false);
|
||||
const [showStatic, setShowStatic] = useState(true);
|
||||
|
||||
// Reserved height held across the static -> live editor swap. The live editor
|
||||
// lays out its content over a few frames, so replacing the (full-height) static
|
||||
// copy with it momentarily shrinks the document; the browser then clamps window
|
||||
// scroll to the top, which yanked the reader off their restored reading position
|
||||
// (and threw their scroll to 0 if they were scrolling at that moment). Pinning a
|
||||
// min-height on the swap wrapper keeps the document tall through the swap so the
|
||||
// scroll position simply survives. `null` = no reservation active.
|
||||
const swapWrapperRef = useRef<HTMLDivElement | null>(null);
|
||||
// Reserve/release wiring lives in the hook so its capture trigger and release
|
||||
// guard/cap are directly unit-testable. Capture stays synchronous at the swap
|
||||
// point (see the collab-sync effect below); the hook only owns the release.
|
||||
const { reservedHeight, captureReservation } = useSwapHeightReservation(
|
||||
showStatic,
|
||||
menuContainerRef,
|
||||
);
|
||||
|
||||
useEffect(() => {
|
||||
const timeout = setTimeout(() => {
|
||||
if (yjsConnectionStatus === WebSocketStatus.Connecting || !isSynced) {
|
||||
@@ -477,6 +494,10 @@ export default function PageEditor({
|
||||
isCollabSynced(yjsConnectionStatus, isSynced)
|
||||
) {
|
||||
hasConnectedOnceRef.current = true;
|
||||
// Capture the current (static, full-height) content height BEFORE the swap
|
||||
// so the wrapper can reserve it while the live editor lays out — otherwise
|
||||
// the transient shrink clamps window scroll to the top.
|
||||
captureReservation(swapWrapperRef.current?.offsetHeight ?? null);
|
||||
setShowStatic(false);
|
||||
}
|
||||
}, [yjsConnectionStatus, isSynced]);
|
||||
@@ -490,6 +511,12 @@ export default function PageEditor({
|
||||
<TransclusionLookupProvider>
|
||||
<PageEmbedLookupProvider>
|
||||
<PageEmbedAncestryProvider hostPageId={pageId}>
|
||||
<div
|
||||
ref={swapWrapperRef}
|
||||
style={
|
||||
reservedHeight != null ? { minHeight: reservedHeight } : undefined
|
||||
}
|
||||
>
|
||||
{showStatic ? (
|
||||
<div style={{ position: "relative" }}>
|
||||
{/* Surface the pre-sync read-only window so edits typed before the
|
||||
@@ -577,6 +604,7 @@ export default function PageEditor({
|
||||
></div>
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
</PageEmbedAncestryProvider>
|
||||
</PageEmbedLookupProvider>
|
||||
</TransclusionLookupProvider>
|
||||
|
||||
Reference in New Issue
Block a user