fix(pdf): keep list markers with their first text fragment (#3443)

* fix(pdf): keep list markers with their first text fragment

* fix(pdf): preserve page breaks while rewinding list companions

* fix(pdf): consume oversized list marker presence hints

* test(pdf): allow cold startup for pagination process guard

* fix(pdf): key list presence spacer
This commit is contained in:
Amruth Pillai
2026-09-05 09:51:09 -07:00
committed by GitHub
parent 861ba8bf60
commit ea2beb8450
5 changed files with 344 additions and 6 deletions
@@ -0,0 +1,259 @@
import { execFile } from "node:child_process";
import { createRequire } from "node:module";
import { dirname, join } from "node:path";
import { fileURLToPath } from "node:url";
import { promisify } from "node:util";
import { describe, expect, it } from "vitest";
import { renderToBuffer } from "@react-pdf/renderer";
import { getDocument } from "pdfjs-dist/legacy/build/pdf.mjs";
import { act } from "react";
import { defaultResumeData } from "@reactive-resume/schema/resume/default";
import { ResumeDocument } from "../../document";
import { resolveResumeRuntime } from "../../semantic/resolve";
async function listPages(
margin: number,
repeats = 5,
css = "",
options: { html?: string; rtl?: boolean; multipleParagraphs?: boolean } = {},
) {
const data = structuredClone(defaultResumeData);
data.metadata.typography.body.fontFamily = "Helvetica";
data.metadata.typography.heading.fontFamily = "Helvetica";
if (options.rtl) data.metadata.page.locale = "ar-SA";
data.metadata.layout.pages = [{ fullWidth: true, main: ["summary"], sidebar: [] }];
data.summary.title = "Summary";
// Normalization unwraps a lone paragraph in a list item. Two paragraphs retain
// real paragraph nodes so authored orphans are exercised by the renderer.
data.summary.content =
options.html ??
`<ul><li><p>TARGET ${"Some words to fill several lines and force wrapping. ".repeat(repeats)} END</p>${options.multipleParagraphs ? "<p>Second paragraph</p>" : ""}</li></ul>`;
data.metadata.stylesheet = {
mode: "semantic",
source: {
languageVersion: 1,
text: `@version 1; page { size: 300pt 300pt; } rich-text { margin-top: ${margin}pt; } ${css}`,
},
};
const runtime = resolveResumeRuntime({ data, template: "onyx", mode: "semantic" });
expect(runtime.diagnostics).toEqual([]);
const bytes = await act(() => renderToBuffer(<ResumeDocument data={data} template="onyx" />));
const task = getDocument({ data: new Uint8Array(bytes), useSystemFonts: true });
try {
const doc = await task.promise;
const pages: string[][] = [];
for (let n = 1; n <= doc.numPages; n++) {
const page = await doc.getPage(n);
const text = await page.getTextContent();
pages.push(text.items.flatMap((item) => ("str" in item && item.str ? [item.str] : [])));
}
return {
marker: pages.findIndex((p) => p.some((s) => s.includes("•") || s.startsWith("1."))),
first: pages.findIndex((p) => p.some((s) => s.includes("TARGET"))),
last: pages.findIndex((p) => p.some((s) => s.includes("END"))),
pages,
};
} finally {
await task.destroy();
}
}
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") {
// The renderer's pagination loop is synchronous. A test timeout cannot
// interrupt it, so run this probe in a killable process with thread workers.
const vitest = join(dirname(createRequire(import.meta.url).resolve("vitest/package.json")), "vitest.mjs");
await promisify(execFile)(
process.execPath,
[
vitest,
"run",
"src/templates/shared/list-pagination.test.tsx",
"-t",
"terminates when an authored marker presence hint exceeds a whole page",
"--pool=threads",
"--maxWorkers=1",
"--passWithNoTests=false",
],
{
cwd: fileURLToPath(new URL("../../../", import.meta.url)),
env: { ...process.env, RR_LIST_PRESENCE_PROBE: "1" },
// Include cold imports when the full suite competes for workers.
timeout: 60000,
killSignal: "SIGKILL",
},
);
return;
}
for (const rtl of [false, true]) {
for (const first of ["list-item-content", "list-marker"]) {
const result = await listPages(
180,
30,
`${first} { order: -1; } list-marker { -resume-min-presence-ahead: 1000pt; }`,
{ rtl },
);
expect(result.first).toBe(1);
expect(result.marker).toBe(result.first);
expect(result.last).toBeGreaterThan(result.first);
expect(result.pages.length).toBeLessThanOrEqual(4);
expect(result.pages.flat().join(" ").match(/•/g)).toHaveLength(1);
expect(
result.pages
.flat()
.join(" ")
.match(/\bSome\b/g),
).toHaveLength(30);
}
}
}, 70000);
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);
});
it("keeps content on the current page when the first paragraph can start", async () => {
const result = await listPages(192);
expect(result.first).toBe(0);
expect(result.marker).toBe(result.first);
});
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);
expect(result.pages.flat().join(" ").match(/•/g)).toHaveLength(1);
expect(
result.pages
.flat()
.join(" ")
.match(/\bSome\b/g),
).toHaveLength(30);
});
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);
});
it("allows the first line when the author requests one orphan line", async () => {
const result = await listPages(194, 30, "paragraph { orphans: 1; }", { multipleParagraphs: true });
expect(result.first).toBe(0);
expect(result.marker).toBe(result.first);
});
it.each([180, 190, 192, 194, 200, 208, 215])(
"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);
},
);
it.each([180, 190, 192, 194, 200, 208, 215])("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("keeps ordered markers with first text", async () => {
const result = await listPages(194, 30, "", {
html: `<ol><li>TARGET ${"Some words to fill several lines and force wrapping. ".repeat(30)} END</li></ol>`,
});
expect(result.marker).toBe(result.first);
});
it("does not add a marker when Semantic CSS hides it", async () => {
const result = await listPages(194, 30, "list-marker { display: none; }");
expect(result.marker).toBe(-1);
expect(result.first).toBe(1);
});
it("respects an explicit marker presence override", async () => {
const result = await listPages(180, 30, "list-marker { -resume-min-presence-ahead: 60pt; }");
expect(result.first).toBe(1);
expect(result.marker).toBe(result.first);
});
it("keeps a nested list marker with its first text", async () => {
const result = await listPages(179, 30, "", {
html: `<ul><li>Outer item<ul><li>TARGET ${"Some words to fill several lines and force wrapping. ".repeat(30)} END</li></ul></li></ul>`,
});
const markerPages = result.pages.flatMap((page, index) =>
page.flatMap((text) => (text.includes("•") ? [index] : [])),
);
expect(markerPages.at(1)).toBe(result.first);
});
it("uses the visible list item's orphan count after filtering", async () => {
const result = await listPages(194, 30, "list-item:first-child { display: none; } paragraph { orphans: 1; }", {
html: `<ul><li>Hidden first item</li><li><p>TARGET ${"Some words to fill several lines and force wrapping. ".repeat(30)} END</p><p>Last paragraph</p></li></ul>`,
});
expect(result.first).toBe(0);
expect(result.marker).toBe(result.first);
expect(result.pages.flat().join(" ")).not.toContain("Hidden first item");
});
it("uses the first rendered paragraph after semantic reordering", async () => {
const result = await listPages(194, 30, "paragraph:last-child { order: -1; orphans: 1; }", {
html: `<ul><li><p>Original first paragraph</p><p>TARGET ${"Some words to fill several lines and force wrapping. ".repeat(30)} END</p></li></ul>`,
});
expect(result.first).toBe(0);
expect(result.marker).toBe(result.first);
});
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);
});
it.each([false, true])("keeps explicitly deferred markers with reordered content (RTL %s)", async (rtl) => {
const result = await listPages(
180,
30,
"list-item-content { order: -1; } list-marker { -resume-min-presence-ahead: 60pt; }",
{ rtl },
);
expect(result.first).toBe(1);
expect(result.marker).toBe(result.first);
expect(
result.pages
.flat()
.join(" ")
.match(/\bSome\b/g),
).toHaveLength(30);
});
it("keeps short content and its marker on one page", async () => {
const result = await listPages(0, 1);
expect(result.pages).toHaveLength(1);
expect(result.marker).toBe(0);
expect(result.first).toBe(0);
});
it.each([
[false, 0],
[false, 180],
[true, 0],
[true, 180],
] as const)(
"consumes reordered marker page breaks and continues long content (RTL %s, margin %i)",
async (rtl, margin) => {
const result = await listPages(
margin,
30,
"list-item-content { order: -1; } list-marker { break-before: page; }",
{ rtl },
);
expect(result.first).toBe(1);
expect(result.marker).toBe(result.first);
expect(result.last).toBeGreaterThan(result.first);
expect(result.pages.flat().join(" ").match(/•/g)).toHaveLength(1);
expect(
result.pages
.flat()
.join(" ")
.match(/\bSome\b/g),
).toHaveLength(30);
},
);
it.each([false, true])("consumes marker-first page breaks without overflowing (RTL %s)", async (rtl) => {
const result = await listPages(180, 30, "list-marker { order: -1; break-before: page; }", { rtl });
expect(result.first).toBe(1);
expect(result.marker).toBe(result.first);
expect(result.last).toBeGreaterThan(result.first);
expect(result.pages.flat().join(" ").match(/•/g)).toHaveLength(1);
expect(
result.pages
.flat()
.join(" ")
.match(/\bSome\b/g),
).toHaveLength(30);
});
});
@@ -248,14 +248,14 @@ export const RichText = ({ children, semanticField }: RichTextProps) => {
const itemStyles = toRichTextStyleArray(style);
const contentItemStyles = itemStyles.map(stripRichTextVerticalMargins);
// The scoped @react-pdf/layout patch keeps these companions together using
// actual text fragments, including authored orphan counts and fallback fonts.
const markerNode = (
<PdfText
key="marker"
data-resume-list-marker
{...resolvedPdfTextProps(markerResolved)}
minPresenceAhead={
markerResolved.minPresenceAhead ?? bodyLineHeight ?? metadata.typography.body.lineHeight
}
style={composeStyles(richListItemMarkerStyle, markerResolved.style)}
style={composeStyles(richListItemMarkerStyle, { alignSelf: "flex-start" }, markerResolved.style)}
>
{marker}
</PdfText>
@@ -265,6 +265,7 @@ export const RichText = ({ children, semanticField }: RichTextProps) => {
const contentNode = rtl ? (
<PdfText
key="content"
data-resume-list-content
{...resolvedPdfTextProps(contentResolved)}
style={composeStyles(
richListItemContentStyle,
@@ -280,6 +281,7 @@ export const RichText = ({ children, semanticField }: RichTextProps) => {
) : (
<View
key="content"
data-resume-list-content
{...resolvedPdfFlowProps(contentResolved)}
style={composeStyles(
richListItemContentStyle,
@@ -311,6 +313,7 @@ export const RichText = ({ children, semanticField }: RichTextProps) => {
// (works fine for split-row/contact-list). Swap DOM order to position the marker.
return (
<View
data-resume-list-item
{...resolvedPdfFlowProps(itemResolved)}
style={composeStyles(
richListItemRowStyle,
@@ -320,6 +323,10 @@ export const RichText = ({ children, semanticField }: RichTextProps) => {
itemResolved.style,
)}
>
{/* React PDF only honors an authored presence hint after a preceding sibling. */}
{markerResolved.minPresenceAhead ? (
<View key="presence-spacer" style={{ position: "absolute", width: 0, height: 0 }} />
) : null}
{renderedChildren}
</View>
);
+70
View File
@@ -0,0 +1,70 @@
diff --git a/lib/index.js b/lib/index.js
index 18c2618f4630b3c1296c8c3cac317b8d7666bbe8..79e1898aadcbbfc78a206e235d868cb178683a07 100644
--- a/lib/index.js
+++ b/lib/index.js
@@ -3163,7 +3163,10 @@ const splitNodes = (height, contentArea, nodes) => {
if (shouldSplit) {
const [currentChild, nextChild] = split(child, height, contentArea);
// All children are moved to the next page, it doesn't make sense to show the parent on the current page
- if (child.children.length > 0 && currentChild.children.length === 0) {
+ // Tagged list companions can intentionally rewind all their children.
+ // Keep their empty fragment and already-consumed break state instead
+ // of restoring the unsplit row (which can exceed a whole page).
+ if (child.children.length > 0 && currentChild.children.length === 0 && !child.props['data-resume-list-item']) {
// But if the current page is empty then we can just include the parent on the current page
if (currentChildren.length === 0) {
currentChildren.push(child, ...futureFixedNodes);
@@ -3196,7 +3199,52 @@ const splitChildren = (height, contentArea, node) => {
};
const splitView = (node, height, contentArea) => {
const [currentNode, nextNode] = splitNode(node, height);
- const [currentChilds, nextChildren] = splitChildren(height, contentArea, node);
+ let [currentChilds, nextChildren] = splitChildren(height, contentArea, node);
+ // Reactive Resume opts list rows into companion pagination. Use the text
+ // fragments after orphan/widow splitting, not estimated font line heights.
+ // Untagged rows and ordinary absolutely positioned nodes are unaffected.
+ if (node.props['data-resume-list-item']) {
+ const hasText = (child) => child && (child.type === P.Text
+ ? child.lines?.length > 0
+ : child.children?.some(hasText));
+ const content = (children) => children.find((child) => child.props['data-resume-list-content']);
+ const markerIndex = node.children.findIndex((child) => child.props['data-resume-list-marker']);
+ const contentIndex = node.children.findIndex((child) => child.props['data-resume-list-content']);
+ const hasMarker = (children) => children.some((child) => child.props['data-resume-list-marker']);
+ const deferredMarker = nextChildren.find((child) => child.props['data-resume-list-marker']);
+ // Presence is advisory once the whole row moves to a fresh page. Consume
+ // the hint on continuation so an impossible window cannot rewind the
+ // same unsplit content indefinitely (including content-first rows).
+ const continuedMarkerProps = (marker) => ({
+ ...(deferredMarker?.props || marker.props),
+ minPresenceAhead: 0,
+ });
+ if (markerIndex !== -1 && !hasMarker(currentChilds) && hasMarker(nextChildren) && hasText(content(currentChilds))) {
+ // A presence hint or page break can defer a marker after content.
+ // Defer unsplit content while retaining the marker's consumed flags.
+ currentChilds = currentChilds.filter((child) => !child.props['data-resume-list-content']);
+ nextChildren = node.children
+ .filter((child) => child.props['data-resume-list-content'] || child.props['data-resume-list-marker'])
+ .map((child) => child.props['data-resume-list-marker']
+ ? { ...child, props: continuedMarkerProps(child) }
+ : child);
+ } else if (markerIndex !== -1 && !hasText(content(currentChilds)) && hasText(content(nextChildren))) {
+ // The first text did not fit. Preserve the authored marker/content
+ // order when moving the marker to the next fragment of this row.
+ const marker = node.children[markerIndex];
+ // Preserve break: false after splitNodes has consumed an explicit
+ // page break; restoring the original props would replay it.
+ const nextMarker = {
+ ...marker,
+ props: continuedMarkerProps(marker),
+ box: { ...marker.box, top: 0 },
+ };
+ currentChilds = currentChilds.filter((child) => !child.props['data-resume-list-marker']);
+ nextChildren = nextChildren.filter((child) => !child.props['data-resume-list-marker']);
+ if (markerIndex < contentIndex) nextChildren.unshift(nextMarker);
+ else nextChildren.push(nextMarker);
+ }
+ }
return [
assingChildren(currentChilds, currentNode),
assingChildren(nextChildren, nextNode),
+3 -2
View File
@@ -12,6 +12,7 @@ overrides:
uuid@<11.1.1: ^11.1.1
patchedDependencies:
'@react-pdf/layout@5.2.0': 75dac7cb8260f9c96cd9ab5522d095a3aa047881a9e9d1d2a597409b45944494
'@react-pdf/textkit': 092a6fe8baf3c472a81cdf2ab52a00ec98ef3364e1b8e744005dbcf219a8a213
importers:
@@ -11197,7 +11198,7 @@ snapshots:
jay-peg: 1.1.1
png-js: 2.0.0
'@react-pdf/layout@5.2.0':
'@react-pdf/layout@5.2.0(patch_hash=75dac7cb8260f9c96cd9ab5522d095a3aa047881a9e9d1d2a597409b45944494)':
dependencies:
'@react-pdf/fns': 3.1.3
'@react-pdf/image': 3.1.2
@@ -11238,7 +11239,7 @@ snapshots:
'@babel/runtime': 7.29.7
'@react-pdf/fns': 3.1.3
'@react-pdf/font': 4.1.2
'@react-pdf/layout': 5.2.0
'@react-pdf/layout': 5.2.0(patch_hash=75dac7cb8260f9c96cd9ab5522d095a3aa047881a9e9d1d2a597409b45944494)
'@react-pdf/primitives': 4.4.0
'@react-pdf/reconciler': 2.0.0(react@19.2.8)
'@react-pdf/render': 4.7.0
+1
View File
@@ -17,4 +17,5 @@ overrides:
samlify@<2.13.0: ^2.13.0
uuid@<11.1.1: ^11.1.1
patchedDependencies:
'@react-pdf/layout@5.2.0': patches/@react-pdf__layout@5.2.0.patch
'@react-pdf/textkit': patches/@react-pdf__textkit.patch