Skip to content

Commit

Permalink
fix: broken, infinitely looping minPresenceAhead calculation (#2400)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
carlobeltrame authored Jan 15, 2024
1 parent def2bcd commit 0538bd9
Show file tree
Hide file tree
Showing 4 changed files with 466 additions and 49 deletions.
5 changes: 5 additions & 0 deletions .changeset/mighty-birds-eat.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@react-pdf/layout': minor
---

Rework minPresenceAhead detection and add tests
22 changes: 0 additions & 22 deletions packages/layout/src/node/getNodesHeight.js

This file was deleted.

55 changes: 28 additions & 27 deletions packages/layout/src/node/shouldBreak.js
Original file line number Diff line number Diff line change
@@ -1,46 +1,47 @@
/* eslint-disable no-continue */

import getWrap from './getWrap';
import getNodesHeight from './getNodesHeight';

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

const getMinPresenceAhead = node => node.props?.minPresenceAhead;
const getMinPresenceAhead = node => node.props?.minPresenceAhead || 0;

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

const getPresenceAhead = (elements, height) => {
let result = 0;

for (let i = 0; i < elements.length; i += 1) {
const element = elements[i];

if (!element.box) continue;

const isElementInside = height > element.box.top;
const presenceAhead =
element.props.presenceAhead || defaultPresenceAhead(element);

if (element && isElementInside) {
result += presenceAhead(height - element.box.top);
}
}
const getEndOfMinPresenceAhead = child => {
return (
child.box.top +
child.box.height +
child.box.marginBottom +
getMinPresenceAhead(child)
);
};

return result;
const getEndOfPresence = (child, futureElements) => {
const afterMinPresenceAhead = getEndOfMinPresenceAhead(child);
const endOfFurthestFutureElement = getFurthestEnd(
futureElements.filter(node => !node.props?.fixed),
);
return Math.min(afterMinPresenceAhead, endOfFurthestFutureElement);
};

const shouldBreak = (child, futureElements, height) => {
const minPresenceAhead = getMinPresenceAhead(child);
const presenceAhead = getPresenceAhead(futureElements, height);
const futureHeight = getNodesHeight(futureElements);
if (child.props?.fixed) return false;

const shouldSplit = height < child.box.top + child.box.height;
const shouldWrap = getWrap(child);
const canWrap = getWrap(child);

// Calculate the y coordinate where the desired presence of the child ends
const endOfPresence = getEndOfPresence(child, futureElements);
// If the child is already at the top of the page, breaking won't improve its presence
// (as long as react-pdf does not support breaking into differently sized containers)
const breakingImprovesPresence = child.box.top > child.box.marginTop;

return (
getBreak(child) ||
(!shouldWrap && shouldSplit) ||
(minPresenceAhead < futureHeight && presenceAhead < minPresenceAhead)
(shouldSplit && !canWrap) ||
(!shouldSplit && endOfPresence > height && breakingImprovesPresence)
);
};

Expand Down
Loading

0 comments on commit 0538bd9

Please sign in to comment.