diff --git a/CHANGELOG.md b/CHANGELOG.md index a5c600d87..7327174c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## 26.10.2 + +### Fixed + +- Picking a section lower down in the host editor, like Warpgate, stuttered on the way and sometimes stopped before it got there + ## 26.10.1 ### Added diff --git a/src/ui/manage/EditorPane.tsx b/src/ui/manage/EditorPane.tsx index 8b1acd6c2..ecdeb427c 100644 --- a/src/ui/manage/EditorPane.tsx +++ b/src/ui/manage/EditorPane.tsx @@ -119,17 +119,45 @@ export function useScrollSpy( let lastHeight = area.scrollHeight; area.scrollTo?.({ top: lastTop, behavior: "smooth" }); // Sections above can still be mounting, which moves the target or - // cuts the scroll short. Follow it until the layout settles. + // cuts the scroll short. Follow it until the scroll lands and the + // layout stops changing, unless the user scrolls first. const started = Date.now(); + let changedAt = started; + let userScrolled = false; + const onUser = () => { + userScrolled = true; + }; + area.addEventListener("wheel", onUser, { passive: true }); + area.addEventListener("touchstart", onUser, { passive: true }); + const done = () => { + area.removeEventListener("wheel", onUser); + area.removeEventListener("touchstart", onUser); + }; const follow = () => { - if (!el.isConnected || Date.now() - started > 1500) return; + const now = Date.now(); + if (userScrolled || !el.isConnected || now - started > 5000) { + return done(); + } const top = target(); + // Keep the spy quiet for the whole trip, not just the first scroll. + lockUntil.current = now + 600; if (Math.abs(top - lastTop) > 4 || area.scrollHeight !== lastHeight) { lastTop = top; lastHeight = area.scrollHeight; - lockUntil.current = Date.now() + 600; + changedAt = now; area.scrollTo?.({ top, behavior: "smooth" }); } + const reachable = Math.min( + lastTop, + area.scrollHeight - area.clientHeight, + ); + if ( + Math.abs(area.scrollTop - reachable) <= 2 && + now - changedAt > 400 + ) { + setActive(id); + return done(); + } requestAnimationFrame(follow); }; requestAnimationFrame(follow); @@ -137,6 +165,24 @@ export function useScrollSpy( }; } +/** Resolves once the element's height holds still for a few frames. */ +function untilLayoutSettles(el: HTMLElement | null, max = 800): Promise { + return new Promise((resolve) => { + if (!el || typeof requestAnimationFrame === "undefined") return resolve(); + const started = Date.now(); + let last = -1; + let still = 0; + const check = () => { + const height = el.scrollHeight; + still = height === last ? still + 1 : 0; + last = height; + if (still >= 3 || Date.now() - started > max) return resolve(); + requestAnimationFrame(check); + }; + requestAnimationFrame(check); + }); +} + /** Wraps a section so the nav can find it. */ export function EditorSection({ id, @@ -253,7 +299,15 @@ export function EditorPane({ const ids = visibleBands.flatMap((band) => band.items.map((item) => item.id)); const { active, scrollTo } = useScrollSpy(scrollRef, ids); const showNav = bands.flatMap((band) => band.items).length > 1; - if (revealRef) revealRef.current = scrollTo; + // A jump mounts every lazy section first and scrolls once the form stops + // growing. Mounting them on the way down made the scroll stall and restart. + const [mountAll, setMountAll] = useState(false); + const jump = (id: string) => { + if (mountAll) return scrollTo(id); + setMountAll(true); + void untilLayoutSettles(scrollRef.current).then(() => scrollTo(id)); + }; + if (revealRef) revealRef.current = jump; return (
@@ -272,7 +326,7 @@ export function EditorPane({ key={item.id} type="button" aria-current={on ? "true" : undefined} - onClick={() => scrollTo(item.id)} + onClick={() => jump(item.id)} className={cn( "flex w-full items-center gap-2 border-l-2 py-1.5 pl-2 pr-2 text-left transition-colors focus-visible:relative focus-visible:z-10 focus-visible:ring-1 focus-visible:ring-ring", on @@ -320,7 +374,7 @@ export function EditorPane({ band.items.map((item) => ({ id: item.id, label: item.label })), )} activeTab={active} - onTabChange={scrollTo} + onTabChange={jump} />
)} @@ -336,7 +390,7 @@ export function EditorPane({ className={`mx-auto flex max-w-3xl flex-col ${PANEL.gap} pb-6`} > {banner} - + {children} diff --git a/src/ui/tests/manage/EditorPane.test.tsx b/src/ui/tests/manage/EditorPane.test.tsx index caab9826e..fb6f2efd8 100644 --- a/src/ui/tests/manage/EditorPane.test.tsx +++ b/src/ui/tests/manage/EditorPane.test.tsx @@ -92,6 +92,25 @@ describe("EditorPane search", () => { }); }); +describe("EditorPane jump", () => { + it("mounts lazy sections before scrolling to one", async () => { + // Never intersecting, so lazy sections only mount when asked to. + vi.stubGlobal( + "IntersectionObserver", + class { + observe() {} + disconnect() {} + }, + ); + renderEditor(); + expect(screen.queryByText("Font size")).toBeNull(); + // The side nav, not the narrow-screen tab strip. + fireEvent.click(screen.getAllByRole("button", { name: "Terminal" })[0]); + await waitFor(() => expect(screen.getByText("Font size")).toBeTruthy()); + vi.unstubAllGlobals(); + }); +}); + describe("useScrollSpy scrollTo", () => { it("follows a section that moves while the form is still mounting", async () => { const container = document.createElement("div"); @@ -116,4 +135,65 @@ describe("useScrollSpy scrollTo", () => { await waitFor(() => expect(calls).toContain(892)); container.remove(); }); + + it("keeps following past 1.5s while lazy sections still mount", async () => { + vi.useFakeTimers({ toFake: ["Date", "requestAnimationFrame"] }); + const container = document.createElement("div"); + const section = document.createElement("div"); + section.dataset.section = "warpgate"; + container.appendChild(section); + document.body.appendChild(container); + let sectionTop = 300; + section.getBoundingClientRect = () => ({ top: sectionTop }) as DOMRect; + container.getBoundingClientRect = () => ({ top: 0 }) as DOMRect; + // Still scrolling toward the target, so the follow stays on. + Object.defineProperty(container, "scrollHeight", { value: 5000 }); + const calls: number[] = []; + container.scrollTo = ((options: ScrollToOptions) => { + calls.push(options.top ?? 0); + }) as typeof container.scrollTo; + + const ref = { current: container }; + const { result } = renderHook(() => + useScrollSpy(ref, ["general", "warpgate"]), + ); + act(() => result.current.scrollTo("warpgate")); + act(() => vi.advanceTimersByTime(2000)); + sectionTop = 1400; + act(() => vi.advanceTimersByTime(100)); + expect(calls).toContain(1392); + + vi.useRealTimers(); + container.remove(); + }); + + it("stops following once the user scrolls", () => { + vi.useFakeTimers({ toFake: ["Date", "requestAnimationFrame"] }); + const container = document.createElement("div"); + const section = document.createElement("div"); + section.dataset.section = "warpgate"; + container.appendChild(section); + document.body.appendChild(container); + let sectionTop = 300; + section.getBoundingClientRect = () => ({ top: sectionTop }) as DOMRect; + container.getBoundingClientRect = () => ({ top: 0 }) as DOMRect; + Object.defineProperty(container, "scrollHeight", { value: 5000 }); + const calls: number[] = []; + container.scrollTo = ((options: ScrollToOptions) => { + calls.push(options.top ?? 0); + }) as typeof container.scrollTo; + + const ref = { current: container }; + const { result } = renderHook(() => + useScrollSpy(ref, ["general", "warpgate"]), + ); + act(() => result.current.scrollTo("warpgate")); + fireEvent.wheel(container); + sectionTop = 900; + act(() => vi.advanceTimersByTime(200)); + expect(calls).toEqual([292]); + + vi.useRealTimers(); + container.remove(); + }); });