From 3405200cf45562be3efbf86a943ece1d7d7f4012 Mon Sep 17 00:00:00 2001 From: Amruth Pillai Date: Tue, 29 Sep 2026 08:25:06 +0200 Subject: [PATCH] fix(pdf): keep a list marker with its item's first line --- packages/pdf/src/forme/render.ts | 36 +++++++++++++++-- packages/pdf/src/forme/to-forme.tsx | 39 ++++++++++++++++--- .../templates/shared/list-pagination.test.tsx | 19 ++++----- 3 files changed, 77 insertions(+), 17 deletions(-) diff --git a/packages/pdf/src/forme/render.ts b/packages/pdf/src/forme/render.ts index b737ae8f3..4b203d592 100644 --- a/packages/pdf/src/forme/render.ts +++ b/packages/pdf/src/forme/render.ts @@ -15,7 +15,7 @@ import { loadFonts } from "./fonts"; import { loadIcons } from "./icons"; import { imageSources, loadImages } from "./images"; import { renderHostTree } from "./reconciler"; -import { FIXED_SOURCE, FREE_FORM_MEASURE_HEIGHT, toFormeDocument } from "./to-forme"; +import { FIXED_SOURCE, FREE_FORM_MEASURE_HEIGHT, LIST_ROLE, toFormeDocument } from "./to-forme"; /** The Forme entry point for this runtime: `@formepdf/core` in Node, `@formepdf/core/worker` in browsers. */ export type FormeEngine = { @@ -59,6 +59,24 @@ const misplacesABox = (result: RenderWithLayoutResult) => const isFixed = (element: ElementInfo): boolean => element.sourceLocation?.file === FIXED_SOURCE || element.children.some(isFixed); +/** List items whose marker sits on an earlier page than their first line (see `LIST_ROLE`). */ +export function listMarkersLeftBehind(layout: RenderWithLayoutResult["layout"]): number[] { + const markerPage = new Map(); + const contentPage = new Map(); + layout.pages.forEach((page, pageIndex) => { + const visit = (element: ElementInfo) => { + const source = element.sourceLocation; + const pages = + source?.column === LIST_ROLE.marker ? markerPage : source?.column === LIST_ROLE.content ? contentPage : null; + // Forme leaves some fragments of a splitting box at y ±Number.MAX_VALUE; they aren't on this page. + if (source && pages && !pages.has(source.line) && !offPage(element.y)) pages.set(source.line, pageIndex); + element.children.forEach(visit); + }; + page.elements.forEach(visit); + }); + return [...markerPage].flatMap(([line, page]) => ((contentPage.get(line) ?? page) > page ? [line - 1] : [])); +} + /** A free-form page's content height: the lowest box on it plus the page's bottom margin. */ function measuredHeight(result: RenderWithLayoutResult, pageIndex: number, marginBottom: number) { const page = result.layout.pages[pageIndex]; @@ -105,8 +123,9 @@ export async function renderResumeElement(engine: FormeEngine, element: ReactEle const tree = renderHostTree(element); const { images, warnings: imageWarnings } = await loadImages(imageSources(tree)); - const layOut = async (keepNestedRowsWhole: boolean) => { - const { document, warnings } = toFormeDocument(tree, { images, keepNestedRowsWhole }); + const breakBeforeListItems = new Set(); + const layOutOnce = async (keepNestedRowsWhole: boolean) => { + const { document, warnings } = toFormeDocument(tree, { images, keepNestedRowsWhole, breakBeforeListItems }); // Forme rewrites the font entries it's given (bytes to base64), so each render gets its own. const render = () => engine.renderSerializedDocWithLayout({ ...document, fonts: fonts.map((font) => ({ ...font })) }); @@ -126,6 +145,17 @@ export async function renderResumeElement(engine: FormeEngine, element: ReactEle return { result, warnings }; }; + // A marker left on the page its first line leaves: that item starts the next page instead. Breaks move what + // follows, so a few passes settle it; an item that already starts a page is never broken again. + const layOut = async (keepNestedRowsWhole: boolean) => { + for (let pass = 0; ; pass++) { + const laidOut = await layOutOnce(keepNestedRowsWhole); + const leftBehind = listMarkersLeftBehind(laidOut.result.layout).filter((item) => !breakBeforeListItems.has(item)); + if (leftBehind.length === 0 || pass === 3) return laidOut; + for (const item of leftBehind) breakBeforeListItems.add(item); + } + }; + let { result, warnings } = await layOut(false); if (misplacesABox(result)) ({ result, warnings } = await layOut(true)); if (misplacesABox(result)) diff --git a/packages/pdf/src/forme/to-forme.tsx b/packages/pdf/src/forme/to-forme.tsx index df1cb5a01..fcf0b7fc6 100644 --- a/packages/pdf/src/forme/to-forme.tsx +++ b/packages/pdf/src/forme/to-forme.tsx @@ -32,6 +32,14 @@ type Edges = { top: number; right: number; bottom: number; left: number }; /** Source location of a repeated page background, so free-form measuring can skip it. */ export const FIXED_SOURCE = "rr-fixed"; +/** + * List items carry their place in the document into the layout: a source location's line is the item's index + 1 + * and its column says what the box is. Forme has no keep-with-next, so `renderResume` finds markers left on a page + * their first line leaves, and renders again with a page break before those items. + */ +export const LIST_ROLE = { marker: 2, content: 3, item: 4 } as const; +const LIST_SOURCE = "rr-list"; + type Context = { fontSize: number; pageWidth: number; @@ -56,6 +64,10 @@ type Context = { textDefaults: FormeStyle; warnings: Set; sourceMap: WeakMap; + /** Counts list items in document order and names those that start a page (see `LIST_ROLE`). */ + lists: { count: number; breakBefore: ReadonlySet }; + /** The list item being converted, and what part of it. */ + listItem?: { index: number; role: (typeof LIST_ROLE)[keyof typeof LIST_ROLE] } | undefined; }; const PADDING_KEYS = [ @@ -353,8 +365,17 @@ function convertNode(node: HostNode, parentContext: Context, key: number): React const ownKey = props[RESUME_NODE_PROP]; // Everything drawn inside a tagged block carries its key, so a block Forme leaves out of its layout (it does when // a plain box breaks across pages) can be found by what it contains. - const context = - typeof ownKey === "string" && ownKey.length > 0 ? { ...parentContext, nodeKey: ownKey } : parentContext; + let context = typeof ownKey === "string" && ownKey.length > 0 ? { ...parentContext, nodeKey: ownKey } : parentContext; + if (props["data-resume-list-item"]) + context = { ...context, listItem: { index: context.lists.count++, role: LIST_ROLE.item } }; + else if (context.listItem && (props["data-resume-list-marker"] || props["data-resume-list-content"])) + context = { + ...context, + listItem: { + index: context.listItem.index, + role: props["data-resume-list-marker"] ? LIST_ROLE.marker : LIST_ROLE.content, + }, + }; switch (node.type) { case HOST.view: { @@ -366,6 +387,8 @@ function convertNode(node: HostNode, parentContext: Context, key: number): React ); let children = spread.children; const viewStyle = flowStyle(props, spread.style); + if (context.listItem?.role === LIST_ROLE.item && context.lists.breakBefore.has(context.listItem.index)) + viewStyle.breakBefore = true; // Forme ignores a page break on an item of a row: the row takes it, as the item can't start a page without it. if (style.flexDirection === "row" || style.flexDirection === "row-reverse") { const breaks = (child: ReactNode): child is ReactElement<{ style: FormeStyle }> => @@ -733,10 +756,13 @@ function fixedOnPage(element: ReactElement, style: FormeStyle, context: Context, /** Remembers which Forme element draws a tagged block, so its box can be found in the layout afterwards. */ function tagNode(element: object, props: Record, context: Context) { const key = props[RESUME_NODE_PROP]; + const { listItem } = context; + const place = listItem ? { line: listItem.index + 1, column: listItem.role } : { line: 1, column: 1 }; if (typeof key === "string" && key.length > 0) - context.sourceMap.set(element, { file: `${NODE_SOURCE_PREFIX}${key}`, line: 1, column: 1 }); + context.sourceMap.set(element, { file: `${NODE_SOURCE_PREFIX}${key}`, ...place }); else if (context.nodeKey) - context.sourceMap.set(element, { file: `${NODE_CONTENT_PREFIX}${context.nodeKey}`, line: 1, column: 1 }); + context.sourceMap.set(element, { file: `${NODE_CONTENT_PREFIX}${context.nodeKey}`, ...place }); + else if (listItem) context.sourceMap.set(element, { file: LIST_SOURCE, ...place }); } function convertPage(page: HostElement, context: Context, key: number): ReactNode { @@ -808,11 +834,13 @@ export type ConvertOptions = { * converts without it first, and again with it only when the engine misplaces a box. */ keepNestedRowsWhole?: boolean; + /** List items (by index, see `LIST_ROLE`) that start a new page, so their marker stays with their first line. */ + breakBeforeListItems?: ReadonlySet; }; export function toFormeDocument( tree: HostNode[], - { images = new Map(), keepNestedRowsWhole = false }: ConvertOptions = {}, + { images = new Map(), keepNestedRowsWhole = false, breakBeforeListItems = new Set() }: ConvertOptions = {}, ): ConvertedDocument { const root = tree.find((node): node is HostElement => node.type === HOST.document); if (!root) throw new Error("The resume didn't render a ."); @@ -838,6 +866,7 @@ export function toFormeDocument( }, warnings: new Set(), sourceMap: new WeakMap(), + lists: { count: 0, breakBefore: breakBeforeListItems }, }; const { props } = root; diff --git a/packages/pdf/src/templates/shared/list-pagination.test.tsx b/packages/pdf/src/templates/shared/list-pagination.test.tsx index 918085d79..09067dd86 100644 --- a/packages/pdf/src/templates/shared/list-pagination.test.tsx +++ b/packages/pdf/src/templates/shared/list-pagination.test.tsx @@ -65,8 +65,9 @@ async function listPages( } } -// react-pdf kept a list marker with its item's first line through a patch to its layout engine. Forme 0.25 has no -// keep-with-next, so a marker can stay on a page its first line leaves; these pass once the engine can keep them. +// Forme 0.25 has no keep-with-next: `renderResume` finds a marker left on a page its first line leaves and renders +// again with that item starting the next page. Presence hints are still ignored (the editor warns); those tests +// pass once the engine supports them. describe("list marker pagination (#3344)", () => { it("terminates when an authored marker presence hint exceeds a whole page", async () => { if (process.env.RR_LIST_PRESENCE_PROBE !== "1") { @@ -116,7 +117,7 @@ describe("list marker pagination (#3344)", () => { } } }, 70000); - it.fails("moves a bullet with its first paragraph when the paragraph cannot start on this page", async () => { + it("moves a bullet with its first paragraph when the paragraph cannot start on this page", async () => { const result = await listPages(194); expect(result.first).toBe(1); expect(result.marker).toBe(result.first); @@ -126,7 +127,7 @@ describe("list marker pagination (#3344)", () => { expect(result.first).toBe(0); expect(result.marker).toBe(result.first); }); - it.fails("allows a long list item to continue across pages", async () => { + it("allows a long list item to continue across pages", async () => { const result = await listPages(194, 30); expect(result.marker).toBe(result.first); expect(result.last).toBeGreaterThan(result.first); @@ -138,7 +139,7 @@ describe("list marker pagination (#3344)", () => { .match(/\bSome\b/g), ).toHaveLength(30); }); - it.fails("respects authored paragraph orphan counts", async () => { + it("respects authored paragraph orphan counts", async () => { const result = await listPages(190, 30, "paragraph { orphans: 3; }", { multipleParagraphs: true }); expect(result.marker).toBe(result.first); expect(result.last).toBeGreaterThan(result.first); @@ -152,7 +153,7 @@ describe("list marker pagination (#3344)", () => { const result = await listPages(margin, 30, "list-item-content { order: -1; }"); expect(result.marker).toBe(result.first); }); - it.fails.each([194, 200, 208])("keeps reordered list markers with content at margin %i", async (margin) => { + it.each([194, 200, 208])("keeps reordered list markers with content at margin %i", async (margin) => { const result = await listPages(margin, 30, "list-item-content { order: -1; }"); expect(result.marker).toBe(result.first); }); @@ -160,11 +161,11 @@ describe("list marker pagination (#3344)", () => { const result = await listPages(margin, 30, "", { rtl: true }); expect(result.marker).toBe(result.first); }); - it.fails.each([194, 200, 208])("keeps RTL markers with content at margin %i", async (margin) => { + it.each([194, 200, 208])("keeps RTL markers with content at margin %i", async (margin) => { const result = await listPages(margin, 30, "", { rtl: true }); expect(result.marker).toBe(result.first); }); - it.fails("keeps ordered markers with first text", async () => { + it("keeps ordered markers with first text", async () => { const result = await listPages(194, 30, "", { html: `
  1. TARGET ${"Some words to fill several lines and force wrapping. ".repeat(30)} END
`, }); @@ -204,7 +205,7 @@ describe("list marker pagination (#3344)", () => { expect(result.first).toBe(0); expect(result.marker).toBe(result.first); }); - it.fails("keeps the marker with a paragraph using larger text", async () => { + it("keeps the marker with a paragraph using larger text", async () => { const result = await listPages(180, 30, "paragraph { font-size: 15pt; orphans: 2; }", { multipleParagraphs: true }); expect(result.marker).toBe(result.first); });