From 580eed8992b94dff920522d9776bef02a1ca902b Mon Sep 17 00:00:00 2001 From: marc Date: Tue, 25 Aug 2026 15:00:54 +0200 Subject: [PATCH] Ctrl and the wheel over a PDF MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The same gesture Aether has, whose comment already claimed InkNote had it. Registered by hand rather than through onWheel, because React attaches wheel listeners passively and a passive listener may not call preventDefault — without which the browser zooms the whole application instead of the document. What stays still depends on how the zoom was asked for, and the two answers differ: the wheel holds the point under the pointer, the buttons hold the page being read. The buttons held nothing at all before, so three presses of + left you somewhere else in the file; that came out of building this and is fixed with it. The checks are the step, that the default really was prevented, and that a point measured on the page is still under the pointer afterwards. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 25 +++++++ e2e/pdf.mjs | 78 ++++++++++++++++++++ frontend/src/features/pdf/PdfViewer.jsx | 98 +++++++++++++++++++++++-- readme.md | 17 ++++- 4 files changed, 211 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d0c997..7769b06 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,31 @@ a rebuild is not bit-identical. --- +## 2026-08-22 — Ctrl and the wheel over a PDF + +**Images** — web `sha256:783ccf92…`, server `sha256:2141f145…` + +No backend source changed this round; the server digest moved only because +`build.sh` builds both and a rebuild is not bit-identical. + +**Added** +- **Ctrl (or ⌘) and the wheel zoom the document**, as in every other viewer, and + a trackpad pinch with it — it arrives as the same event. The listener is + registered by hand rather than through `onWheel`: React attaches wheel + listeners passively, a passive listener may not call `preventDefault`, and + without that the *browser* zooms the whole application instead, toolbar and + all. **The point under the pointer stays under the pointer**: you are pointing + at a word and asking for that to grow, not to be sent to the top of the page. + +**Fixed** +- **Zooming with the buttons drifted.** A zoom changes how tall the document is, + so the scroll position means something else afterwards, and nothing was holding + anything still — three presses of + and you were elsewhere in the file. They + hold the page being read now, which is what "show me this page larger" asks + for. + +--- + ## 2026-08-22 — A bigger face, and the theme where the settings are **Images** — web `sha256:fe35b0e7…`, server `sha256:a557fb4d…` diff --git a/e2e/pdf.mjs b/e2e/pdf.mjs index 680b750..cb6a18a 100644 --- a/e2e/pdf.mjs +++ b/e2e/pdf.mjs @@ -270,6 +270,84 @@ try { await p.getByTitle("Zoom out").click(); await p.waitForTimeout(1500); + /* ── Ctrl and the wheel ── + * The claim is not only "the number changes". It is that the browser is kept + * out of it — an unprevented Ctrl+wheel zooms the whole application, toolbar + * and all — and that the spot under the pointer is the spot that stays put, + * which is the whole difference between a viewer that feels steady and one + * that jumps to the top of the page at every step. */ + const zoomNow = () => p.locator("[data-pdf-zoom]").evaluate((el) => Number(el.dataset.pdfZoom)); + const pageRect = () => + p.locator('[data-pdf-page="1"]').evaluate((el) => { + const r = el.getBoundingClientRect(); + return { top: r.top, height: r.height }; + }); + + const firstPage = await p.locator('[data-pdf-page="1"]').boundingBox(); + // A point well inside the page, and low enough that holding it still means + // scrolling rather than doing nothing. + const at = { x: firstPage.x + firstPage.width * 0.5, y: firstPage.y + firstPage.height * 0.6 }; + const beforeRect = await pageRect(); + const fraction = (at.y - beforeRect.top) / beforeRect.height; + const zoomBefore = await zoomNow(); + + const wheeled = await p.evaluate( + ([x, y]) => { + const el = document.elementFromPoint(x, y); + let prevented = null; + const spy = (e) => (prevented = e.defaultPrevented); + // Added after the viewer's own listener, so it runs after it and can see + // whether the default was called off. + document.addEventListener("wheel", spy, { passive: true }); + el.dispatchEvent( + new WheelEvent("wheel", { deltaY: -120, ctrlKey: true, bubbles: true, cancelable: true, clientX: x, clientY: y }) + ); + document.removeEventListener("wheel", spy); + return prevented; + }, + [at.x, at.y] + ); + await p.waitForTimeout(2000); + + check((await zoomNow()) > zoomBefore, "Ctrl and the wheel zoom in", `${zoomBefore} → ${await zoomNow()}`); + check(wheeled === true, "and the browser is stopped from zooming itself instead", `defaultPrevented ${wheeled}`); + + const afterRect = await pageRect(); + const held = afterRect.top + fraction * afterRect.height; + check( + Math.abs(held - at.y) < 12, + "and the spot under the pointer stays under the pointer", + `${Math.round(at.y)} → ${Math.round(held)}` + ); + + await p.evaluate( + ([x, y]) => { + document + .elementFromPoint(x, y) + .dispatchEvent( + new WheelEvent("wheel", { deltaY: 120, ctrlKey: true, bubbles: true, cancelable: true, clientX: x, clientY: y }) + ); + }, + [at.x, at.y] + ); + await p.waitForTimeout(1800); + check((await zoomNow()) === zoomBefore, "and the other way out again", `${await zoomNow()}`); + + /* The buttons hold something else still: the page being read. "Show me this + page larger" should not answer by moving to another one. */ + await p.locator('[data-pdf-page="2"]').scrollIntoViewIfNeeded(); + await p.waitForTimeout(1200); + const readingBefore = await p.locator("[data-pdf-position]").innerText(); + await p.getByTitle("Zoom in").click(); + await p.waitForTimeout(2200); + check( + (await p.locator("[data-pdf-position]").innerText()) === readingBefore, + "zooming from the buttons keeps the page being read", + `${readingBefore} → ${await p.locator("[data-pdf-position]").innerText()}` + ); + await p.getByTitle("Zoom out").click(); + await p.waitForTimeout(1500); + /* ── hiding it ── */ await p.locator("[data-pdf-hide-ink]").click(); await p.waitForTimeout(600); diff --git a/frontend/src/features/pdf/PdfViewer.jsx b/frontend/src/features/pdf/PdfViewer.jsx index 1abeb98..b19c2e3 100644 --- a/frontend/src/features/pdf/PdfViewer.jsx +++ b/frontend/src/features/pdf/PdfViewer.jsx @@ -500,12 +500,93 @@ export function PdfViewer({ note, dark, onUpdate, onPagesReady, notesToggle, foc } }; + /* ── zooming, and what stays still while it happens ── + * + * A zoom changes how tall the document is, and the scroll position means + * something different afterwards. Left alone, every step drifts: press + three + * times and you are somewhere else in the document. + * + * What should stay put depends on how the zoom was asked for. From the + * buttons, it is the page you are reading — the request is "show me this page + * larger". From the wheel, it is the point under the pointer: you are pointing + * at a word and asking for *that* to grow, and having the page jump to its top + * instead is exactly what makes a viewer feel unruly. + */ + const keepAnchor = useRef(null); + const keepPage = useRef(null); + + useLayoutEffect(() => { + const el = scrollRef.current; + if (!el) return; + const anchor = keepAnchor.current; + if (anchor) { + keepAnchor.current = null; + // The document grew by this much, so the point that was under the pointer + // is now that much further from the top. + const ratio = pageWidth / anchor.from; + el.scrollTop = Math.max(0, anchor.py * ratio - anchor.cy); + el.scrollLeft = Math.max(0, anchor.px * ratio - anchor.cx); + return; + } + const page = keepPage.current; + if (page === null) return; + keepPage.current = null; + const target = innerRef.current?.querySelector(`[data-pdf-page="${page}"]`); + // Not `goToPage`: that one glides, which is right for "take me to page 7" + // and wrong for holding still — the glide is a jump with a delay on it. + if (target) el.scrollTop = Math.max(0, target.offsetTop - 8); + }, [pageWidth]); + + /** The next step up or down, or nothing at the end of the ladder. */ + const nextZoom = (from, dir) => + dir > 0 ? ZOOM_STEPS.find((s) => s > from + 0.001) : [...ZOOM_STEPS].reverse().find((s) => s < from - 0.001); + const stepZoom = (dir) => { - const i = ZOOM_STEPS.indexOf(zoom); - const next = ZOOM_STEPS[Math.min(ZOOM_STEPS.length - 1, Math.max(0, (i < 0 ? 2 : i) + dir))]; - if (next) setZoom(next); + const next = nextZoom(zoom, dir); + if (!next) return; + keepPage.current = pageNo; + setZoom(next); }; + /** + * Ctrl (or ⌘) and the wheel, as in every other viewer. + * + * Registered by hand rather than with `onWheel`: React attaches wheel + * listeners passively, and a passive listener may not call `preventDefault`. + * Without that call the *browser* zooms the whole page instead — toolbar, + * sidebar and all — which is not what the gesture means over a document. + * + * A trackpad pinch arrives here as the same event, so it works too without + * anything further. + */ + useEffect(() => { + const el = scrollRef.current; + if (!el || state.status !== "ready") return; + const onWheel = (e) => { + if (!e.ctrlKey && !e.metaKey) return; + if (!e.deltaY) return; // a sideways wheel is not a zoom + e.preventDefault(); + const next = nextZoom(zoom, e.deltaY < 0 ? 1 : -1); + if (!next) return; + const r = el.getBoundingClientRect(); + const cx = e.clientX - r.left; + const cy = e.clientY - r.top; + keepAnchor.current = { + px: el.scrollLeft + cx, + py: el.scrollTop + cy, + cx, + cy, + // Measured in page width rather than in the zoom factor: the fitted + // width is what the pages are actually drawn from, and it changes on + // its own when the window is resized or the margin column appears. + from: pageWidth, + }; + setZoom(next); + }; + el.addEventListener("wheel", onWheel, { passive: false }); + return () => el.removeEventListener("wheel", onWheel); + }, [zoom, pageWidth, state.status]); + const barButton = { display: "flex", alignItems: "center", @@ -724,11 +805,16 @@ export function PdfViewer({ note, dark, onUpdate, onPagesReady, notesToggle, foc {notesToggle}
- - {Math.round(zoom * 100)}% - diff --git a/readme.md b/readme.md index cc48672..5208920 100644 --- a/readme.md +++ b/readme.md @@ -618,7 +618,7 @@ docker compose -f docker-compose.dev.yml up -d --build # must be running | `search-attachments` | 12 | body-only matches, ranking, umlauts; images as blob URLs, not inline bytes | | `history` | 10 | a large deletion is snapshotted, restoring works, restoring is itself undoable | | `drawing` | 22 | strokes survive the poller, match across four viewport widths, and stay on the page they were drawn on | -| `pdf` | 91 | import, page proportions, lazy loading, and reading with the network off | +| `pdf` | 96 | import, page proportions, lazy loading, and reading with the network off | | `attachments-lifecycle` | 10 | a picture is kept while any version shows it, and freed once none does | | `conflict` | 10 | a late write wins but the state it buries survives as a version | | `export` | 23 | the downloaded bytes are real Markdown, ZIP and PNG, unpacked and read back | @@ -880,6 +880,21 @@ page is expensive to draw and cheap to hold, so scrolling back up repaints nothing. Canvases are sized in device pixels and scaled down by CSS, which is what keeps text sharp on a high-density screen. +**Zooming holds something still.** A zoom changes how tall the document is, so +the scroll position means something different afterwards — left alone, every step +drifts and three presses of + leave you somewhere else in the file. What should +stay put depends on how the zoom was asked for, and the two answers are +different: from the buttons it is **the page you are reading**, because the +request is "show me this page larger"; from **Ctrl + the wheel** it is the point +under the pointer, because you are pointing at a word and asking for that to +grow. The wheel listener is registered by hand rather than through `onWheel`: +React attaches wheel listeners passively, a passive listener may not call +`preventDefault`, and without that call the *browser* zooms the whole application +— toolbar, sidebar and all. A trackpad pinch arrives as the same event and needs +nothing further. The suite checks all three: the step, that the default really +was prevented, and that a point picked on the page is still under the pointer +afterwards. + **pdf.js is not in the main bundle.** It is larger than the rest of the application put together, so it sits behind a dynamic import: a session that never opens a PDF never downloads it. The worker is bundled too, not fetched