fix(layout): don't squash a container's sole child at a page break (#3449) - #3450
billinspiratobell wants to merge 2 commits into
Conversation
…iegomura#3449) When splitting a node whose children all move to the next page, splitNodes kept the (now empty) parent on the current page whenever `currentChildren` was empty. That is a per-container signal, not proof the page is empty, so an element that is the only child of its container but lands on an already- populated page (a `wrap={false}` block, an itinerary row, etc.) was force-fit into the little space left and its lines were drawn on top of each other ("overlapping at the page break"). Base the keep-vs-move decision on whether the page is empty *above* the node, threaded through the split recursion. When the page already has content the node now moves to the next page; it is only kept (and allowed to overflow) when the page is genuinely empty, which also preserves termination and avoids an infinite page loop. Adds regression tests covering the overlap across several page heights and the tall-fixed-header loop guard. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 38a3154 The changes in this PR will be included in the next version bump. This PR includes changesets to release 7 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
OK this is ready. See my https://react-pdf.org/repl paste in the code above. It also fixes the recursion. test case is there too. |
|
CAN WE GET THIS MERGED? |
|
please merge this - it is critical fixes. |
|
This should be obsolete after pagination overhaul done in #3502 |
|
This is still not fixed. See the fix I put in. the legacy path is still the default and still ships the bug, so the fix isn't obsolete until the cutover major lands. // layout 5.2.0, lib/index.js |
Fixes #3449.
Problem
Content overlaps itself at a page break — a block that lands near the bottom of a page is force-fit into the few points of space left, drawing its lines on top of each other, instead of moving to the next page. It shows up with two-column rows /
wrap={false}blocks (headers, footers, itinerary-style entries) near a page boundary, and is present on the latest published@react-pdf/layout(4.6.1).A full step-by-step reproduction (paste-into-REPL) is in
REPL.md.Root cause
In
resolvePagination'ssplitNodes, when a node is split and all of its children move to the next page (leaving an empty shell), the parent is kept on the current page whenevercurrentChildren.length === 0:currentChildren.length === 0is meant to mean "the page is empty, so keep this here (even if it overflows) rather than emit an empty page / loop forever." ButcurrentChildrenis the local list for the node's own container — it is0simply because the node is the first child of its container, which is not the same as the page being empty. So a node that is the sole child of its container but lands on an already-populated page is force-fit into the space left, and Yoga compresses its lines on top of each other.Fix
Decide based on whether the page is actually empty above the node, threaded through the split recursion as
pageEmpty:The change is contained to
splitNodes/split/splitView/splitChildren.Tests
Added to
packages/layout/tests/steps/resolvePagination.test.ts:wrap={false}two-column entry that is the sole child of its container, swept across several page heights. Fails before this change (7/6/5/3 overlapping spans), passes after.All 416 layout tests pass. Also verified end-to-end by rendering a real document that reproduced the overlap: 8 overlapping spans → 0 after the fix (the entry moves cleanly to the next page).
🤖 Generated with Claude Code