From 8311eb1c231c1af2b2c0ecdbf49e127142d76f45 Mon Sep 17 00:00:00 2001 From: James Brunton Date: Fri, 21 Aug 2026 15:04:22 +0100 Subject: [PATCH] Switch to lazy loaded tracks to support large PDFs --- .../pageTracks/PageTracks.module.css | 23 +++-- .../core/components/pageTracks/PageTracks.tsx | 18 +++- .../components/pageTracks/TrackPageTile.tsx | 4 + .../core/components/pageTracks/TrackRow.tsx | 96 ++++++++++++++----- .../core/components/pageTracks/constants.ts | 21 ++++ .../core/tests/stubbed/page-tracks.spec.ts | 60 ++++++++++++ 6 files changed, 187 insertions(+), 35 deletions(-) create mode 100644 frontend/editor/src/core/components/pageTracks/constants.ts diff --git a/frontend/editor/src/core/components/pageTracks/PageTracks.module.css b/frontend/editor/src/core/components/pageTracks/PageTracks.module.css index 01fe378909..0013c93c8a 100644 --- a/frontend/editor/src/core/components/pageTracks/PageTracks.module.css +++ b/frontend/editor/src/core/components/pageTracks/PageTracks.module.css @@ -19,8 +19,8 @@ /* ── Track ───────────────────────────────────────────────────────────────── */ .track { - --pt-tile-h: 11.5rem; - --pt-tile-w: 8.5rem; + /* --pt-tile-w / --pt-tile-h / --pt-tile-footer-h / --pt-gap come from + constants.ts, so the virtualiser and the CSS agree on tile geometry. */ /* The scroller is a flex column: without this, an overflowing stack of tracks squeezes each lane below its tiles and clips them. */ flex: 0 0 auto; @@ -71,19 +71,25 @@ } .lane { - display: flex; - align-items: flex-start; - gap: 0.5rem; + position: relative; padding: 0.75rem; overflow-x: auto; overflow-y: hidden; } +/* Sized by the virtualiser; the tiles inside are absolutely placed, so the + height has to be stated rather than derived from content. */ +.laneInner { + position: relative; + height: calc(var(--pt-tile-h) + var(--pt-tile-footer-h)); +} + .laneHint { pointer-events: none; } .laneEmpty { + display: flex; align-items: center; justify-content: center; min-height: calc(var(--pt-tile-h) + 2.5rem); @@ -94,8 +100,8 @@ /* ── Tile ────────────────────────────────────────────────────────────────── */ .tile { - position: relative; - flex: 0 0 auto; + position: absolute; + top: 0; width: var(--pt-tile-w); cursor: grab; user-select: none; @@ -161,7 +167,8 @@ align-items: center; justify-content: space-between; gap: 0.25rem; - padding: 0.25rem 0.375rem; + height: var(--pt-tile-footer-h); + padding: 0 0.375rem; font-size: 0.6875rem; color: var(--c-text-muted); min-width: 0; diff --git a/frontend/editor/src/core/components/pageTracks/PageTracks.tsx b/frontend/editor/src/core/components/pageTracks/PageTracks.tsx index 64eaa11217..fc5eb9c0ca 100644 --- a/frontend/editor/src/core/components/pageTracks/PageTracks.tsx +++ b/frontend/editor/src/core/components/pageTracks/PageTracks.tsx @@ -42,6 +42,13 @@ const collisionDetection: CollisionDetection = (args) => { return rectIntersection(args); }; +const sameHint = (a: DropHint | null, b: DropHint | null): boolean => + a === b || + (a != null && + b != null && + a.fileId === b.fileId && + a.beforePageId === b.beforePageId); + /** * The pointer x dnd-kit is itself working from: the activator's position plus * the drag delta. Using this rather than a live pointermove listener keeps the @@ -182,12 +189,13 @@ export default function PageTracks() { // latter. Recomputing per move is what keeps the marker and the drop in sync. const handleDragMove = useCallback( (event: DragMoveEvent) => { - setDropHint( - resolveHint( - event.over ? String(event.over.id) : null, - pointerXOf(event), - ), + const next = resolveHint( + event.over ? String(event.over.id) : null, + pointerXOf(event), ); + // Most moves land on the same side of the same tile. Keeping the previous + // object bails the re-render out, so only a real change costs anything. + setDropHint((prev) => (sameHint(prev, next) ? prev : next)); }, [resolveHint], ); diff --git a/frontend/editor/src/core/components/pageTracks/TrackPageTile.tsx b/frontend/editor/src/core/components/pageTracks/TrackPageTile.tsx index ace33e9985..89dfd9f522 100644 --- a/frontend/editor/src/core/components/pageTracks/TrackPageTile.tsx +++ b/frontend/editor/src/core/components/pageTracks/TrackPageTile.tsx @@ -24,6 +24,8 @@ export interface TrackPageTileProps { trackFileId: FileId; /** 1-based position within the track. */ position: number; + /** Horizontal offset within the lane, from the virtualiser. */ + offsetX: number; selected: boolean; dragging: boolean; /** Draw the insertion line on this tile's leading edge. */ @@ -44,6 +46,7 @@ function TrackPageTileImpl({ page, trackFileId, position, + offsetX, selected, dragging, dropBefore, @@ -98,6 +101,7 @@ function TrackPageTileImpl({ ] .filter(Boolean) .join(" ")} + style={{ left: offsetX }} {...attributes} {...listeners} data-page-id={page.id} diff --git a/frontend/editor/src/core/components/pageTracks/TrackRow.tsx b/frontend/editor/src/core/components/pageTracks/TrackRow.tsx index df1ba07bec..c4bceb9e9c 100644 --- a/frontend/editor/src/core/components/pageTracks/TrackRow.tsx +++ b/frontend/editor/src/core/components/pageTracks/TrackRow.tsx @@ -1,4 +1,5 @@ -import React, { useCallback, useMemo } from "react"; +import React, { useCallback, useMemo, useRef } from "react"; +import { useVirtualizer } from "@tanstack/react-virtual"; import { useDroppable } from "@dnd-kit/core"; import { useTranslation } from "react-i18next"; import RotateLeftIcon from "@mui/icons-material/RotateLeft"; @@ -12,6 +13,10 @@ import { Track } from "@app/components/pageTracks/types"; import { TrackThumbnailStore } from "@app/components/pageTracks/hooks/useTrackThumbnails"; import { PageClickModifiers } from "@app/components/pageTracks/hooks/useTrackSelection"; import TrackPageTile from "@app/components/pageTracks/TrackPageTile"; +import { + TRACK_GEOMETRY, + rootFontSizePx, +} from "@app/components/pageTracks/constants"; import styles from "@app/components/pageTracks/PageTracks.module.css"; export const trackDroppableId = (fileId: FileId) => `track:${fileId}`; @@ -64,6 +69,40 @@ function TrackRowImpl({ data: { type: "track", fileId: track.fileId }, }); + // A lane can hold hundreds of pages. Mounting them all is what made a single + // click cost ~700ms and a drag ~300ms per pointer move: every tile is a + // dnd-kit draggable AND droppable, so the whole set gets re-registered on + // each render, re-measured on drag start and hit-tested on every move. + const laneRef = useRef(null); + const geometry = useMemo(() => { + const px = rootFontSizePx(); + return { + tileWidth: TRACK_GEOMETRY.tileWidthRem * px, + stride: (TRACK_GEOMETRY.tileWidthRem + TRACK_GEOMETRY.gapRem) * px, + cssVars: { + "--pt-tile-w": `${TRACK_GEOMETRY.tileWidthRem}rem`, + "--pt-tile-h": `${TRACK_GEOMETRY.tileCanvasHeightRem}rem`, + "--pt-tile-footer-h": `${TRACK_GEOMETRY.tileFooterHeightRem}rem`, + } as React.CSSProperties, + }; + }, []); + + const virtualizer = useVirtualizer({ + count: track.pages.length, + horizontal: true, + getScrollElement: () => laneRef.current, + estimateSize: () => geometry.stride, + overscan: TRACK_GEOMETRY.overscan, + }); + + const setLaneRef = useCallback( + (element: HTMLDivElement | null) => { + laneRef.current = element; + setNodeRef(element); + }, + [setNodeRef], + ); + // Track-level actions apply to the selection inside this track, falling back // to the whole track so the buttons stay useful with nothing selected. const targetIds = useMemo(() => { @@ -84,6 +123,7 @@ function TrackRowImpl({ return (
)} - {track.pages.map((page, index) => ( - - ))} + {track.pages.length > 0 && ( +
+ {virtualizer.getVirtualItems().map((item) => { + const page = track.pages[item.index]; + if (!page) return null; + return ( + + ); + })} +
+ )}
); diff --git a/frontend/editor/src/core/components/pageTracks/constants.ts b/frontend/editor/src/core/components/pageTracks/constants.ts new file mode 100644 index 0000000000..48407584f8 --- /dev/null +++ b/frontend/editor/src/core/components/pageTracks/constants.ts @@ -0,0 +1,21 @@ +/** + * Lane geometry. Single source of truth: the numbers are pushed onto the track + * element as CSS custom properties AND used by the horizontal virtualiser, so + * the two can never drift apart. + */ +export const TRACK_GEOMETRY = { + tileWidthRem: 8.5, + tileCanvasHeightRem: 11.5, + tileFooterHeightRem: 1.375, + gapRem: 0.5, + /** Extra tiles rendered either side of the visible window. */ + overscan: 6, +} as const; + +export const rootFontSizePx = (): number => { + if (typeof window === "undefined") return 16; + const parsed = parseFloat( + getComputedStyle(document.documentElement).fontSize, + ); + return Number.isNaN(parsed) ? 16 : parsed; +}; diff --git a/frontend/editor/src/core/tests/stubbed/page-tracks.spec.ts b/frontend/editor/src/core/tests/stubbed/page-tracks.spec.ts index 7b56b5cea3..fa235575e4 100644 --- a/frontend/editor/src/core/tests/stubbed/page-tracks.spec.ts +++ b/frontend/editor/src/core/tests/stubbed/page-tracks.spec.ts @@ -1,5 +1,9 @@ +import fs from "node:fs"; +import os from "node:os"; import path from "node:path"; +import { PDFDocument, StandardFonts } from "@cantoo/pdf-lib"; + import { test, expect } from "@app/tests/helpers/stub-test-base"; import { uploadFiles, dismissTourTooltip } from "@app/tests/helpers/ui-helpers"; @@ -415,4 +419,60 @@ test.describe("Page Editor tracks", () => { page.locator('[data-page-id][data-selected="true"]'), ).toHaveCount(2); }); + + test("a long track mounts only a window of its pages", async ({ page }) => { + // Built here rather than committed as a fixture: the point is the page + // COUNT, and 300 pages of real PDF is a lot of bytes to carry in the repo. + const doc = await PDFDocument.create(); + const font = await doc.embedFont(StandardFonts.Helvetica); + for (let i = 1; i <= 300; i++) { + doc + .addPage([595, 842]) + .drawText(`page ${i}`, { x: 60, y: 700, size: 28, font }); + } + const longPdf = path.join(os.tmpdir(), `tracks-long-${process.pid}.pdf`); + fs.writeFileSync(longPdf, await doc.save()); + + try { + await page.setViewportSize({ width: 1280, height: 800 }); + await page.goto("/editor", { + waitUntil: "domcontentloaded", + timeout: 120_000, + }); + await uploadFiles(page, longPdf); + await dismissTourTooltip(page); + await switchView(page, "Page Editor"); + + const lane = page.locator("[data-track-lane]"); + await expect(page.locator("section[aria-label]").first()).toContainText( + "300 pages", + { timeout: 60_000 }, + ); + + // Mounting all 300 is what made a click cost ~700ms and a drag ~300ms per + // pointer move, since every tile is a dnd-kit draggable AND droppable. + const mounted = await page.locator("[data-page-id]").count(); + expect(mounted).toBeGreaterThan(0); + expect(mounted).toBeLessThan(40); + + // The window follows the lane's scroll rather than being a fixed prefix. + const firstBefore = await page + .locator("[data-page-id]") + .first() + .getAttribute("data-page-id"); + await lane.evaluate((el) => { + el.scrollLeft = 9000; + }); + await expect + .poll( + async () => + page.locator("[data-page-id]").first().getAttribute("data-page-id"), + { timeout: 30_000 }, + ) + .not.toBe(firstBefore); + expect(await page.locator("[data-page-id]").count()).toBeLessThan(40); + } finally { + fs.rmSync(longPdf, { force: true }); + } + }); });