Skip to content

Commit f1d8b6f

Browse files
committed
fix: include fixed siblings and margins in next-page box.top synthesis
The relayout-skip optimization left nextPage input with stale box.top values. For a page with a normal-flow fixed header above a flex:1 body, the next splitPage iteration saw the body at top=0 instead of below the header. splitChildren then over-allocated content into subsequent pages and yoga flex-shrank the result on render.
1 parent c415035 commit f1d8b6f

5 files changed

Lines changed: 317 additions & 21 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@react-pdf/layout": patch
3+
---
4+
5+
fix: synthesize page-relative `box.top` for nodes pushed to the next page during pagination so subsequent iterations match what a yoga relayout would have produced. This accounts for normal-flow fixed siblings (e.g., a fixed header above a flex:1 body) and for sibling margins (`marginTop`/`marginBottom`). Without these, common two-pane report layouts packed too much content onto subsequent pages, causing yoga to flex-shrink content and overflow page borders.

packages/layout/src/steps/resolvePagination.ts

Lines changed: 60 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -85,46 +85,90 @@ const warnUnavailableSpace = (node: SafeNode) => {
8585
);
8686
};
8787

88+
const isAbsolutePositioned = (node: SafeNode) =>
89+
node.style?.position === 'absolute';
90+
91+
// Sum the vertical space a node would occupy in a flex column: top margin +
92+
// height + bottom margin. Yoga's flex layout positions siblings using this
93+
// (margins do not collapse in flexbox), so we mirror it when synthesizing
94+
// next-page positions without a yoga relayout.
95+
const getOutsetHeight = (node: SafeNode) =>
96+
(node.box?.marginTop || 0) +
97+
(node.box?.height || 0) +
98+
(node.box?.marginBottom || 0);
99+
88100
const splitNodes = (height: number, contentArea: number, nodes: SafeNode[]) => {
89101
const currentChildren: SafeNode[] = [];
90102
const nextChildren: SafeNode[] = [];
91103
const suffixFurthestEnd = computeSuffixFurthestEnd(nodes);
92104
const fixedIndices = collectFixedIndices(nodes);
105+
const length = nodes.length;
93106

94107
const pushFutureFixed = (target: SafeNode[], afterIndex: number) => {
95108
for (const idx of fixedIndices) {
96109
if (idx > afterIndex) target.push(nodes[idx]);
97110
}
98111
};
99112

100-
// Adjust box.top for remaining nodes that are pushed to nextChildren without relayout.
101-
// Fixed nodes keep their positions (absolute positioning).
102-
const adjustRemaining = (fromIndex: number): SafeNode[] =>
103-
nodes.slice(fromIndex).map((node) => {
104-
if (isFixed(node)) return node;
105-
return Object.assign({}, node, {
106-
box: Object.assign({}, node.box, { top: node.box.top - height }),
107-
});
113+
// The next iteration's splitNodes makes decisions from `box.top`/`box.height`.
114+
// To match what yoga's relayout would have produced (the dropped step), we
115+
// synthesize page-relative positions: cumFixedHeight is the bottom edge of the
116+
// last normal-flow fixed sibling, cumNonFixedNextHeight is the bottom edge of
117+
// the last non-fixed sibling already pushed to nextChildren.
118+
let cumFixedHeight = 0;
119+
let cumNonFixedNextHeight = 0;
120+
121+
// Returns `node` cloned with `box.top` set to its expected position on the
122+
// next page. Mutates the running cursor — must only be called when pushing to
123+
// nextChildren, in left-to-right sibling order.
124+
const placeOnNextPage = (node: SafeNode): SafeNode => {
125+
const marginTop = node.box?.marginTop || 0;
126+
const newTop = cumFixedHeight + cumNonFixedNextHeight + marginTop;
127+
cumNonFixedNextHeight += getOutsetHeight(node);
128+
return Object.assign({}, node, {
129+
box: Object.assign({}, node.box, { top: newTop }),
108130
});
131+
};
132+
133+
// Keep cumFixedHeight in sync for normal-flow fixed siblings we encounter
134+
// after the loop's break point — without this, non-fixed siblings that
135+
// follow them in nextChildren would be positioned too high.
136+
const advanceFixed = (node: SafeNode) => {
137+
if (!isAbsolutePositioned(node)) {
138+
cumFixedHeight += getOutsetHeight(node);
139+
}
140+
};
141+
142+
const adjustRemaining = (fromIndex: number): SafeNode[] => {
143+
const result: SafeNode[] = [];
144+
for (let j = fromIndex; j < length; j += 1) {
145+
const node = nodes[j];
146+
if (isFixed(node)) {
147+
result.push(node);
148+
advanceFixed(node);
149+
continue;
150+
}
151+
result.push(placeOnNextPage(node));
152+
}
153+
return result;
154+
};
109155

110156
let hasNonFixedPrevious = false;
111157

112-
const length = nodes.length;
113158
for (let i = 0; i < length; i += 1) {
114159
const child = nodes[i];
115160

116161
if (isFixed(child)) {
117162
nextChildren.push(child);
118163
currentChildren.push(child);
164+
advanceFixed(child);
119165
continue;
120166
}
121167

122168
const nodeTop = getTop(child);
123169
const isOutside = height <= nodeTop;
124170
if (isOutside) {
125-
const box = Object.assign({}, child.box, { top: child.box.top - height });
126-
const next = Object.assign({}, child, { box });
127-
nextChildren.push(next);
171+
nextChildren.push(placeOnNextPage(child));
128172
continue;
129173
}
130174

@@ -144,12 +188,12 @@ const splitNodes = (height: number, contentArea: number, nodes: SafeNode[]) => {
144188
hasNonFixedPrevious,
145189
);
146190
if (shouldBreak) {
147-
const box = Object.assign({}, child.box, { top: child.box.top - height });
148191
const props = Object.assign({}, child.props, {
149192
wrap: true,
150193
break: false,
151194
});
152-
const next = Object.assign({}, child, { box, props });
195+
const placed = placeOnNextPage(child);
196+
const next = Object.assign({}, placed, { props });
153197

154198
pushFutureFixed(currentChildren, i);
155199
nextChildren.push(next, ...adjustRemaining(i + 1));
@@ -166,19 +210,14 @@ const splitNodes = (height: number, contentArea: number, nodes: SafeNode[]) => {
166210
pushFutureFixed(currentChildren, i);
167211
nextChildren.push(...adjustRemaining(i + 1));
168212
} else {
169-
const box = Object.assign({}, child.box, {
170-
top: child.box.top - height,
171-
});
172-
const next = Object.assign({}, child, { box });
173-
174213
pushFutureFixed(currentChildren, i);
175-
nextChildren.push(next, ...adjustRemaining(i + 1));
214+
nextChildren.push(placeOnNextPage(child), ...adjustRemaining(i + 1));
176215
}
177216
break;
178217
}
179218

180219
if (currentChild) currentChildren.push(currentChild);
181-
if (nextChild) nextChildren.push(nextChild);
220+
if (nextChild) nextChildren.push(placeOnNextPage(nextChild));
182221

183222
hasNonFixedPrevious = true;
184223
continue;
Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,178 @@
1+
import { describe, expect, test } from 'vitest';
2+
import FontStore from '@react-pdf/font';
3+
4+
import { loadYoga } from '../../src/yoga';
5+
import resolvePagination from '../../src/steps/resolvePagination';
6+
import resolveDimensions from '../../src/steps/resolveDimensions';
7+
import { SafeDocumentNode, SafeNode } from '../../src/types';
8+
9+
const fontStore = new FontStore();
10+
11+
const calcLayout = (node: SafeDocumentNode) =>
12+
resolvePagination(resolveDimensions(node, fontStore), fontStore);
13+
14+
const makeBlock = (id: string, style: Record<string, number>): SafeNode =>
15+
({
16+
type: 'VIEW',
17+
style,
18+
props: { id },
19+
children: [],
20+
}) as any;
21+
22+
describe('pagination with page-level fixed siblings', () => {
23+
// Regression for a bug in the splitNodes optimization: when a page has a
24+
// normal-flow fixed sibling (header) above a flex:1 container that must split
25+
// across pages, the next-page input was given box.top=0 instead of the actual
26+
// header-offset position. splitChildren then computed availableHeight=wrapArea
27+
// instead of wrapArea-headerHeight, packing too much content onto subsequent
28+
// pages and causing yoga to compress sections (content loss).
29+
test('flex:1 body below fixed header splits without losing content', async () => {
30+
const yoga = await loadYoga();
31+
32+
const layout = calcLayout({
33+
type: 'DOCUMENT',
34+
yoga,
35+
props: {},
36+
children: [
37+
{
38+
type: 'PAGE',
39+
props: {},
40+
style: { width: 100, height: 100 },
41+
children: [
42+
{
43+
type: 'VIEW',
44+
style: { height: 20 },
45+
props: { id: 'header', fixed: true },
46+
children: [],
47+
},
48+
{
49+
type: 'VIEW',
50+
style: { flex: 1, position: 'relative' },
51+
props: { id: 'body' },
52+
children: [
53+
{
54+
type: 'VIEW',
55+
style: {
56+
position: 'absolute',
57+
bottom: 0,
58+
left: 0,
59+
right: 0,
60+
height: 1,
61+
},
62+
props: { id: 'bottomBorder', fixed: true },
63+
children: [],
64+
},
65+
makeBlock('section1', { height: 50 }) as any,
66+
makeBlock('section2', { height: 50 }) as any,
67+
makeBlock('section3', { height: 50 }) as any,
68+
makeBlock('section4', { height: 50 }) as any,
69+
makeBlock('section5', { height: 50 }) as any,
70+
],
71+
},
72+
],
73+
},
74+
],
75+
});
76+
// 5 sections × 50pt = 250pt; body area per page = 100 - 20 (header) = 80pt.
77+
// ceil(250 / 80) = 4 pages expected.
78+
expect(layout.children.length).toBe(4);
79+
80+
// Verify the body on every page is laid out below the header.
81+
for (const page of layout.children) {
82+
const body = page.children!.find(
83+
(c) => c.props && (c.props as any).id === 'body',
84+
);
85+
expect(body).toBeDefined();
86+
expect(body!.box!.top).toBe(20);
87+
expect(body!.box!.height).toBe(80);
88+
}
89+
90+
// Verify no content was lost — the heights of all section fragments across
91+
// pages should sum back to the original (50pt × 5 = 250pt). Buggy behavior
92+
// compresses sections via yoga shrink and the sum is < 250.
93+
const fragmentHeightById = new Map<string, number>();
94+
for (const page of layout.children) {
95+
const body = page.children!.find(
96+
(c) => c.props && (c.props as any).id === 'body',
97+
)!;
98+
for (const child of body.children || []) {
99+
const id = (child.props as any).id as string;
100+
if (!id?.startsWith('section')) continue;
101+
fragmentHeightById.set(
102+
id,
103+
(fragmentHeightById.get(id) || 0) + (child.box?.height ?? 0),
104+
);
105+
}
106+
}
107+
for (const [id, total] of fragmentHeightById) {
108+
expect(total, `section ${id} total height after split`).toBe(50);
109+
}
110+
expect(fragmentHeightById.size).toBe(5);
111+
});
112+
113+
// Regression for incomplete fix: cumulative tracking in splitNodes ignored
114+
// marginTop/marginBottom of siblings. Yoga's layout positions siblings with
115+
// these margins included, so without margin awareness the synthesized box.top
116+
// values for the next page diverge from what yoga relayout would produce —
117+
// splitNode's `current.style.height = height - nodeTop` becomes too large and
118+
// yoga compresses content via flex-shrink.
119+
test('header marginBottom shifts body splits and no content is compressed', async () => {
120+
const yoga = await loadYoga();
121+
122+
const layout = calcLayout({
123+
type: 'DOCUMENT',
124+
yoga,
125+
props: {},
126+
children: [
127+
{
128+
type: 'PAGE',
129+
props: {},
130+
style: { width: 100, height: 100 },
131+
children: [
132+
{
133+
type: 'VIEW',
134+
style: { height: 20, marginBottom: 10 },
135+
props: { id: 'header', fixed: true },
136+
children: [],
137+
},
138+
makeBlock('section1', { height: 30 }) as any,
139+
makeBlock('section2', { height: 30 }) as any,
140+
makeBlock('section3', { height: 30 }) as any,
141+
makeBlock('section4', { height: 30 }) as any,
142+
makeBlock('section5', { height: 30 }) as any,
143+
],
144+
},
145+
],
146+
});
147+
148+
// Effective space below header per page = 100 - 20 - 10 (mb) = 70.
149+
// 5 sections × 30pt = 150pt; ceil(150 / 70) = 3 pages expected.
150+
expect(layout.children.length).toBe(3);
151+
152+
// Heights of each section across pages should sum to original 30.
153+
const fragmentHeightById = new Map<string, number>();
154+
for (const page of layout.children) {
155+
for (const child of page.children || []) {
156+
const id = (child.props as any).id as string;
157+
if (!id?.startsWith('section')) continue;
158+
fragmentHeightById.set(
159+
id,
160+
(fragmentHeightById.get(id) || 0) + (child.box?.height ?? 0),
161+
);
162+
}
163+
}
164+
for (const [id, total] of fragmentHeightById) {
165+
expect(total, `section ${id} total height after split`).toBe(30);
166+
}
167+
expect(fragmentHeightById.size).toBe(5);
168+
169+
// Header must keep its original height on every page (no flex-shrink).
170+
for (const page of layout.children) {
171+
const header = page.children!.find(
172+
(c) => c.props && (c.props as any).id === 'header',
173+
);
174+
expect(header).toBeDefined();
175+
expect(header!.box!.height).toBe(20);
176+
}
177+
});
178+
});
Loading
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
import { describe, expect, test } from 'vitest';
2+
3+
import { Document, Page, View, Text, StyleSheet } from '@react-pdf/renderer';
4+
import renderToImage from './renderComponent';
5+
6+
const BORDER = '1pt solid #000';
7+
8+
const styles = StyleSheet.create({
9+
header: {
10+
height: 30,
11+
borderBottom: BORDER,
12+
backgroundColor: '#f0f0f0',
13+
flexDirection: 'row',
14+
paddingHorizontal: 4,
15+
alignItems: 'center',
16+
},
17+
body: {
18+
borderLeft: BORDER,
19+
borderRight: BORDER,
20+
flex: 1,
21+
position: 'relative',
22+
},
23+
bottomBorder: {
24+
position: 'absolute',
25+
bottom: 0,
26+
left: -1,
27+
right: -1,
28+
borderBottom: BORDER,
29+
},
30+
row: {
31+
height: 25,
32+
borderBottom: '0.5pt solid #888',
33+
flexDirection: 'row',
34+
alignItems: 'center',
35+
paddingHorizontal: 4,
36+
},
37+
rowLabel: { flex: 1, fontSize: 7 },
38+
rowValue: { flex: 1, fontSize: 7 },
39+
});
40+
41+
const Row = ({ no }) => (
42+
<View style={styles.row} wrap={false}>
43+
<Text style={styles.rowLabel}>Row {no} label</Text>
44+
<Text style={styles.rowValue}>Row {no} value</Text>
45+
</View>
46+
);
47+
48+
const rowCount = 16;
49+
50+
const TwoPaneTable = () => (
51+
<Document>
52+
<Page size={[260, 200]} style={{ padding: 8 }}>
53+
<View style={styles.header} fixed>
54+
<Text style={{ fontSize: 8 }}>Report header (fixed, 30pt tall)</Text>
55+
</View>
56+
57+
<View style={styles.body}>
58+
<View style={styles.bottomBorder} fixed />
59+
60+
{Array.from({ length: rowCount }, (_, i) => (
61+
<Row key={i} no={i + 1} />
62+
))}
63+
</View>
64+
</Page>
65+
</Document>
66+
);
67+
68+
describe('two-pane table page wrap', () => {
69+
test('should match snapshot', async () => {
70+
const image = await renderToImage(<TwoPaneTable />);
71+
72+
expect(image).toMatchImageSnapshot();
73+
}, 30_000);
74+
});

0 commit comments

Comments
 (0)