Skip to content

Commit 746038d

Browse files
committed
fix: broken, infinitely looping minPresenceAhead calculation (#2400)
* Rework breaking logic and add tests to fix minPresenceAhead The old minPresenceAhead had some major issues, among others because vertical margins were completely ignored in the presence calculations. * Add changeset * Add regression tests * Ignore fixed elements when calculating page breaks * Do not break before a wrapping element * Rename variable to make it easier to understand * Refactor for better readability
1 parent cf6809f commit 746038d

File tree

4 files changed

+466
-49
lines changed

4 files changed

+466
-49
lines changed

.changeset/mighty-birds-eat.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@react-pdf/layout': minor
3+
---
4+
5+
Rework minPresenceAhead detection and add tests

packages/layout/src/node/getNodesHeight.js

Lines changed: 0 additions & 22 deletions
This file was deleted.

packages/layout/src/node/shouldBreak.js

Lines changed: 28 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,46 +1,47 @@
11
/* eslint-disable no-continue */
22

33
import getWrap from './getWrap';
4-
import getNodesHeight from './getNodesHeight';
54

65
const getBreak = node => node.props?.break || false;
76

8-
const getMinPresenceAhead = node => node.props?.minPresenceAhead;
7+
const getMinPresenceAhead = node => node.props?.minPresenceAhead || 0;
98

10-
const defaultPresenceAhead = element => height =>
11-
Math.min(element.box.height, height);
9+
const getFurthestEnd = elements =>
10+
Math.max(...elements.map(node => node.box.top + node.box.height));
1211

13-
const getPresenceAhead = (elements, height) => {
14-
let result = 0;
15-
16-
for (let i = 0; i < elements.length; i += 1) {
17-
const element = elements[i];
18-
19-
if (!element.box) continue;
20-
21-
const isElementInside = height > element.box.top;
22-
const presenceAhead =
23-
element.props.presenceAhead || defaultPresenceAhead(element);
24-
25-
if (element && isElementInside) {
26-
result += presenceAhead(height - element.box.top);
27-
}
28-
}
12+
const getEndOfMinPresenceAhead = child => {
13+
return (
14+
child.box.top +
15+
child.box.height +
16+
child.box.marginBottom +
17+
getMinPresenceAhead(child)
18+
);
19+
};
2920

30-
return result;
21+
const getEndOfPresence = (child, futureElements) => {
22+
const afterMinPresenceAhead = getEndOfMinPresenceAhead(child);
23+
const endOfFurthestFutureElement = getFurthestEnd(
24+
futureElements.filter(node => !node.props?.fixed),
25+
);
26+
return Math.min(afterMinPresenceAhead, endOfFurthestFutureElement);
3127
};
3228

3329
const shouldBreak = (child, futureElements, height) => {
34-
const minPresenceAhead = getMinPresenceAhead(child);
35-
const presenceAhead = getPresenceAhead(futureElements, height);
36-
const futureHeight = getNodesHeight(futureElements);
30+
if (child.props?.fixed) return false;
31+
3732
const shouldSplit = height < child.box.top + child.box.height;
38-
const shouldWrap = getWrap(child);
33+
const canWrap = getWrap(child);
34+
35+
// Calculate the y coordinate where the desired presence of the child ends
36+
const endOfPresence = getEndOfPresence(child, futureElements);
37+
// If the child is already at the top of the page, breaking won't improve its presence
38+
// (as long as react-pdf does not support breaking into differently sized containers)
39+
const breakingImprovesPresence = child.box.top > child.box.marginTop;
3940

4041
return (
4142
getBreak(child) ||
42-
(!shouldWrap && shouldSplit) ||
43-
(minPresenceAhead < futureHeight && presenceAhead < minPresenceAhead)
43+
(shouldSplit && !canWrap) ||
44+
(!shouldSplit && endOfPresence > height && breakingImprovesPresence)
4445
);
4546
};
4647

0 commit comments

Comments
 (0)