Ryan/Reactive-Resume
1
0
Fork 0
mirror of https://github.com/AmruthPillai/Reactive-Resume.git synced 2026-10-04 10:43:46 +10:00
Code Issues Packages Projects Releases Wiki Activity

fix(pdf): keep a list marker with its item's first line

Browse Source
This commit is contained in:
Amruth Pillai
2026-09-29 08:25:06 +02:00
parent ce2f1857f9
commit 3405200cf4
3 changed files with 77 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/pdf/src/forme/render.ts
+33 -3
View File
@@ -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<number, number>();
const contentPage = new Map<number, number>();
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<number>();
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))
packages/pdf/src/forme/to-forme.tsx
+34 -5
View File
@@ -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<string>;
sourceMap: WeakMap<object, SourceLocation>;
/** Counts list items in document order and names those that start a page (see `LIST_ROLE`). */
lists: { count: number; breakBefore: ReadonlySet<number> };
/** 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<string, unknown>, 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<number>;
};
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 <Document>.");
@@ -838,6 +866,7 @@ export function toFormeDocument(
},
warnings: new Set(),
sourceMap: new WeakMap(),
lists: { count: 0, breakBefore: breakBeforeListItems },
};
const { props } = root;
packages/pdf/src/templates/shared/list-pagination.test.tsx
+10 -9
View File
@@ -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: `<ol><li>TARGET ${"Some words to fill several lines and force wrapping. ".repeat(30)} END</li></ol>`,
});
@@ -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);
});