From 40eaf94e44b749bcbdaa9c3d08ea336fe8aeee20 Mon Sep 17 00:00:00 2001 From: callumalpass Date: Mon, 19 Oct 2026 00:31:27 +1100 Subject: [PATCH] fix: keep outline heading paging stable during editing --- .github/workflows/ci.yml | 4 ++ apps/writer/package.json | 2 + apps/writer/scripts/browser-editing-test.mjs | 66 ++++++++++++++++++++ apps/writer/src/ui/OutlinePanel.test.tsx | 66 ++++++++++++++++++++ apps/writer/src/ui/OutlinePanel.tsx | 6 +- docs/development.md | 11 ++++ 6 files changed, 154 insertions(+), 1 deletion(-) create mode 100644 apps/writer/scripts/browser-editing-test.mjs create mode 100644 apps/writer/src/ui/OutlinePanel.test.tsx diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c89a7332..b82622db 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -40,6 +40,10 @@ jobs: sudo apt-get install --yes --no-install-recommends pandoc unzip - name: Install Chromium run: pnpm --filter @mdbase-writer/app exec playwright install --with-deps chromium + - name: Verify production editing with a real Preview canvas + run: | + pnpm --filter @mdbase-writer/app build:editing-test + pnpm --filter @mdbase-writer/app test:browser:editing - name: Run baseline and reliability suites against the demo collection env: BASE: http://127.0.0.1:5320/ diff --git a/apps/writer/package.json b/apps/writer/package.json index 2d58a870..2fbf02f0 100644 --- a/apps/writer/package.json +++ b/apps/writer/package.json @@ -8,6 +8,7 @@ "build": "pnpm manifest && pnpm manifest:validate && vite build", "build:lab": "VITE_MDBASE_ENV=lab pnpm build", "build:feedback-test": "VITE_WRITER_DEMO=1 VITE_MDBASE_ENV=lab VITE_MDBASE_FEEDBACK_URL=http://127.0.0.1:8892/v1/feedback pnpm build", + "build:editing-test": "VITE_WRITER_DEMO=1 pnpm build", "dev": "pnpm manifest && vite --host 127.0.0.1", "manifest": "node scripts/write-manifest.mjs", "manifest:validate": "mdbase-connect-dev validate-manifest public/.well-known/mdbase-app.json", @@ -16,6 +17,7 @@ "test:browser": "node scripts/browser-test.mjs && node scripts/browser-reliability-test.mjs", "test:feedback": "node scripts/feedback-browser-test.mjs", "test:browser:reliability": "node scripts/browser-reliability-test.mjs", + "test:browser:editing": "node scripts/browser-editing-test.mjs", "typecheck": "pnpm manifest && tsc -p tsconfig.json", "deploy:lab": "MDBASE_ENV=lab node scripts/deploy-pages.mjs", "deploy:staging": "MDBASE_ENV=staging node scripts/deploy-pages.mjs", diff --git a/apps/writer/scripts/browser-editing-test.mjs b/apps/writer/scripts/browser-editing-test.mjs new file mode 100644 index 00000000..30fdadd6 --- /dev/null +++ b/apps/writer/scripts/browser-editing-test.mjs @@ -0,0 +1,66 @@ +// Real production React + Preview canvas regression; no account or service. +// Build first: pnpm build:editing-test, then node scripts/browser-editing-test.mjs. +import assert from "node:assert/strict"; +import http from "node:http"; +import { readFile } from "node:fs/promises"; +import { dirname, extname, join, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; +import { chromium } from "playwright"; + +const app = resolve(dirname(fileURLToPath(import.meta.url)), ".."); +const root = join(app, "dist"); +const compiler = join(app, "node_modules/@myriaddreamin/typst-ts-web-compiler"); +const { version } = JSON.parse(await readFile(join(compiler, "package.json"), "utf8")); +const mime = { ".html": "text/html", ".js": "text/javascript", ".wasm": "application/wasm", ".css": "text/css", ".json": "application/json", ".svg": "image/svg+xml", ".otf": "font/otf", ".ttf": "font/ttf", ".woff2": "font/woff2" }; +// Pages serves the oversized compiler from R2. This isolated test serves the +// exact installed compiler instead, without credentials or external requests. +const server = http.createServer(async (request, response) => { + try { + const requested = decodeURIComponent(new URL(request.url, "http://localhost").pathname); + const file = requested === `/wasm/typst_ts_web_compiler-${version}.wasm` + ? join(compiler, "pkg/typst_ts_web_compiler_bg.wasm") + : join(root, requested === "/" ? "index.html" : requested); + if (!file.startsWith(`${root}/`) && file !== join(compiler, "pkg/typst_ts_web_compiler_bg.wasm")) throw new Error("Invalid test asset path"); + const bytes = await readFile(file); + response.writeHead(200, { "Content-Type": mime[extname(file)] ?? "application/octet-stream" }); + response.end(bytes); + } catch { response.writeHead(404); response.end("Not found"); } +}); +await new Promise(resolve => server.listen(0, "127.0.0.1", resolve)); +let browser; +try { + const base = `http://127.0.0.1:${server.address().port}`; + browser = await chromium.launch({ executablePath: process.env.CHROME }); + const context = await browser.newContext({ viewport: { width: 1500, height: 950 }, serviceWorkers: "block" }); + await context.route("**/*", route => route.request().url().startsWith(`${base}/`) ? route.continue() : route.abort()); + const page = await context.newPage(); + const errors = []; + page.on("console", message => { if (message.type() === "error") errors.push(message.text()); }); + page.on("pageerror", error => errors.push(error.stack ?? error.message)); + const painted = () => page.waitForFunction(() => { + const snapshot = window.writer?.workspace?.getSnapshot(); + return snapshot?.artifact && !snapshot.compiling && Number(document.querySelector(".preview-pages")?.dataset.renderedRevision) === snapshot.artifactRevision && document.querySelector(".preview canvas[data-version]"); + }, undefined, { timeout: 60_000 }); + const body = () => page.evaluate(() => { const workspace = window.writer.workspace; return workspace.getSnapshot().records.get(workspace.main).snapshot.body; }); + await page.goto(`${base}/?demo&manuscript=manuscripts%2Fpatient-observation.md`); + await painted(); + const editor = page.locator(".cm-content[contenteditable=true]"); + await editor.click(); + await page.keyboard.press("Control+a"); + await page.keyboard.insertText(`# Long editing test\n\n## Section one\n\n${"A sentence with several words. ".repeat(380)}\n\n## Section two\n\n- First item\n- Second item\n\n[Link](https://example.com)\n`); + await painted(); + for (let round = 0; round < 4; round++) { + await page.keyboard.press("Control+End"); + await page.keyboard.type(" More text to exercise cursor changes, ordinary typing, undo and redo. ".repeat(4), { delay: 1 }); + const typed = await body(); + for (let i = 0; i < 3; i++) await page.keyboard.press("Control+z"); + for (let i = 0; i < 3; i++) await page.keyboard.press("Control+y"); + assert.equal(await body(), typed, "Undo/redo must preserve the final text"); + await painted(); + } + assert.deepEqual(errors, [], "Typing with a real canvas must not log React/CodeMirror errors"); + console.log("ok production editing, cursor movement and undo/redo with a real Preview canvas"); +} finally { + await browser?.close(); + await new Promise(resolve => server.close(resolve)); +} diff --git a/apps/writer/src/ui/OutlinePanel.test.tsx b/apps/writer/src/ui/OutlinePanel.test.tsx new file mode 100644 index 00000000..a1836449 --- /dev/null +++ b/apps/writer/src/ui/OutlinePanel.test.tsx @@ -0,0 +1,66 @@ +// @vitest-environment jsdom +import { act, cleanup, render } from "@testing-library/react"; +import { useCallback } from "react"; +import { createRecordTestAuthority } from "@mdbase-dev/connect-testing"; +import { afterEach, beforeEach, expect, it, vi } from "vitest"; +import type { RecordView } from "../workspace/workspace.js"; +import { OutlinePanel } from "./OutlinePanel.js"; + +const reveals = vi.hoisted(() => vi.fn()); +vi.mock("./paging.js", async importOriginal => { + const original = await importOriginal(); + return { useRowWindow(initial?: number) { + const { reveal, ...window } = original.useRowWindow(initial); + const tracked = useCallback((rank: number) => { reveals(rank); reveal(rank); }, [reveal]); + return { ...window, reveal: tracked }; + } }; +}); +const scrollIntoView = Object.getOwnPropertyDescriptor(HTMLElement.prototype, "scrollIntoView"); +beforeEach(() => Object.defineProperty(HTMLElement.prototype, "scrollIntoView", { configurable: true, value() {} })); +afterEach(() => { cleanup(); reveals.mockClear(); }); + +async function fixture(body: string) { + const authority = createRecordTestAuthority(); + authority.seed("main.md", { body, frontmatter: { title: "Document" } }); + const opened = await authority.records.open("main.md", { autosave: false }); + if (!opened.ok) throw new Error(opened.problem.code); + const { session, release } = opened.value; + const props = { main: "main.md", order: ["main.md"], chapters: [], words: 100, byRecord: new Map(), active: "main.md", onOpen: vi.fn(), onJump: vi.fn(), onMove: vi.fn(), onAdd: vi.fn() }; + const panel = (offset: number) => { + const view: RecordView = { path: "main.md", snapshot: session.getSnapshot() }; + return ; + }; + return { session, release, panel }; +} + +it("does not request the same heading window again when typing reparses its rows", async () => { + const body = "# Document\n\n## First\n\nText\n\n## Second\n\nMore text\n"; + const h = await fixture(body); + try { + const rendered = render(h.panel(body.indexOf("Text"))); + reveals.mockClear(); + for (let i = 0; i < 5; i++) { + act(() => h.session.setBody(body.replace("Text", `Text ${"more ".repeat(i + 1)}`))); + rendered.rerender(h.panel(body.indexOf("Text") + i)); + } + expect(reveals).not.toHaveBeenCalled(); + } finally { h.release(); } +}); + +it("still reveals a different logical heading beyond the first page", async () => { + const body = Array.from({ length: 65 }, (_, i) => `## Section ${i}\n\nText\n`).join("\n"); + const h = await fixture(body); + try { + const rendered = render(h.panel(0)); + const at = body.indexOf("## Section 55\n"); + expect(rendered.container.querySelector(`[data-heading="${at}"]`)).toBeNull(); + rendered.rerender(h.panel(at)); + expect(reveals).toHaveBeenCalledWith(55); + expect(rendered.container.querySelector(`[data-heading="${at}"]`)).not.toBeNull(); + } finally { h.release(); } +}); + +afterEach(() => { + if (scrollIntoView) Object.defineProperty(HTMLElement.prototype, "scrollIntoView", scrollIntoView); + else Reflect.deleteProperty(HTMLElement.prototype, "scrollIntoView"); +}); diff --git a/apps/writer/src/ui/OutlinePanel.tsx b/apps/writer/src/ui/OutlinePanel.tsx index 227c2e96..38b06589 100644 --- a/apps/writer/src/ui/OutlinePanel.tsx +++ b/apps/writer/src/ui/OutlinePanel.tsx @@ -270,7 +270,11 @@ const RecordHeadings = memo(function RecordHeadings({ view, path, cursor, onJump const activeRow = useRef(null); const headingList = useRef(null); const listRanks = useMemo(() => new Map(list.map((h, i) => [h.offset, i])), [list]); - useEffect(() => { window.reveal(here ? list.indexOf(here) : -1); }, [here?.offset, list]); + // Reveal a different logical row, not every new parse of the same row. + // Re-enqueuing an unchanged page after each edit keeps synchronous work + // pending while the workspace is updating and can exhaust React's limit. + const activeRank = here ? listRanks.get(here.offset) ?? -1 : -1; + useEffect(() => { window.reveal(activeRank); }, [activeRank, window.reveal]); useEffect(() => { activeRow.current?.scrollIntoView({ block: "nearest" }); }, [here?.offset, window.from, window.size]); diff --git a/docs/development.md b/docs/development.md index 5b96c5a7..a0fe6b45 100644 --- a/docs/development.md +++ b/docs/development.md @@ -40,6 +40,17 @@ This runs the baseline suite and fault-injection checks for recovery, early expo history, conflicts, cancellation and selectable preview text. Run only the latter with `pnpm --filter @mdbase-writer/app test:browser:reliability`. +The cursor/undo/redo regression also runs against optimized production React with a real +compiler and Preview canvas (development React can hide update-scheduling failures): + +```sh +pnpm --filter @mdbase-writer/app build:editing-test +pnpm --filter @mdbase-writer/app test:browser:editing +``` + +This test starts and closes its own loopback server, serves the installed compiler WASM, +and blocks external requests. Its demo-enabled bundle is a test fixture, not a deployment artifact. + The existing browser acceptance coverage uses the demo collection, including the LAB deployment. Acceptance against a real Connect collection has not yet been recorded in the architecture notes. Do not treat demo coverage as proof of connected-collection behaviour.