From 791cf625670ea6952d4175a8fdbe81570e45877b Mon Sep 17 00:00:00 2001 From: Lycoon Date: Thu, 13 Aug 2026 00:49:01 +0200 Subject: [PATCH] fixed pdf export on mobile endless scroll, fixed revision stripe rendering on straddling nodes on mobile --- src/lib/adapters/pdf/pdf-adapter.ts | 100 ++++++++++++------ .../extensions/revisions-extension.ts | 33 ++++-- ....test.ts => pdf-layout-invariance.test.ts} | 95 +++++++++++++---- styles/scriptio.css | 23 +++- 4 files changed, 186 insertions(+), 65 deletions(-) rename src/tests/adapters/{pdf-scale-invariance.test.ts => pdf-layout-invariance.test.ts} (56%) diff --git a/src/lib/adapters/pdf/pdf-adapter.ts b/src/lib/adapters/pdf/pdf-adapter.ts index caa8c3e4..c5cb3945 100644 --- a/src/lib/adapters/pdf/pdf-adapter.ts +++ b/src/lib/adapters/pdf/pdf-adapter.ts @@ -59,6 +59,10 @@ const PDF_PAGE_SIZES: Record = { A4: { width: PAGE_SIZES.A4.pageWidth * PX_TO_PT, height: PAGE_SIZES.A4.pageHeight * PX_TO_PT }, }; +/** Display-only declarations the measurement pass overrides on each editor and + * restores afterwards — see {@link PDFAdapter.withCanonicalLayout}. */ +const CANONICAL_PINNED_PROPERTIES = ["transform", "width", "--display-margin-scale"] as const; + // ─── Helpers ───────────────────────────────────────────────────────────────── /** Map a `ScriptFont` value to the jsPDF font-family name. */ @@ -131,10 +135,10 @@ export class PDFAdapter extends ProjectAdapter { const titlePageEl = options.titlePageElement; // Every coordinate below comes from the live DOM, so the whole geometry - // pass runs with the editor's on-screen scaling pinned to 1× — see - // `withCanonicalScale`. Keeping it in a single closure means the layout - // is neutralised (and restored) exactly once per export. - const measured = this.withCanonicalScale([editorEl, titlePageEl], () => { + // pass runs with the editor's display-only layout neutralised — see + // `withCanonicalLayout`. Keeping it in a single closure means the layout + // is pinned (and restored) exactly once per export. + const measured = this.withCanonicalLayout([editorEl, titlePageEl], () => { // Header/footer columns are laid out within the configured page // margins: the editor's `.pagination-header-area` / // `.pagination-footer-area` are padded by @@ -242,37 +246,60 @@ export class PDFAdapter extends ProjectAdapter { throw new Error("Method not implemented."); } - // ── Canonical (unscaled) measurement ──────────────────────────────────── + // ── Canonical (page-shaped) measurement ───────────────────────────────── /** - * Run `measure` with the editors' display-only scaling pinned to 1×, so - * every DOM coordinate it reads is the canonical page geometry. + * Run `measure` with the editors' display-only layout pinned to the + * canonical page, so every DOM coordinate it reads is the real page + * geometry rather than whatever the current screen renders. + * + * Two phone view modes deform that geometry, and both feed straight into + * `getBoundingClientRect()` / `Range.getClientRects()`, the only source of + * coordinates in this exporter: + * + * - PAGED: the page is scaled to fit the viewport — `transform: + * scale(var(--editor-zoom))`. Left in place, a 0.48× fit shrinks every X + * offset and line gap by half while the PDF still draws a fixed 12pt + * font. + * - ENDLESS: there is no page rectangle at all — the editor is widened to + * the viewport (`width: 100%`) and the screenplay margins are compressed + * to `--display-margin-scale: 0.3` so text reflows large on a narrow + * screen (see EditorPanel.module.css). Left in place, the export keeps + * those compressed margins and the viewport's much earlier line wrapping + * — the layout of the PDF is then the phone's, not the page's. * - * The editor is scaled on screen in phone paged mode — `transform: - * scale(var(--editor-zoom))`, the fit-to-width ratio — which feeds straight - * into `getBoundingClientRect()` / `Range.getClientRects()`, the only source - * of coordinates in this exporter. Left in place, a 0.48× fit shrinks every - * X offset and line gap by half while the PDF still draws a fixed 12pt font, - * so the exported layout would depend on the screen the user happened to be - * writing on. + * So both are neutralised here: the scale is dropped, the margins go back + * to 1×, and the editor is widened back to `--page-width` (the same custom + * property the pagination stylesheet sizes the page from, left untouched by + * either mode; skipped if it isn't set, rather than collapsing the element + * to `width: auto`). * - * Pinning the layout is preferred over dividing the measurements by the - * scale factor: `getComputedStyle` lengths (which this pass also reads) do - * not follow the transform, so the two would need opposite corrections. The - * override is set `!important` so no stylesheet rule can outvote it, and the - * original inline declaration is restored afterwards. + * Pinning the layout is preferred over correcting the measurements after + * the fact: `getComputedStyle` lengths (which this pass also reads) don't + * follow the transform, so scale and margins would need opposite + * corrections. Overrides are set `!important` so no stylesheet rule can + * outvote them, and the original inline declarations are restored + * afterwards. + * + * The hidden page-break widgets of endless mode need no such treatment: + * `collectLines` finds them by class whatever their `display`, and the + * worker resets its Y cursor to the top of the page on every break, so the + * gap those widgets would have occupied is never read. * * Nothing here yields to the event loop, so the browser never paints the - * unscaled state — the export is invisible to the user. Scroll offsets are - * restored explicitly, since dropping the scale can change the layout box - * and clamp the scroll position of every scrollable ancestor. + * pinned state — the export is invisible to the user. Scroll offsets are + * restored explicitly, since re-shaping the page changes the layout box and + * can clamp the scroll position of every scrollable ancestor. */ - private withCanonicalScale(elements: (HTMLElement | undefined)[], measure: () => T): T { + private withCanonicalLayout(elements: (HTMLElement | undefined)[], measure: () => T): T { const targets = elements.filter((el): el is HTMLElement => !!el); const savedStyles = targets.map((el) => ({ el, - transform: el.style.getPropertyValue("transform"), - transformPriority: el.style.getPropertyPriority("transform"), + declarations: CANONICAL_PINNED_PROPERTIES.map((name) => ({ + name, + value: el.style.getPropertyValue(name), + priority: el.style.getPropertyPriority(name), + })), })); const savedScroll = new Map(); @@ -286,21 +313,34 @@ export class PDFAdapter extends ProjectAdapter { try { for (const el of targets) { + // Both writers of --page-width (the pagination extension's + // syncVars and the editor wrapper's inline style) emit px, so + // anything else is not a length to pin to — leave the width as + // it is rather than guess at it. + const rawPageWidth = getComputedStyle(el).getPropertyValue("--page-width").trim(); + const pageWidthPx = rawPageWidth.endsWith("px") ? parseFloat(rawPageWidth) : NaN; el.style.setProperty("transform", "none", "important"); + el.style.setProperty("--display-margin-scale", "1", "important"); + if (Number.isFinite(pageWidthPx) && pageWidthPx > 0) { + el.style.setProperty("width", `${pageWidthPx}px`, "important"); + } } // No explicit reflow needed: the first geometry read inside // `measure` flushes the pending layout for us. return measure(); } finally { for (const saved of savedStyles) { - const restore = (name: string, value: string, priority: string) => { + for (const { name, value, priority } of saved.declarations) { + // Always clear first: WebKit ignores a `setProperty` that + // lowers a custom property's priority, so overwriting the + // `!important` pin in place would leave the editor stuck at + // the canonical value (a 1× margin scale on a phone). + saved.el.style.removeProperty(name); if (value) saved.el.style.setProperty(name, value, priority); - else saved.el.style.removeProperty(name); - }; - restore("transform", saved.transform, saved.transformPriority); + } } // Assigning scroll offsets flushes the restored layout first, so - // these land against the scaled extents they were taken from. + // these land against the on-screen extents they were taken from. for (const [node, pos] of savedScroll) { node.scrollTop = pos.top; node.scrollLeft = pos.left; diff --git a/src/lib/screenplay/extensions/revisions-extension.ts b/src/lib/screenplay/extensions/revisions-extension.ts index 35c068d1..d4dcfd09 100644 --- a/src/lib/screenplay/extensions/revisions-extension.ts +++ b/src/lib/screenplay/extensions/revisions-extension.ts @@ -512,15 +512,30 @@ const renderOverlay = ( if (parent !== doc) return false; // top-level nodes only; don't descend while (bp < breaks.length && breaks[bp].pos <= pos) bp++; - // A node whose own span contains the next break is split across two pages - // by a mid-node sentence-break widget. Its per-line offsets (measured - // relative to the node top) then include the inter-page gap, so a cache - // entry taken before it straddled would be stale — placing the far-side - // asterisk on the wrong page (or outside the content band, where it's - // dropped). Always measure such a node fresh and never cache it. They - // exist only at a page boundary and are measured only when actually - // carrying marks, so the hot path is unaffected. - const straddles = bp < breaks.length && breaks[bp].pos < pos + node.nodeSize; + // A node whose own span contains a break is split across two pages by a + // mid-node sentence-break widget. Its per-line offsets (measured relative + // to the node top) then include the inter-page gap, so a cache entry taken + // before it straddled would be stale — placing the far-side asterisk on + // the wrong page (or outside the content band, where it's dropped). Always + // measure such a node fresh and never cache it. They exist only at a page + // boundary and are measured only when actually carrying marks, so the hot + // path is unaffected. + // + // TWO breaks can fall inside the node, and both have to be tested. The + // obvious one is the next break ahead of it. The other only bites the + // FIRST node the walk visits: `bp` is seeded from the page arithmetic, so + // it already points *past* the break the visible window opens on — yet + // that break's position is `fromPos`, which is exactly what pulls a node + // straddling it into the walk. Testing `breaks[bp]` alone reads the node + // as unsplit, so it takes the cache path and stores gap-inclusive offsets; + // once the break later moves off the node those stale offsets are what get + // painted, throwing its asterisks a page out and leaving the page they + // belonged to with no stripe. `breaks[bp - 1]` can only sit after `pos` + // in that seeded case (the loop above advances past every break at or + // before `pos`), so the extra test costs one comparison and never + // false-positives. + const straddles = + (bp < breaks.length && breaks[bp].pos < pos + node.nodeSize) || (bp > 0 && breaks[bp - 1].pos > pos); let lines = straddles ? undefined : cache.map.get(node); if (lines === undefined) { diff --git a/src/tests/adapters/pdf-scale-invariance.test.ts b/src/tests/adapters/pdf-layout-invariance.test.ts similarity index 56% rename from src/tests/adapters/pdf-scale-invariance.test.ts rename to src/tests/adapters/pdf-layout-invariance.test.ts index a8527546..62de6986 100644 --- a/src/tests/adapters/pdf-scale-invariance.test.ts +++ b/src/tests/adapters/pdf-layout-invariance.test.ts @@ -4,11 +4,14 @@ import { PDFAdapter, type PDFExportOptions } from "@src/lib/adapters/pdf/pdf-ada import type { VisualLine } from "@src/lib/adapters/pdf/pdf.worker"; /** - * The PDF exporter reads its geometry from the live editor DOM, which is scaled - * on screen in phone paged mode (`transform: scale(var(--editor-zoom))`, the - * fit-to-width ratio). These tests mount a miniature editor, scale it, and - * assert the collected lines are the same as at 1× — the PDF must not depend on - * the viewport the script happens to be open on. + * The PDF exporter reads its geometry from the live editor DOM, which the two + * phone view modes deform: paged scales the page to fit the viewport + * (`transform: scale(var(--editor-zoom))`) and endless drops the page + * rectangle altogether, widening the editor to the viewport and compressing the + * screenplay margins (`--display-margin-scale`). These tests mount a miniature + * editor, deform it each way, and assert the collected lines still match the + * canonical page — the PDF must not depend on the viewport the script happens + * to be open on. * * Runs in real Chromium and WebKit (see vitest.config.ts): the whole point is * browser layout, which jsdom cannot provide. @@ -18,7 +21,7 @@ import type { VisualLine } from "@src/lib/adapters/pdf/pdf.worker"; // booting a worker + jsPDF to produce a blob. type AdapterInternals = { collectLines(el: HTMLElement, options: PDFExportOptions): VisualLine[]; - withCanonicalScale(elements: (HTMLElement | undefined)[], measure: () => T): T; + withCanonicalLayout(elements: (HTMLElement | undefined)[], measure: () => T): T; getPageLeftPx(el: HTMLElement): number; }; @@ -38,15 +41,22 @@ afterEach(() => { /** * Mount a stand-in for the editor: a scroll container holding a page-width - * `.ProseMirror` whose scale is driven by the same CSS variable the real - * stylesheet uses, so pinning it exercises the production code path. + * `.ProseMirror` whose scale, width and margin compression are driven by the + * same CSS variables and `!important` overrides the real stylesheets use, so + * pinning them exercises the production code path. + * + * `.endless` on the scroller reproduces the phone endless-scroll mode from + * EditorPanel.module.css: a phone-narrow viewport, an editor widened to fill it + * instead of the page, and screenplay margins compressed to 0.3×. */ const mountEditor = () => { const style = document.createElement("style"); style.textContent = ` .test-scroller { width: 600px; height: 300px; overflow: auto; } .test-pm { - width: 612px; + --page-width: 612px; + --display-margin-scale: 1; + width: var(--page-width) !important; box-sizing: border-box; font: 16px monospace; line-height: 16px; @@ -55,9 +65,16 @@ const mountEditor = () => { transform: scale(var(--editor-zoom, 1)); transform-origin: top center; } - .test-pm p { margin: 0 0 16px 0; padding: 0 96px; } - .test-pm p.dialogue { padding: 0 168px 0 240px; } - .test-pm p.character { padding: 0 0 0 336px; text-transform: uppercase; } + .test-pm p { margin: 0 0 16px 0; padding: 0 calc(96px * var(--display-margin-scale)); } + .test-pm p.dialogue { + padding: 0 calc(168px * var(--display-margin-scale)) 0 calc(240px * var(--display-margin-scale)); + } + .test-pm p.character { + padding: 0 0 0 calc(336px * var(--display-margin-scale)); + text-transform: uppercase; + } + .test-scroller.endless { width: 390px; } + .test-scroller.endless .test-pm { width: 100% !important; --display-margin-scale: 0.3; } `; document.head.appendChild(style); @@ -99,7 +116,12 @@ const signature = (adapter: PDFAdapter, editor: HTMLElement): string[] => { }); }; -describe("PDF export is invariant of the editor's on-screen scale", () => { +/** Page-relative X of the first line of the given paragraph type, out of a + * {@link signature} — its third field. */ +const firstX = (lines: string[], type: string): number => + Number(lines.find((line) => line.startsWith(`${type}|`))!.split("|")[2]); + +describe("PDF export is invariant of the editor's on-screen layout", () => { // 0.48 is about what a phone-width viewport fits a US Letter page to; 0.8 // covers a roomier device so the assertion isn't tied to one ratio. for (const scale of [0.48, 0.8]) { @@ -116,38 +138,63 @@ describe("PDF export is invariant of the editor's on-screen scale", () => { // contaminated by the scale, otherwise nothing is being proven. expect(signature(adapter, editor)).not.toEqual(canonical); - const pinned = internals(adapter).withCanonicalScale([editor], () => signature(adapter, editor)); + const pinned = internals(adapter).withCanonicalLayout([editor], () => signature(adapter, editor)); expect(pinned).toEqual(canonical); }); } - it("restores the on-screen scale and scroll position afterwards", () => { + it("matches the page layout in phone endless-scroll mode", () => { const adapter = new PDFAdapter(); const { scroller, editor } = mountEditor(); - editor.style.setProperty("--editor-zoom", "0.48"); + + const canonical = signature(adapter, editor); + expect(canonical.length).toBeGreaterThan(6); + + scroller.classList.add("endless"); + + // Endless reflows the text into the viewport at 0.3× margins, so the + // raw measurements must actually be contaminated — the whole layout + // differs, and every left offset is compressed towards the page edge — + // otherwise the assertion below proves nothing. (Line *count* is no + // guard here: the compressed margins widen the dialogue column as much + // as they narrow the action one, so the totals can coincide.) + const reflowed = signature(adapter, editor); + expect(reflowed).not.toEqual(canonical); + expect(firstX(reflowed, "dialogue")).toBeLessThan(firstX(canonical, "dialogue")); + + const pinned = internals(adapter).withCanonicalLayout([editor], () => signature(adapter, editor)); + expect(pinned).toEqual(canonical); + }); + + it("restores the on-screen layout and scroll position afterwards", () => { + const adapter = new PDFAdapter(); + const { scroller, editor } = mountEditor(); + scroller.classList.add("endless"); editor.style.setProperty("opacity", "0.9"); // an unrelated inline style must survive - const scaledWidth = editor.getBoundingClientRect().width; + const reflowedWidth = editor.getBoundingClientRect().width; scroller.scrollTop = scroller.scrollHeight - scroller.clientHeight; const { scrollTop } = scroller; expect(scrollTop).toBeGreaterThan(0); - internals(adapter).withCanonicalScale([editor], () => signature(adapter, editor)); + internals(adapter).withCanonicalLayout([editor], () => signature(adapter, editor)); - expect(editor.getBoundingClientRect().width).toBeCloseTo(scaledWidth, 1); + expect(editor.getBoundingClientRect().width).toBeCloseTo(reflowedWidth, 1); expect(editor.style.transform).toBe(""); + expect(editor.style.width).toBe(""); + expect(editor.style.getPropertyValue("--display-margin-scale")).toBe(""); expect(editor.style.opacity).toBe("0.9"); expect(scroller.scrollTop).toBe(scrollTop); }); - it("restores the scale even when the measurement throws", () => { + it("restores the layout even when the measurement throws", () => { const adapter = new PDFAdapter(); const { editor } = mountEditor(); editor.style.setProperty("--editor-zoom", "0.48"); const scaledWidth = editor.getBoundingClientRect().width; expect(() => - internals(adapter).withCanonicalScale([editor], () => { + internals(adapter).withCanonicalLayout([editor], () => { throw new Error("measurement failed"); }), ).toThrow("measurement failed"); @@ -155,14 +202,16 @@ describe("PDF export is invariant of the editor's on-screen scale", () => { expect(editor.getBoundingClientRect().width).toBeCloseTo(scaledWidth, 1); }); - it("keeps an inline scale the editor itself set", () => { + it("keeps inline display overrides the editor itself set", () => { const adapter = new PDFAdapter(); const { editor } = mountEditor(); editor.style.setProperty("transform", "scale(0.5)", "important"); + editor.style.setProperty("--display-margin-scale", "0.3"); - internals(adapter).withCanonicalScale([editor], () => signature(adapter, editor)); + internals(adapter).withCanonicalLayout([editor], () => signature(adapter, editor)); expect(editor.style.transform).toBe("scale(0.5)"); expect(editor.style.getPropertyPriority("transform")).toBe("important"); + expect(editor.style.getPropertyValue("--display-margin-scale")).toBe("0.3"); }); }); diff --git a/styles/scriptio.css b/styles/scriptio.css index 4c06694a..1a5c69c3 100644 --- a/styles/scriptio.css +++ b/styles/scriptio.css @@ -301,15 +301,32 @@ /* A single overlay layer mounted at the editor origin (a stable widget decoration). It owns the absolutely-positioned stripe/asterisk elements painted by the revisions extension's overlay renderer; their top/left/ - size/colour are all set inline. Sits above the content background but - below the pagination chrome (z-index 10) at the page margins. */ + size/colour are all set inline. + + z-index 11 paints it above .pagination-overlay (z-index: 10), same tier as + .spellcheck-error below and for the same reason. It only matters where the + stripe is tucked INSIDE the page's left margin instead of the canvas gutter + — the phone's paged view, and pinched desktop windows (see the stripe + geometry in revisions-extension.ts) — because there it shares x-range with + the page chrome, which paints an opaque page background across the full + page width. + + Most break widgets are harmless at z-index 4: content-visibility: auto + (containOffscreen) implies paint containment, which makes the widget a + stacking context and confines its overlay to the widget's own z-index:auto + level. MID-NODE SENTENCE SPLITS are the exception — they are deliberately + exempt from that containment (it would clip the overlay escaping the + parent

's padding), so their z-index:10 overlay competes directly here + and used to paint its footer/gap/header band straight over the stripe of a + node straddling the break. Asterisks are culled to the content band, so + raising the layer never puts one over a header or footer. */ .revision-overlay { position: absolute; top: 0; left: 0; width: 0; height: 0; - z-index: 4; + z-index: 11; pointer-events: none; }